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