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