RFR: 8255883: Avoid multiple GeneratedMethodAccessor for same NativeMethod… [v2]

[Hui Shi]

On Tue, 10 Nov 2020 18:28:11 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Hui Shi has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
>> 
>>   8255883: Avoid multiple GeneratedAccessor for same NativeMethod/ConstructorAccessorImpl object
>
> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java line 37:
> 
>> 35: 
>> 36: class NativeConstructorAccessorImpl extends ConstructorAccessorImpl {
>> 37:     private static final Unsafe unsafe = Unsafe.getUnsafe();
> 
> `static final` field identifiers should be capitalized. Suggestion: `private static final Unsafe U = Unsafe.getUnsafe()`.

fixed

> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java line 38:
> 
>> 36: class NativeConstructorAccessorImpl extends ConstructorAccessorImpl {
>> 37:     private static final Unsafe unsafe = Unsafe.getUnsafe();
>> 38:     private static final long accessorGeneratedaOffset
> 
> Typo "Generated*a*Offset". But also this is `static final`, so it should be e.g. `GENERATED_OFFSET`. The field can be just `generated` to match the name of this offset.

field name udpated

> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java line 61:
> 
>> 59:                 && !c.getDeclaringClass().isHidden()
>> 60:                 && !ReflectUtil.isVMAnonymousClass(c.getDeclaringClass())
>> 61:                 && accessorGenerated == false
> 
> Should be `!accessorGenerated`.

change to "generated == 0", as generated is int type now

> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java line 62:
> 
>> 60:                 && !ReflectUtil.isVMAnonymousClass(c.getDeclaringClass())
>> 61:                 && accessorGenerated == false
>> 62:                 && unsafe.compareAndSetBoolean(this, accessorGeneratedaOffset, false, true)) {
> 
> `compareAndSetBoolean` gets us into the awkward territory of sub-word CASes. (Click through to `Unsafe.compareAndExchangeByte` to see what I am talking about). It is probably not relevant here, though. Still, might be as good to use `int` field for generated flag.

use int type now

> src/java.base/share/classes/jdk/internal/reflect/NativeConstructorAccessorImpl.java line 72:
> 
>> 70:                 parent.setDelegate(acc);
>> 71:             } catch (Throwable t) {
>> 72:                 // in case Thowable happens in generateMethod
> 
> Typo: `Thowable`.

updated

-------------

PR: https://git.openjdk.java.net/jdk/pull/1070