Re: RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod…

David Holmes david.holmes at oracle.com
Thu Nov 5 22:29:34 UTC 2020


On 5/11/2020 7:09 pm, Aleksey Shipilev wrote:
> On Thu, 5 Nov 2020 02:52:05 GMT, Hui Shi <hshi at openjdk.org> wrote:
> 
>> …AccessorImpl object
>>
>> We met real problem when using protobuf with option optimized for code size, detail in JBS https://bugs.openjdk.java.net/browse/JDK-8255883
>>
>> Optimize solution is adding a new boolean field to detect concurrent method accessor generation in same NativeMethodAccessorImpl object, only one thread is allowed to generate accessor, other threads still invoke in jni way until parent's delegator is updated from NativeMethodAccessorImpl  to generated accessor.
>>
>> In common case, extra overhead is an atomic operation, compared with method accessor generate, this cost is trivial.
> 
> I do wonder if it makes sense to handle triple-state `int` here: "not yet generated", "generated", "in error"? So that we don't try to generate the accessor over and over again when it is in error?

You have no idea why generation failed so don't know whether a retry 
will succeed or not. Current code will retry so it would be a change in 
behaviour to declare it permanently "in error".

Cheers,
David

> src/java.base/share/classes/jdk/internal/reflect/NativeMethodAccessorImpl.java line 80:
> 
>> 78:                 succ = true;
>> 79:             } finally {
>> 80:                 if (succ == false) {
> 
> Why `succ` variable, if you can just `catch (Throwable e)` and restore the `accessorGenerated`?
> 
> src/java.base/share/classes/jdk/internal/reflect/NativeMethodAccessorImpl.java line 66:
> 
>> 64:                 && !method.getDeclaringClass().isHidden()
>> 65:                 && !ReflectUtil.isVMAnonymousClass(method.getDeclaringClass())
>> 66:                 && ACCESSOR_GENERATED.compareAndSet(this, false, true)) {
> 
> As the micro-optimization, checking that `accessor_generated` is `false` before attempting a (potentially contended) CAS might fit better. (See https://en.wikipedia.org/wiki/Test_and_test-and-set).
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/1070
> 


More information about the core-libs-dev mailing list