On Fri, 12 Mar 2021 13:23:39 GMT, Peter Levart <plev...@openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Follow Peter Levart's review suggestion - remove volatile
>
> This looks good to me. Maybe if you wanted the previous performance back 
> (when the `cannonicalMap` field was static final and therefore JIT could 
> constant-fold the Map instance), you could now annotate the field with 
> `@Stable` annotation to allow JIT to produce similar code. I would also move 
> the `Map.ofEntries(...)` expression into a separate private static method 
> since I believe it has substantial bytecode size. `tryCanonicalize()` method 
> bytecode size would then more likely fall below the inlining threshold.

What you have is probably fine, but has any consideration been given to using 
the initialization-on-demand holder idiom? e.g.

     static private class CanonicalMapHolder {
        static final Map<MethodHandleDesc, Function<DynamicConstantDesc<?>, 
ConstantDesc>> CANONICAL_MAP
              = Map.ofEntries(..);
     }

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

PR: https://git.openjdk.java.net/jdk/pull/2893

Reply via email to