On Thu, 5 Nov 2020 02:52:05 GMT, Hui Shi <[email protected]> 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?
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
