On Fri, 12 Mar 2021 13:23:39 GMT, Peter Levart <[email protected]> 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