On Wed, 12 Aug 2026 22:19:59 GMT, Chris Plummer <[email protected]> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Improve OopField::getOffset
>
> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/Inline.java line 64:
>
>> 62:
>> 63: public boolean isFlattened() {
>> 64: return klass != null;
>
> It's confusing how the setting of `klass` is the flag that determines if this
> represents an oop for a flattened field (I assume this will eventually extend
> to locals and array elements), or is just for a regular heap object.
>
> There is also the issue of class being inlineable vs actually being inlined.
> This class at first seems to mirror inlineOop, but in fact it does not since
> inlineOop only references actual heap objects. So this class is instead is an
> SA specific way of having and "oop" reference a flattened field. I think this
> needs to be made more clear.
>
> Perhaps this class shouldn't be used if the inlineable type is not flattened.
> newOop(OopHandle) can just create an `Instance` when the Klass is InlineKlass
> intead of an `Inline` with no klass. It doesn't seem that there is any place
> in SA where it needs to know if the Instance is an InlineKlass or just
> InstanceKlass. newOop(OopHandle, InlineKlass) can be used only when the field
> is flattened. If you go this route, you might want to rename this klass to
> FlattenedInstance.
>
> I'm ok if you want to keep it as-is, but the purpose of this class needs to
> better explained and probably given a different name to avoid the incorrect
> Inline == inlineOop assumption.
This code relates to following changes in `OopField`. It intends to FlatArray
(thus we can remove this so far).
if (inlineKlass == null) {
if (isFlat()) {
InlineKlass kls = (InlineKlass)getKlassFromSignature();
return heap.newOop(obj.getHandle().addOffsetToAsOopHandle(getOffset()),
kls);
} else {
return heap.newOop(getValueAsOopHandle(obj));
}
} else {
return heap.newOop(obj.getHandle().addOffsetToAsOopHandle(getOffset()),
inlineKlass);
}
As I sand in above, we need to set `InlineKlass` in force in FlatArray, thus I
used `klass != null` to know whether this Klass is inlined or not.
I agree with you to confuse this change, so I will update it. However I want to
know I can include FlatArray related code into this PR before making changes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32310#discussion_r3771680787
