RFR: 8366024: Remove unnecessary InstanceKlass::cast()
Ioi Lam
iklam at openjdk.org
Mon Sep 1 04:06:49 UTC 2025
On Wed, 27 Aug 2025 09:17:31 GMT, Andrew Dinn <adinn at openjdk.org> wrote:
>> We have a lot of `InstanceKlass::cast(k)` calls where `k` is statically known to be an `InstanceKlass`. I fixed many instances of this pattern:
>>
>>
>> InstanceKlass* x = ....;
>> Klass* s = x->super(); // should call java_super()
>> InstanceKlass::cast(s)->xyz();
>>
>>
>> The `super()` method has a very confusing API. It has the return type of `Klass*` because for for an `ObjArrayKlass` like `[Ljava/lang/String;`:
>>
>> - `super()` returns `[Ljava/lang/Object;`
>> - `java_super()` returns `Ljava/lang/Object;`
>>
>> However, for `[Ljava/lang/Object;`, all `TypeArrayKlasses` and all `InstanceKlasses`, `super()` and `java_super()` return an identical value of that always have the actual type of `InstanceKlass*`.
>>
>> See here about the difference between `super()` and `java_super()`: https://github.com/openjdk/jdk/blob/7b9969dc8f20989497ff617abb45543d182b684d/src/hotspot/share/oops/klass.hpp#L218-L221
>>
>> Unfortunately, we have a lot of code that incorrectly uses `super()` instead of `java_super()`, which leads to ` InstanceKlass::cast()` calls. I tried to fixed a bunch of easy ones in this PR, although there are a few more to go.
>>
>> I also fixed some calls to `local_interafaces()->at()` that widens the return type for `InstanceKlass*` to `Klass*`, which may lead to unnecessary ` InstanceKlass::cast()` calls.
>>
>> I also removed the `Klass::superklass()` API. This was used only in a few places and all of them can be safely replaced with `Klass::java_super()`.
>>
>> To avoid confusion, I think we should rename `super()` to something more obvious, but let's do that in a future PR.
>
> I found two more occurrences of casting super() to InstanceKlass:
>
> src/hotspot/share/jfr/leakprofiler/chains/edgeUtils.cpp:79
>
> ik = (const InstanceKlass*)ik->super();
>
> src/hotspot/share/prims/jni.cpp:209
>
> Klass* field_klass = k;
> Klass* super_klass = field_klass->super();
> // With compressed oops the most super class with nonstatic fields would
> // be the owner of fields embedded in the header.
> while (InstanceKlass::cast(super_klass)->has_nonstatic_fields() &&
> InstanceKlass::cast(super_klass)->contains_field_offset(offset)) {
> field_klass = super_klass; // super contains the field also
> super_klass = field_klass->super();
> }
>
> The first one ought perhaps to be using InstanceKlass::superklass()?
Thanks @adinn @dholmes-ora @coleenp for the review
-------------
PR Comment: https://git.openjdk.org/jdk/pull/26908#issuecomment-3240791556
More information about the hotspot-dev
mailing list