-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add json reporting. #10
Conversation
bed8235
to
c542597
Compare
7122a23
to
b15f93e
Compare
93ffda5
to
5fbc4ac
Compare
private int data_size; | ||
private double get_existing_average_latency; | ||
private double get_existing_p50_latency; | ||
private double get_existing_p90_latency; | ||
private double get_existing_p99_latency; | ||
private double get_existing_std_dev; | ||
private double get_non_existing_average_latency; | ||
private double get_non_existing_p50_latency; | ||
private double get_non_existing_p90_latency; | ||
private double get_non_existing_p99_latency; | ||
private double get_non_existing_std_dev; | ||
private boolean is_cluster; | ||
private int num_of_tasks; | ||
private double set_average_latency; | ||
private double set_p50_latency; | ||
private double set_p90_latency; | ||
private double set_p99_latency; | ||
private double set_std_dev; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are most of these snake case while clientCount is camel case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is how fields in json named - json parser reads these fields: https://github.com/Bit-Quill/babushka/blob/43a313747a9323a3a80bbc11281191732c5a1926/benchmarks/utilities/csv_exporter.py#L9-L32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
San is fixing this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0bf99f9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of comments to fix
@@ -50,7 +47,7 @@ public interface Operation { | |||
void go() throws Exception; | |||
} | |||
|
|||
private static Pair<ChosenAction, Long> getLatency(Map<ChosenAction, Operation> actions) { | |||
public static Pair<ChosenAction, Long> getLatency(Map<ChosenAction, Operation> actions) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have to make this public - not for a test.
Better to add an argument to measurePerformance
that takes a action map. Then create a public method that generates the actionMap in measurePerformance
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides, the tests here add little-to-no value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is added to let IT compiled. Json reporting does not need that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 8544807.
percentile(latencies, 90), | ||
percentile(latencies, 99), | ||
stdDeviation(latencies, avgLatency))); | ||
1e-9 * percentile(latencies, 50), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1e-9 should be a constant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 8544807.
import java.util.Map; | ||
import lombok.Getter; | ||
|
||
public class Reporting { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JsonWriter
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed in 8544807.
private int data_size; | ||
private double get_existing_average_latency; | ||
private double get_existing_p50_latency; | ||
private double get_existing_p90_latency; | ||
private double get_existing_p99_latency; | ||
private double get_existing_std_dev; | ||
private double get_non_existing_average_latency; | ||
private double get_non_existing_p50_latency; | ||
private double get_non_existing_p90_latency; | ||
private double get_non_existing_p99_latency; | ||
private double get_non_existing_std_dev; | ||
private boolean is_cluster; | ||
private int num_of_tasks; | ||
private double set_average_latency; | ||
private double set_p50_latency; | ||
private double set_p90_latency; | ||
private double set_p99_latency; | ||
private double set_std_dev; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
San is fixing this...
Signed-off-by: Yury-Fridlyand <[email protected]>
8544807
to
b243d11
Compare
Signed-off-by: Yury-Fridlyand <[email protected]>
@@ -222,7 +206,7 @@ public static class RunConfiguration { | |||
|
|||
public RunConfiguration() { | |||
configuration = "Release"; | |||
resultsFile = Optional.empty(); | |||
resultsFile = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not support null
s anywhere. If we support null
, we need null
checkers each and every time we use this varilable.
I'd prefer to use the empty string or Optionals to support stdout.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b60cb25.
java/benchmarks/src/main/java/javababushka/benchmarks/utils/Benchmarking.java
Outdated
Show resolved
Hide resolved
concurrentNum, | ||
iterationCounter.get() * 1e9 / (System.nanoTime() - started)); | ||
} | ||
printResults(calculatedResults); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to print to stdout each time?
data.client_count = clientCount; | ||
data.num_of_tasks = numOfTasks; | ||
data.tps = tps; | ||
// TODO: is_cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should raise a ticket to do this :P
…nchmarking.java Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Some minor comments then we can merge.
|
||
public class JsonWriter { | ||
|
||
public static void WriteJson( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Write
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e6ef449.
var json = new String(Files.readAllBytes(path)); | ||
recordings = gson.fromJson(json, collectionType); | ||
} | ||
var data = new Measurements(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe create a constructor method to do everything below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in e6ef449.
Signed-off-by: Yury-Fridlyand <[email protected]>
commit b50d57e Author: Yury-Fridlyand <[email protected]> Date: Tue Nov 21 18:16:20 2023 -0800 Java client jni netty (#32) * Add Java-client benchmarking app Signed-off-by: acarbonetto <[email protected]> * spotless apply Signed-off-by: acarbonetto <[email protected]> * Update on command line options Signed-off-by: acarbonetto <[email protected]> * Update README Signed-off-by: acarbonetto <[email protected]> * Spotless apply: Signed-off-by: acarbonetto <[email protected]> * Update README example Signed-off-by: acarbonetto <[email protected]> * update commandline defaults for review comments Signed-off-by: acarbonetto <[email protected]> * Remove TLS flag argument from option Signed-off-by: acarbonetto <[email protected]> * Add lettuce clients for benchmarking Signed-off-by: acarbonetto <[email protected]> * Spotless apply Signed-off-by: acarbonetto <[email protected]> * Add Jedis clients Signed-off-by: acarbonetto <[email protected]> * Add to app Signed-off-by: acarbonetto <[email protected]> * Add for-loop for data size list Signed-off-by: acarbonetto <[email protected]> * Add TPS for all async items Signed-off-by: acarbonetto <[email protected]> * spotless apply Signed-off-by: acarbonetto <[email protected]> * Fix TPS calculations Signed-off-by: acarbonetto <[email protected]> * Accept TLS as a flag Signed-off-by: acarbonetto <[email protected]> * Start threads; then wait for results Signed-off-by: acarbonetto <[email protected]> * Add java-jni client Signed-off-by: acarbonetto <[email protected]> * Handle Exceptions from client; add JniSyncClient fixes Signed-off-by: acarbonetto <[email protected]> * Clean up latency and add error checking Signed-off-by: acarbonetto <[email protected]> * Minor fixes. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix result printing. Signed-off-by: Yury-Fridlyand <[email protected]> * Add TPS. Signed-off-by: Yury-Fridlyand <[email protected]> * Remove duplicates. Reorganize and fix imports. Signed-off-by: Yury-Fridlyand <[email protected]> * Int ctor fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Iteration 1. Signed-off-by: Yury-Fridlyand <[email protected]> * Iteration 2: connected! Signed-off-by: Yury-Fridlyand <[email protected]> * Iteration 3: `get` and `set`. Signed-off-by: Yury-Fridlyand <[email protected]> * Iteration 4: benchmark. Signed-off-by: Yury-Fridlyand <[email protected]> * Iteration 5: some fixes. Signed-off-by: Yury-Fridlyand <[email protected]> * Change number of threads in Benchmarking threadpool * Revert "Change number of threads in Benchmarking threadpool" This reverts commit e3f7596. * Add more flushing rules and UT. Signed-off-by: Yury-Fridlyand <[email protected]> * Client clean up. Signed-off-by: Yury-Fridlyand <[email protected]> * Client optimizations. (#37) * Client optimizations. Signed-off-by: Yury-Fridlyand <[email protected]> * minor cleanup. Signed-off-by: Yury-Fridlyand <[email protected]> * Optimize building a command. Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor rename. Signed-off-by: Yury-Fridlyand <[email protected]> * Clean up Redis close connection Signed-off-by: Andrew Carbonetto <[email protected]> * Clean up Redis close connection Signed-off-by: Andrew Carbonetto <[email protected]> * Minor changes. Signed-off-by: Yury-Fridlyand <[email protected]> * Add todos to closeConnection() Signed-off-by: Andrew Carbonetto <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Andrew Carbonetto <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Address PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> * Rename Signed-off-by: Yury-Fridlyand <[email protected]> * Rename2 Signed-off-by: Yury-Fridlyand <[email protected]> * Fix CI Signed-off-by: Yury-Fridlyand <[email protected]> * More fixes. Signed-off-by: Yury-Fridlyand <[email protected]> * Some changes. Signed-off-by: Yury-Fridlyand <[email protected]> * add null check Signed-off-by: Yury-Fridlyand <[email protected]> * autoflush Signed-off-by: Yury-Fridlyand <[email protected]> * Apply suggestions from code review Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * minor changes Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: acarbonetto <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Andrew Carbonetto <[email protected]> Co-authored-by: acarbonetto <[email protected]> Co-authored-by: Jonathan Louie <[email protected]> commit bcf188c Author: acarbonetto <[email protected]> Date: Tue Nov 14 14:54:36 2023 -0800 Clean up timer Signed-off-by: acarbonetto <[email protected]> commit 05590b0 Author: Andrew Carbonetto <[email protected]> Date: Tue Nov 14 11:41:51 2023 -0800 Java benchmarks clusters (#34) * Add lettuce cluster client when cluster mode enabled --------- Signed-off-by: Andrew Carbonetto <[email protected]> commit 30f2f62 Merge: a62fe92 8a0449d Author: acarbonetto <[email protected]> Date: Mon Nov 6 14:50:51 2023 -0800 Merge branch 'main' into java_benchmarks commit a62fe92 Author: acarbonetto <[email protected]> Date: Mon Nov 6 14:50:18 2023 -0800 fix java install_and_test script variables Signed-off-by: acarbonetto <[email protected]> commit b00a205 Merge: 9cbc9c2 d533b7f Author: Yury-Fridlyand <[email protected]> Date: Wed Oct 25 11:16:12 2023 -0700 Merge branch 'java_benchmarks' of github.com:Bit-Quill/babushka into java_benchmarks Signed-off-by: Yury-Fridlyand <[email protected]> commit 9cbc9c2 Author: Yury-Fridlyand <[email protected]> Date: Wed Oct 25 11:15:46 2023 -0700 Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> commit d533b7f Author: Yury-Fridlyand <[email protected]> Date: Tue Oct 24 17:16:12 2023 -0700 Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> commit fe9bb98 Merge: c3d235a 231a229 Author: Yury-Fridlyand <[email protected]> Date: Tue Oct 24 17:13:19 2023 -0700 Merge remote-tracking branch 'upstream/main' into java_benchmarks Signed-off-by: Yury-Fridlyand <[email protected]> commit c3d235a Author: Yury-Fridlyand <[email protected]> Date: Fri Oct 20 10:06:23 2023 -0700 Add json reporting. (#10) * Add JSON reporting. Signed-off-by: Yury-Fridlyand <[email protected]> * Fix for #26. Signed-off-by: Yury-Fridlyand <[email protected]> * Update java/benchmarks/src/main/java/javababushka/benchmarks/utils/Benchmarking.java Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> * Use `Optional`. Signed-off-by: Yury-Fridlyand <[email protected]> * Address PR feedback. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> commit 8a0449d Author: SanHalacogluImproving <[email protected]> Date: Wed Oct 18 15:18:08 2023 -0700 Convert client count to snake case for rust benchmark. (#27) * Updated ClientCount to client_count for uniformity for rust. commit 65090b4 Author: SanHalacogluImproving <[email protected]> Date: Tue Oct 17 09:50:38 2023 -0700 Updated ClientCount to client_count for uniformity. commit 540f49a Author: Andrew Carbonetto <[email protected]> Date: Fri Oct 6 15:52:19 2023 -0700 Create clients only once per iteration (#19) Signed-off-by: acarbonetto <[email protected]> commit d99d27a Author: acarbonetto <[email protected]> Date: Fri Oct 6 14:08:29 2023 -0700 Update redis-rs to match main branch Signed-off-by: acarbonetto <[email protected]> commit 1bab56a Author: Yury-Fridlyand <[email protected]> Date: Thu Oct 5 14:01:59 2023 -0700 Add option to run tests on multiple clients in concurrency (#16) * Add option to run tests on multiple clients in concurrency * Common pool of iterations. * Awaiting result from async methods. Signed-off-by: Yury-Fridlyand <[email protected]> * minor fix Signed-off-by: Yury-Fridlyand <[email protected]> * Change while-loop; Spotless Apply Signed-off-by: acarbonetto <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: acarbonetto <[email protected]> Co-authored-by: acarbonetto <[email protected]> commit b15f93e Author: Yury-Fridlyand <[email protected]> Date: Wed Sep 27 10:02:38 2023 -0700 Add missing renames. (#17) Signed-off-by: Yury-Fridlyand <[email protected]> commit 8664d05 Author: Andrew Carbonetto <[email protected]> Date: Tue Sep 26 15:27:11 2023 -0700 Rename jabushka to javababushka (#14) Signed-off-by: acarbonetto <[email protected]> commit e57c1ff Author: Yury-Fridlyand <[email protected]> Date: Mon Sep 25 17:25:41 2023 -0700 Add dataSize option to java benchmark. (#11) * Add Jedis and Lettuce benchmarks * Start ignoring .gradle files * Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> * Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> * Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> * Add benchmarks for GET non-existing * Revert "Update gitignore and remove generated files from git" This reverts commit d9b26a6. * fix redis-rs submodules Signed-off-by: acarbonetto <[email protected]> * Randomize commands in Java benchmarks * rename chooseAction to randomAction * Add a Java benchmarking app (#7) * Add a java app to run benchmarks --------- Signed-off-by: acarbonetto <[email protected]> * Add Readme and update install_and_test script to runJava Signed-off-by: acarbonetto <[email protected]> * Add Readme and update install_and_test script to runJava Signed-off-by: acarbonetto <[email protected]> * Combine java pipeline and java benchmarks (#8) * Merge Pull Request #5 - Add java pipeline. Also changed: * Merged two projects. * Updated CI. * Fixed tests and updated `junit` version. * Spotless. * Add new gradle tasks. Signed-off-by: Yury-Fridlyand <[email protected]> * Add sync and async clients both to tests. (#12) * Add sync and async clients both to tests. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor fixes. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> * Add dataSize option to java benchmark. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: acarbonetto <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Co-authored-by: Jonathan Louie <[email protected]> Co-authored-by: acarbonetto <[email protected]> Co-authored-by: jonathanl-bq <[email protected]> commit 016f5f6 Author: acarbonetto <[email protected]> Date: Mon Sep 25 15:10:47 2023 -0700 Move duplicated logic in benchmark JS scripts to a single file, and convert to TypeScript. (valkey-io#456) removed duplicated logic and refactored to typescript Signed-off-by: acarbonetto <[email protected]> commit 436da8f Author: Yury-Fridlyand <[email protected]> Date: Mon Sep 25 13:58:01 2023 -0700 Add sync and async clients both to tests. (#12) * Add sync and async clients both to tests. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor fixes. Signed-off-by: Yury-Fridlyand <[email protected]> --------- Signed-off-by: Yury-Fridlyand <[email protected]> commit d526f96 Author: Yury-Fridlyand <[email protected]> Date: Thu Sep 21 17:37:59 2023 -0700 Combine java pipeline and java benchmarks (#8) * Merge Pull Request #5 - Add java pipeline. Also changed: * Merged two projects. * Updated CI. * Fixed tests and updated `junit` version. * Spotless. * Add new gradle tasks. Signed-off-by: Yury-Fridlyand <[email protected]> commit 46d0cf6 Author: acarbonetto <[email protected]> Date: Wed Sep 20 00:42:14 2023 -0700 Add Readme and update install_and_test script to runJava Signed-off-by: acarbonetto <[email protected]> commit 6c1fb45 Author: acarbonetto <[email protected]> Date: Wed Sep 20 00:40:10 2023 -0700 Add Readme and update install_and_test script to runJava Signed-off-by: acarbonetto <[email protected]> commit 1983974 Author: Andrew Carbonetto <[email protected]> Date: Tue Sep 19 18:24:20 2023 -0700 Add a Java benchmarking app (#7) * Add a java app to run benchmarks --------- Signed-off-by: acarbonetto <[email protected]> commit 84f0efc Author: Jonathan Louie <[email protected]> Date: Thu Sep 14 16:09:11 2023 -0700 rename chooseAction to randomAction commit 52df672 Author: Jonathan Louie <[email protected]> Date: Wed Sep 13 16:04:03 2023 -0700 Randomize commands in Java benchmarks commit 5f51a5b Author: acarbonetto <[email protected]> Date: Fri Sep 8 15:47:00 2023 -0700 fix redis-rs submodules Signed-off-by: acarbonetto <[email protected]> commit cbb0dcb Author: acarbonetto <[email protected]> Date: Fri Sep 8 15:36:45 2023 -0700 Revert "Update gitignore and remove generated files from git" This reverts commit d9b26a6. commit 2a11e9a Author: Jonathan Louie <[email protected]> Date: Fri Sep 8 14:58:41 2023 -0700 Add benchmarks for GET non-existing commit e517744 Author: acarbonetto <[email protected]> Date: Fri Sep 8 13:45:34 2023 -0700 Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> commit 8203c4d Author: acarbonetto <[email protected]> Date: Fri Sep 8 13:30:53 2023 -0700 Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> commit 6ae93f5 Author: acarbonetto <[email protected]> Date: Tue Oct 3 13:35:07 2023 -0700 Update gitignore and remove generated files from git Signed-off-by: acarbonetto <[email protected]> commit 5990767 Author: Jonathan Louie <[email protected]> Date: Thu Sep 7 17:54:15 2023 -0700 Start ignoring .gradle files commit 06574ea Author: Jonathan Louie <[email protected]> Date: Wed Sep 6 20:35:12 2023 -0700 Add Jedis and Lettuce benchmarks Signed-off-by: Yury-Fridlyand <[email protected]>
TODOS
data_size
client
clientCount
num_of_tasks
is_cluster
tps
Output example
Example from rust benchmark