On Thu, 13 Aug 2026 01:31:49 GMT, Yasumasa Suenaga <[email protected]> wrote:
>> 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.
I refactored this change to make it simpler. The new commit will be easier to
understand (for maintenance POV).
>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopField.java line
>> 64:
>>
>>> 62: public long getOffset() {
>>> 63: long ofs = super.getOffset();
>>> 64: if ((inlineKlass != null) || (inlineKlass == null && isFlat())) {
>>
>> Can you explain a bit how this works. I thought an InstanceKlass that is
>> inlineable is marked as such, but that doesn't imply it will actually be
>> inlined (flattened) for every field reference. The field referencing the
>> inlineable class is marked as "flat" if it is actually flattened. How do we
>> end up with (inlineKlass == null && isFlat()) being true? I would think that
>> isFlat() being true would imply (inlineKlass != null) being true.
>
> I've added this condition for followup bugs (FlatArray).
>
> In FlatArray, I think we can traverse flattened array oop as following. We
> need to pass holder (element) Klass to `OopField`. In `Instance` (`oop`),
> `OopField` associates with the field owner, so `isFlat()` can work. OTOH
> `Inline` in `FlatArray` is just the value, and we need to instantiate
> `OopField` directly. It means we cannot access metadata - it is JDK-8247507.
>
>
> FlatArrayKlass klass = (FlatArrayKlass)getKlass();
> InlineKlass elementKlass = (InlineKlass)klass.getElementKlass();
>
> for (int index = 0; index < length; index++) {
> long offset = baseOffset + (index * elementSize);
> OopField field = new OopField(new IndexableFieldIdentifier(index),
> offset, false, elementKlass);
> visitor.doOop(field, false);
> }
>
>
> Hence we need to consider two conditions to detect vaule object:
>
> 1. `inlineKlass != null`: for FlatArray: `Klass` is injected in force.
> 2. `inlineKlass == null && isFlat()`: Flattened field in (normal) oop: we can
> access metadata via `isFlat()`.
I've done to make the condition simpler in new commit.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32310#discussion_r3775227331
PR Review Comment: https://git.openjdk.org/jdk/pull/32310#discussion_r3775226316
