RFR: 8283574: Use Klass::_id for type checks in the C++ code

[Kim Barrett]

On Wed, 23 Mar 2022 14:42:06 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> We have at least three ways to check the type of a given Klass*:
> 1) Using the Klass::_id field
> 2) Using the layout helper
> 3) Using the InstanceKlass::_kind field
> 
> The Klass::_id field was something that was added when we rewrote the oop_oop_iterate dispatch mechanism, but the other mechanisms where left in place.
> 
> The current Loom code uses both (2) and (3) every time a the code checks if an object is of type InstanceStackChunkKlass. In the Loom repository I intend to reduce that check to be a single test against the (1) field. To keep the code unified, and simpler, I changed all C++ Klass type checks to use (1).
> 
> I propose that we upstream this change to the mainline, to slightly reduce the Loom diff.

Mostly looks good, subject to later nomenclature change as suggested by @rose00 .  A couple comments that you can act on or not.

src/hotspot/share/oops/instanceKlass.hpp line 139:

> 137: 
> 138:  protected:
> 139:   InstanceKlass(const ClassFileParser& parser, KlassID id = ID);

I think I would prefer that KlassID was required.  I don't know how much fanout that might have though.  That preference is despite making construction of a concrete InstanceKlass different from the others.  That's an artifact of InstanceKlass being overloaded as both a base class and a leaf class, a pattern that seems to often lead to trouble.

src/hotspot/share/oops/oop.cpp line 141:

> 139: bool oopDesc::is_array_noinline()       const { return is_array();       }
> 140: bool oopDesc::is_objArray_noinline()    const { return is_objArray();    }
> 141: bool oopDesc::is_typeArray_noinline()   const { return is_typeArray();   }

Is there a reason for this change that I'm not spotting?  Oh, yes, there is.  Buried in the whitespace formatting changes is the addition of `is_instanceRef_noinline`.  Fortunately, I found github's "turn off whitespace differences" button.  But I really dislike this kind of formatting, and esp. reformatting when combined with other changes.  I think the formatting should have been left alone here and in similar places elsewhere.

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

Marked as reviewed by kbarrett (Reviewer).

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