On Tue, 9 Mar 2021 13:46:04 GMT, Jaikiran Pai <[email protected]> wrote:
> An alternate approach that I thought of was to completely get rid of this
> shared cache canonicalMap and instead just use method level map (that gets
> initialized each time) in the tryCanonicalize method (thus requiring no
> locks/synchronization). I ran a JMH benchmark with the current change
> proposed in this PR and with the alternate approach of using the method level
> map. The performance number from that run showed that the approach of using
> the method level map has relatively big impact on performance (even when
> there's a cache hit in that method). So I decided to not pursue that
> approach. I'll include the benchmark code and the numbers in a comment in
> this PR, for reference.
The benchmark code:
package org.myapp;
import org.openjdk.jmh.annotations.*;
import java.lang.constant.*;
import java.util.concurrent.TimeUnit;
public class MyBenchmark {
enum MyEnum {A, B}
@Benchmark
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Threads(10)
@Fork(3)
public void dynamicConstantDesc_ofCanonical() {
final ConstantDesc desc =
DynamicConstantDesc.ofCanonical(ConstantDescs.BSM_ENUM_CONSTANT, "A",
ClassDesc.of("org.myapp.MyBenchmark").nested("MyEnum"),
new ConstantDesc[0]);
}
}
Benchmark results:
Current proposed patch in this PR:
Benchmark Mode Cnt Score Error
Units
MyBenchmark.dynamicConstantDesc_ofCanonical avgt 15 2295.714 ± 228.951
ns/op
Alternate approach of _not_ using the `canonicalMap` and instead using method
level map:
Benchmark Mode Cnt Score Error
Units
MyBenchmark.dynamicConstantDesc_ofCanonical avgt 15 4220.446 ± 831.905
ns/op
-------------
PR: https://git.openjdk.java.net/jdk/pull/2893
