On 5/11/2020 7:09 pm, Aleksey Shipilev wrote:
On Thu, 5 Nov 2020 02:52:05 GMT, Hui Shi <h...@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
