RFR: 8285364: Remove REF_ enum for java.lang.ref.Reference [v4]
Kim Barrett
kbarrett at openjdk.java.net
Thu Apr 28 11:17:44 UTC 2022
On Thu, 28 Apr 2022 09:15:33 GMT, Albert Mingkun Yang <ayang at 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.yang at 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
More information about the serviceability-dev
mailing list