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

John R Rose jrose at openjdk.java.net
Wed Mar 23 16:56:25 UTC 2022


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.

Consolidating two ad hoc tags, from kind+id to id, is good.  Using the value of id to gate the klass-subtype checks is good.

There's something bad here, which unfortunately came in before but it now getting greater prominence:  The word itself, "id".  That (a) conveys very little, leaving the user to guess a alot, and (b) the understandable guess will be wrong.  The "ID" of a class, if it is anything one might predict, is not its "kind" but rather its identity, as in `System.identityHashCode`.  This is the use of ID elsewhere in the source base, and raising the profile of Klass::id in this way creates conceptual conflicts.  For more regular uses of the term "ID" please see `vmSymbols.hpp`, `vmIntrinsics.hpp`, `vmClasses.hpp`.  In all of these cases, the thing we call an ID is an enum member which exactly identifies the identity of a particular well-known name.

I request that, as you remove `kind` (which was the more informative name, though it was less functional), you also rename `id`/`ID` to something along the lines of `kind_id`/`KindID` or even drop "id" and say just `klass_kind` or  even `kind` .  I realize this makes the change set larger, but this vague and misleading word "id" should not be getting more entrenched in this central API.

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

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


More information about the hotspot-dev mailing list