On Wed, 19 Jul 2023 00:52:49 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Mandy Chung has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> review feedback
>
> src/java.base/share/classes/java/lang/invoke/IndirectVarHandle.java line 93:
>
>> 91: @ForceInline
>> 92: boolean checkAccessModeThenIsDirect(VarHandle.AccessDescriptor ad) {
>> 93: if (exact && accessModeType(ad.type) !=
>> ad.symbolicMethodTypeExact) {
>
> Can we add a comment that the detailed UOE is thrown via
> `directTarget.getMethodHandleUncached`?
Actually `VarHandle::checkAccessModeThenIsDirect` can call
`isAccessModeSupported` so that this will check properly for
`IndirectVarHandle`.
> src/java.base/share/classes/java/lang/invoke/VarHandle.java line 2020:
>
>> 2018:
>> 2019: static AccessMode valueFromOrdinal(int mode) {
>> 2020: return modeToAccessMode.get(mode);
>
> Can't this be simplified to `AccessMode.values()[mode]` since this is only
> rarely used in the exception logic?
>
> If we do need a cache, I would recommend a stable array like
>
> private static final @Stable AccessMode[] VALUES = values();
>
> and users call this accessor instead.
>
> The code in `getMethodHandleUncached` can benefit from this caching mechanism.
Good suggestion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14928#discussion_r1267459610
PR Review Comment: https://git.openjdk.org/jdk/pull/14928#discussion_r1267459744
