On Thu, 28 Apr 2022 09:15:33 GMT, Albert Mingkun Yang <ay...@openjdk.org> wrote:
>> Simple rename and some comments update.
>>
>> Test: build
>
> Albert Mingkun Yang has updated the pull request incrementally with two
> additional commits since the last revision:
>
> - comment
> - Rework reference type initialization
>
> Signed-off-by: Albert Yang <albert.m.y...@oracle.com>
There are further cleanups possible if we didn't have REF_NONE and
InstanceKlass didn't have a reference type. But there are a number of uses of
InstanceKlass::reference_type. Doing anything along those lines (assuming it's
deemed worthwhile to do so) can be done in a followup RFE.
src/hotspot/share/classfile/classFileParser.cpp line 6098:
> 6096:
> 6097: return _super_klass->reference_type() != REF_NONE;
> 6098: }
Stylistically, I'd prefer an if-ladder here. I might also swap the
reference-type check and the name check, so the for-bootstrapping name check
case being last (with a comment to that effect).
src/hotspot/share/oops/instanceKlass.cpp line 497:
> 495: _nest_host_index(0),
> 496: _init_state(allocated),
> 497: _reference_type(REF_NONE),
This is initializing `_reference_type` to the wrong value for a
`InstanceRefKlass` object, which then needs to reset it in the derived
constructor. Why not get the reference type from the parser? The (currently
file-scoped static) determine_reference_type function in instanceRefKlass.cpp
doesn't have any dependency on the klass object being constructed, just the
parser.
src/hotspot/share/oops/instanceKlass.hpp line 580:
> 578: protected:
> 579: // Only used by the InstanceRefKlass constructor
> 580: void set_reference_type(ReferenceType t) {
This function wouldn't be needed at all if the reference type was properly
initialized.
src/hotspot/share/oops/instanceRefKlass.cpp line 55:
> 53: }
> 54:
> 55: // Bootstrapping: this is either of the four direct subclasses of
> java.lang.ref.Reference
s/either of the four/one of the/. In particular remove "four" else, someday,
finalization will finally be GC'd and this comment will need to be noticed and
updated.
-------------
Changes requested by kbarrett (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/8332
