You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/08/05 13:58:10 UTC

[GitHub] [kafka] ijuma opened a new pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

ijuma opened a new pull request #9129:
URL: https://github.com/apache/kafka/pull/9129


   Highlights:
   
   * async-profiler integration. Can be used with -prof async,
   pass -prof async:help to look for the accepted options.
   * perf c2c [2] integration. Can be used with -prof perfc2c,
   if available.
   
   Full details: https://mail.openjdk.java.net/pipermail/jmh-dev/2020-August/002982.html
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540631610



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so
+
+With flame graph output (the semicolon is escaped to ensure it is not treated as a command separator):
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph
+
+A number of arguments can be passed to configure async profiler, run the following for a description:
+
+    ./jmh-benchmarks/jmh.sh -prof async:help
+
+### Using JMH GC profiler
+
+It's good practice to run your benchmark with `-prof gc` to measure its allocation rate:
+
+    ./jmh-benchmarks/jmh.sh -prof gc
+
+Of particular importance is the `norm` alloc rates, which measure the allocations per operation rather than allocations
+per second which can increase when you have make your code faster.
+
+### Running JMH outside of gradle
+
+The JMH benchmarks can be run outside of gradle as you would with any executable jar file:
+
+    java -jar <kafka-repo-dir>/jmh-benchmarks/build/libs/kafka-jmh-benchmarks-all.jar -f2 LRUCacheBenchmark
+
+### Writing benchmarks

Review comment:
       I will do this in a separate PR since it's a bit unrelated to the original goal of this PR and it may take a few iterations to get right.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540182407



##########
File path: gradle/spotbugs-exclude.xml
##########
@@ -237,19 +237,8 @@ For a detailed description of spotbugs bug categories, see https://spotbugs.read
     </Match>
 
     <Match>
-        <!-- Suppress some minor warnings about machine-generated code for
-             benchmarking. -->
-        <Or>
-            <Package name="org.apache.kafka.jmh.cache.generated"/>
-            <Package name="org.apache.kafka.jmh.common.generated"/>
-            <Package name="org.apache.kafka.jmh.record.generated"/>
-            <Package name="org.apache.kafka.jmh.partition.generated"/>
-            <Package name="org.apache.kafka.jmh.producer.generated"/>
-            <Package name="org.apache.kafka.jmh.fetchsession.generated"/>
-            <Package name="org.apache.kafka.jmh.fetcher.generated"/>
-            <Package name="org.apache.kafka.jmh.server.generated"/>
-            <Package name="org.apache.kafka.jmh.consumer.generated"/>
-        </Or>
+        <!-- Suppress some minor warnings about machine-generated code for benchmarking. -->
+        <Package name="~org\.apache\.kafka\.jmh\..*\.jmh_generated"/>

Review comment:
       jmh now uses `jmh_generated` instead of `generated`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma merged pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #9129:
URL: https://github.com/apache/kafka/pull/9129


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-706744365


   @ijuma we should bump this to 1.26 as it has some relevant bug fixes, e.g. https://github.com/openjdk/jmh/commit/2398e8e8b5518ea1229ba254792a4bd9df428ec0


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-671832339


   @omkreddy this is ready for review.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r468661509



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       I'll provide some text for this soon.

##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       While we're at it we can also mention `-prof gc` and how you should look for the norm allocation rate (since we care about garbage generated per operation rather than per second).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540373445



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so
+
+With flame graph output (the semicolon is escaped to ensure it is not treated as a command separator):
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph
+
+A number of arguments can be passed to configure async profiler, run the following for a description:
+
+    ./jmh-benchmarks/jmh.sh -prof async:help
+
+### Using JMH GC profiler
+
+It's good practice to run your benchmark with `-prof gc` to measure its allocation rate:
+
+    ./jmh-benchmarks/jmh.sh -prof gc
+
+Of particular importance is the `norm` alloc rates, which measure the allocations per operation rather than allocations
+per second which can increase when you have make your code faster.
+
+### Running JMH outside of gradle
+
+The JMH benchmarks can be run outside of gradle as you would with any executable jar file:
+
+    java -jar <kafka-repo-dir>/jmh-benchmarks/build/libs/kafka-jmh-benchmarks-all.jar -f2 LRUCacheBenchmark
+
+### Writing benchmarks

Review comment:
       Good idea. We should also link from the contributor docs.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478639981



##########
File path: jmh-benchmarks/README.md
##########
@@ -34,7 +34,18 @@ the jmh.sh script from the jmh-benchmarks module.
 * By default all JMH output goes to stdout.  To run a benchmark and capture the results in a file:
 `./jmh.sh -f 2 -o benchmarkResults.txt LRUCacheBenchmark`
 NOTE: For now this script needs to be run from the jmh-benchmarks directory.
+
+### Using JMH with async-profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
  
+    LD_LIBRARY_PATH=/path/to/async-profiler ./jmh-benchmarks/jmh.sh -prof async
+    
+A number of arguments can be passed to async-profiler, run the following for a description: 
+
+    ./jmh-benchmarks/jmh.sh -prof async:help
+

Review comment:
       Will add.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-742532333


   @chia7712 Pushed a couple of follow ups. I think this is ready to merge.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] chia7712 commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r539901142



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof 'async:libPath=/path/to/async-profiler.so;output=flamegraph' LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/async-profiler.so

Review comment:
       The file name of async-profiler is ```libasyncProfiler.so```. Should we follow the naming in this example?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-669221375


   Actually, it looks like I need to improve the jmh script to make this easier to use. Will also update the README. So, please don't review it yet.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478640519



##########
File path: jmh-benchmarks/README.md
##########
@@ -34,7 +34,18 @@ the jmh.sh script from the jmh-benchmarks module.
 * By default all JMH output goes to stdout.  To run a benchmark and capture the results in a file:
 `./jmh.sh -f 2 -o benchmarkResults.txt LRUCacheBenchmark`
 NOTE: For now this script needs to be run from the jmh-benchmarks directory.
+
+### Using JMH with async-profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
  
+    LD_LIBRARY_PATH=/path/to/async-profiler ./jmh-benchmarks/jmh.sh -prof async

Review comment:
       Agreed, I tested this variant on Mac, but not Linux yet. Will do before changing.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-678660654


   @ijuma the text mode output here looks good however it would be good to include instructions for producing an svg flamegraph if possible. Do you know if we can do that?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r468658313



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###
+We use [JMH](https://openjdk.java.net/projects/code-tools/jmh/) to write microbenchmarks that produce reliable results in the JVM.
+You can run all the benchmarks using:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks, for example:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.

Review comment:
       Rather than "in order to verify that they are valid", how about "in order to verify that they are measuring what you think they are. For example, if you use mocks they may use reflection that can easily dominate the benchmark's run time.".




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478636719



##########
File path: jmh-benchmarks/README.md
##########
@@ -34,7 +34,18 @@ the jmh.sh script from the jmh-benchmarks module.
 * By default all JMH output goes to stdout.  To run a benchmark and capture the results in a file:
 `./jmh.sh -f 2 -o benchmarkResults.txt LRUCacheBenchmark`
 NOTE: For now this script needs to be run from the jmh-benchmarks directory.
+
+### Using JMH with async-profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
  
+    LD_LIBRARY_PATH=/path/to/async-profiler ./jmh-benchmarks/jmh.sh -prof async

Review comment:
       We should document this variant instead so linux and mac os users are covered.
   ```
    -prof async:libPath=/Users/lucas/Downloads/async-profiler-1.8-macos-x64/build/libasyncProfiler.so
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#issuecomment-742264273


   @lbradstreet @chia7712 @omkreddy Please take a look when you have a chance.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r539871442



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       Updated.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478639377



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       Yeah, I fixed this in my local copy, but I wanted to test it before pushing the changes.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540361683



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so
+
+With flame graph output (the semicolon is escaped to ensure it is not treated as a command separator):
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph
+
+A number of arguments can be passed to configure async profiler, run the following for a description:
+
+    ./jmh-benchmarks/jmh.sh -prof async:help
+
+### Using JMH GC profiler
+
+It's good practice to run your benchmark with `-prof gc` to measure its allocation rate:
+
+    ./jmh-benchmarks/jmh.sh -prof gc
+
+Of particular importance is the `norm` alloc rates, which measure the allocations per operation rather than allocations
+per second which can increase when you have make your code faster.
+
+### Running JMH outside of gradle
+
+The JMH benchmarks can be run outside of gradle as you would with any executable jar file:
+
+    java -jar <kafka-repo-dir>/jmh-benchmarks/build/libs/kafka-jmh-benchmarks-all.jar -f2 LRUCacheBenchmark
+
+### Writing benchmarks

Review comment:
       Could we include a short section here about what should be put into a PR that has been benchmarked?
   
   I'm thinking:
   1. Benchmark comparisons for the code before and after the change.
   1. `-prof gc` results.
   2. An example async profile from at least one run.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540360441



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.

Review comment:
       How about:
   
   "It's good practice to check profiler output for microbenchmarks in order to verify that they represent application behavior and measure what you expect to measure. Some example pitfalls include the use of expensive mocks or accidental inclusion of test setup code in the benchmarked code."




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478635770



##########
File path: jmh-benchmarks/README.md
##########
@@ -34,7 +34,18 @@ the jmh.sh script from the jmh-benchmarks module.
 * By default all JMH output goes to stdout.  To run a benchmark and capture the results in a file:
 `./jmh.sh -f 2 -o benchmarkResults.txt LRUCacheBenchmark`
 NOTE: For now this script needs to be run from the jmh-benchmarks directory.
+
+### Using JMH with async-profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
  
+    LD_LIBRARY_PATH=/path/to/async-profiler ./jmh-benchmarks/jmh.sh -prof async
+    
+A number of arguments can be passed to async-profiler, run the following for a description: 
+
+    ./jmh-benchmarks/jmh.sh -prof async:help
+

Review comment:
       Suggested addition:
   
   ### Using JMH GC profiler
   
   It's good practice to run your benchmark with -prof gc to measure the allocation rate for your code:
   
         ./jmh-benchmarks/jmh.sh -prof gc
   
   Of particular importance is the "norm" alloc rates, which measure the allocations per operation rather than allocations per second which can increase when you have made your code faster.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r468895227



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       Sure. Do you think I should have only the link to jmh-benchmarks/README.md and have all the information on to use it in that page?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.25 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r478637097



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       Let's drop this and include that link before merge.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r540372753



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof async:libPath=/path/to/libasyncProfiler.so\;output=flamegraph LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.

Review comment:
       Will add.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] lbradstreet commented on a change in pull request #9129: MINOR: Update jmh to 1.24 for async profiler support

Posted by GitBox <gi...@apache.org>.
lbradstreet commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r475106000



##########
File path: README.md
##########
@@ -199,6 +199,27 @@ You can run spotbugs using:
 The spotbugs warnings will be found in `reports/spotbugs/main.html` and `reports/spotbugs/test.html` files in the subproject build
 directories.  Use -PxmlSpotBugsReport=true to generate an XML report instead of an HTML one.
 
+### JMH microbenchmarks ###

Review comment:
       Including only the link sounds good.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #9129: MINOR: Update jmh to 1.27 for async profiler support

Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #9129:
URL: https://github.com/apache/kafka/pull/9129#discussion_r539918580



##########
File path: jmh-benchmarks/README.md
##########
@@ -1,10 +1,69 @@
-### JMH-Benchmark module
+### JMH-Benchmarks module
 
 This module contains benchmarks written using [JMH](https://openjdk.java.net/projects/code-tools/jmh/) from OpenJDK.
 Writing correct micro-benchmarks in Java (or another JVM language) is difficult and there are many non-obvious pitfalls (many
 due to compiler optimizations). JMH is a framework for running and analyzing benchmarks (micro or macro) written in Java (or
 another JVM language).
 
+### Running benchmarks
+
+If you want to set specific JMH flags or only run certain benchmarks, passing arguments via
+gradle tasks is cumbersome. These are simplified by the provided `jmh.sh` script.
+
+The default behavior is to run all benchmarks:
+
+    ./jmh-benchmarks/jmh.sh
+    
+Pass a pattern or name after the command to select the benchmarks:
+
+    ./jmh-benchmarks/jmh.sh LRUCacheBenchmark
+
+Check which benchmarks that match the provided pattern:
+
+    ./jmh-benchmarks/jmh.sh -l LRUCacheBenchmark
+
+Run a specific test and override the number of forks, iterations and warm-up iteration to `2`:
+
+    ./jmh-benchmarks/jmh.sh -f 2 -i 2 -wi 2 LRUCacheBenchmark
+
+Run a specific test with async and GC profilers on Linux and flame graph output:
+
+    ./jmh-benchmarks/jmh.sh -prof gc -prof 'async:libPath=/path/to/async-profiler.so;output=flamegraph' LRUCacheBenchmark
+
+The following sections cover async profiler and GC profilers in more detail.
+
+### Using JMH with async profiler
+
+It's good practice to check profiler output for microbenchmarks in order to verify that they are valid.
+JMH includes [async-profiler](https://github.com/jvm-profiling-tools/async-profiler) integration that makes this easy:
+
+    ./jmh-benchmarks/jmh.sh -prof async:libPath=/path/to/async-profiler.so

Review comment:
       Yeah, that makes sense. Will do when I'm near a computer.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org