On Mon, 26 Oct 2020 13:37:58 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> @PaulSandoz I've implemented your suggestion, by moving the `exact` flag to
>> VarHandle itself. FWIW, the VH::accessModeType method took an AccessMode
>> value as an argument, and the AccessDescriptor only stored the ordinal, so I
>> added an `@Stable` array of values to AccessMode to map from ordinal to enum
>> value. But, maybe it makes more sense to just store the AccessMode in the
>> AccessDescriptor directly? (instead of the ordinal). Not sure what the
>> original motivation was for only storing the ordinal.
>>
>> I've also sharpened the tests to check the exception message. Do you think
>> the testing is sufficient? (Note that I did not add tests to the template
>> files since only a select set of argument type conversions causes the WMTE
>> we're looking for. So, that's why I created a new test file).
>>
>> FWIW, there seems to have been a bug in the implementation of
>> IndirectVarHandle::accessModeTypeUncached, where it was using the
>> VarHandle's type as the receiver argument (unlike all the other impls). I've
>> fixed this by passing `null` instead, and also removed a workaround for this
>> problem in VarHandles::maybeAdapt.
>
> I've updated the javadoc, and added two benchmarks that show the existing
> discrepancy between an exact and a generic use of a VarHandle, as well as
> showing that an exact VarHandle is as fast as a generic VarHandle for an
> exact invocation. (1 benchmark for normal Java field access, and 1 benchmark
> for the foreign memory-access API).
>
> Benchmark Mode
> Cnt Score Error Units
> o.o.b.java.lang.invoke.VarHandleExact.exact_exactInvocation avgt
> 30 0.729 □ 0.010 ns/op
> o.o.b.java.lang.invoke.VarHandleExact.generic_exactInvocation avgt
> 30 0.735 □ 0.019 ns/op
> o.o.b.java.lang.invoke.VarHandleExact.generic_genericInvocation avgt
> 30 14.696 □ 0.498 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.exact_exactInvocation avgt
> 30 2.274 □ 0.012 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_exactInvocation avgt
> 30 2.278 □ 0.015 ns/op
> o.o.b.jdk.incubator.foreign.VarHandleExact.generic_genericInvocation avgt
> 30 24.759 □ 0.367 ns/op
The direct use of the enum ordinal is because HotSpot accessing it from the
enum value is (or was) not optimal in C2.
You can avoid the addition of the stable array by doing the following:
public final MethodType accessModeType(AccessMode accessMode) {
return accessModeType(accessMode.at.ordinal());
}
@ForceInline
final MethodType accessModeType(int accessModeOrdinal) {
TypesAndInvokers tis = getTypesAndInvokers();
MethodType mt = tis.methodType_table[accessModeOrdinal];
if (mt == null) {
mt = tis.methodType_table[accessModeOrdinal] =
accessModeTypeUncached(accessModeOrdinal);
}
return mt;
}
final MethodType accessModeTypeUncached(int accessModeOrdinal) {
return accessModeTypeUncached(AccessMode.values()[accessModeOrdinal]);
}
It's a little odd going back and forth between the ordinal and the enum type,
but it's all on the slow uncached path, and it avoids some duplication of code.
I'll take a closer looks at the other areas and respond in another comment.
-------------
PR: https://git.openjdk.java.net/jdk/pull/843
