On Sat, 1 Jul 2023 07:53:17 GMT, Swati Sharma <d...@openjdk.org> wrote:
> The below benchmark files have scaling issues due to cache contention and > leads to poor scaling when run on multiple threads. The patch sets the scope > from benchmark level to thread level to fix the issue: > - org/openjdk/bench/java/io/DataOutputStreamTest.java > - org/openjdk/bench/java/lang/ArrayCopyObject.java > - org/openjdk/bench/java/lang/ArrayFiddle.java > - org/openjdk/bench/java/time/format/DateTimeFormatterBench.java > - org/openjdk/bench/jdk/incubator/vector/IndexInRangeBenchmark.java > - org/openjdk/bench/jdk/incubator/vector/MemorySegmentVectorAccess.java > - org/openjdk/bench/jdk/incubator/vector/StoreMaskedBenchmark.java > - org/openjdk/bench/jdk/incubator/vector/StoreMaskedIOOBEBenchmark.java > - org/openjdk/bench/vm/compiler/ArrayFill.java > - org/openjdk/bench/vm/compiler/IndexVector.java > > Also removing the static scope for variables in > org/openjdk/bench/jdk/incubator/vector/VectorFPtoIntCastOperations.java for > better scaling. > > Please review and share your feedback. > > Thanks, > Swati > Hi, I'm not sure if I understand this improvement correctly. I'm not quite > familiar with JMH and it's annotations, but seems to me, the change from > `@State(Scope.Benchmark)` to `@State(Scope.Thread)` should not improve the > performance by reducing cache contention, as in the jmh doc it says "State > objects are usually injected into Benchmark methods as _**arguments**_, and > JMH takes care of their instantiation and sharing.", this seems mean that > @State only matters when the annotated class is used as a parameter of a > @benchmark method, but in the tests you modifed, seems there is no such use > case. Please also check the sample usages at > https://github.com/openjdk/jmh/blob/master/jmh-samples/src/main/java/org/openjdk/jmh/samples/JMHSample_03_States.java. Please check the https://github.com/openjdk/jmh/blob/master/jmh-samples/src/main/java/org/openjdk/jmh/samples/JMHSample_04_DefaultState.java sample case where @State annotation applies to all instance methods. The PR proposal is to change default state from benchmark to thread. ------------- PR Comment: https://git.openjdk.org/jdk/pull/14746#issuecomment-1627181265
