On Fri, 7 Jul 2023 08:29:06 GMT, Hamlin Li <m...@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.

@Hamlin-Li 
The PR is fully correct.
Don't forget, every Java instance method has a specific argument which called 
"this".  That is why @State annotation is working.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14746#issuecomment-1625627033

Reply via email to