Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix java min benchmarking. #105

Merged
merged 6 commits into from
Feb 27, 2024
Merged

Fix java min benchmarking. #105

merged 6 commits into from
Feb 27, 2024

Conversation

Yury-Fridlyand
Copy link

@Yury-Fridlyand Yury-Fridlyand commented Feb 21, 2024 •
edited
Loading

As discussed in valkey-io#969, java CI should run minimal benchmark on every PR.
Features:

$ ./gradlew :benchmarks:run --args="--help"

> Task :client:buildRustRelease
    Finished release [optimized + debuginfo] target(s) in 5.01s

> Task :benchmarks:run
usage: glide
    --clientCount <arg>       Number of clients to run [1]
    --clients <arg>           one of: all|jedis|lettuce|glide
    --clusterModeEnabled      Is cluster-mode enabled, other standalone
                              mode is used [false]
    --concurrentTasks <arg>   Number of concurrent tasks [100, 1000]
    --configuration <arg>     Configuration flag [Release]
    --dataSize <arg>          Data block size [100 4000]
    --debugLogging            Verbose logs [false]
 -h,--help                    Print this message
    --host <arg>              Hostname [localhost]
    --minimal                 Run benchmark in minimal mode
    --port <arg>              Port number [6379]
    --resultsFile <arg>       Result filepath (stdout if empty) []
    --tls                     TLS [false]

@Yury-Fridlyand
Fix java min benchmarking.
40fa462 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
typo fix.
7e57c60 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
Fix for macos.
3b6ab7b 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
typo fix.
bd0bd9b 
Signed-off-by: Yury-Fridlyand <[email protected]>
acarbonetto
acarbonetto approved these changes Feb 22, 2024
View reviewed changes
Copy link

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also clean up the dashes in the README:
https://github.com/Bit-Quill/glide-for-redis/blob/bd0bd9bf7c265b1fe1060527da1d1d29142ce7c7/java/README.md?plain=1#L109-L125

acarbonetto
acarbonetto reviewed Feb 22, 2024
View reviewed changes
Copy link

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why python/node jobs are failing?

@Yury-Fridlyand
Update readme.
7103d59 
Signed-off-by: Yury-Fridlyand <[email protected]>
@Yury-Fridlyand
Copy link
Author

They are failing on our repo only. We need to investigate this.
Thank you for reminding about readme. Updated in 7103d59.

@Yury-Fridlyand
fix
7daa4e6 
Signed-off-by: Yury-Fridlyand <[email protected]>
jonathanl-bq
jonathanl-bq reviewed Feb 22, 2024
View reviewed changes
java/benchmarks/src/main/java/glide/benchmarks/BenchmarkingApp.java
@@ -66,36 +66,66 @@ private static Options getOptions() {
// create the Options
Options options = new Options();

options.addOption(Option.builder("help").desc("print this message").build());
options.addOption(Option.builder("h").longOpt("help").desc("Print this message").build());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to update this message later? Or is it meant to just be a dummy message? If we want to update this later, let's make sure that it doesn't fall through the cracks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A dummy message

jonathanl-bq
jonathanl-bq approved these changes Feb 22, 2024
View reviewed changes
Copy link

@jonathanl-bq jonathanl-bq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left one comment, but otherwise seems fine to me

@Yury-Fridlyand Yury-Fridlyand merged commit 6c66608 into java/integ_yuryf_minimal_benchmark Feb 27, 2024
18 of 20 checks passed
@Yury-Fridlyand Yury-Fridlyand deleted the java/dev_yuryf_minimal_benchmark branch February 27, 2024 01:59
Yury-Fridlyand added a commit that referenced this pull request Feb 29, 2024
@Yury-Fridlyand
Fix java min benchmarking. (#105) (valkey-io#1046)
565eef7 
* Fix java min benchmarking.

---------

Signed-off-by: Yury-Fridlyand <[email protected]>
cyip10 pushed a commit that referenced this pull request Jun 24, 2024
@Yury-Fridlyand @cyip10
Fix java min benchmarking. (#105) (valkey-io#1046)
30ac58f 
* Fix java min benchmarking.

---------

Signed-off-by: Yury-Fridlyand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acarbonetto acarbonetto acarbonetto approved these changes

@jonathanl-bq jonathanl-bq jonathanl-bq approved these changes

@aaron-congo aaron-congo Awaiting requested review from aaron-congo

@Elen-Ghulam Elen-Ghulam Awaiting requested review from Elen-Ghulam

@SanHalacogluImproving SanHalacogluImproving Awaiting requested review from SanHalacogluImproving

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Yury-Fridlyand @jonathanl-bq @acarbonetto