On Tue, 10 Nov 2020 18:28:11 GMT, Aleksey Shipilev <[email protected]> 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
