On Tue, 6 Oct 2026 09:03:16 GMT, Yasumasa Suenaga <[email protected]> wrote:
>> Valhalla introduced new array type which hosts flattened value object,
>> however SA cannot inspect it.
>> Actually `FlatArray.iterateFields()` has following lines:
>>
>>
>> for (int index = 0; index < length; index++) {
>> // FIXME - call visitor.doXXX() for each component of each value object
>> }
>>
>>
>> This PR addressed it.
>>
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Yasumasa Suenaga has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Fix
This looks good in general. I've posted one question where I have some doubts.
I'm still reading the PR as it is not easy to be sure in all technical details
in this update.
src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/FlatArray.java line 37:
> 35: // A FlatArray is an array containing flattened value objects.
> 36:
> 37: public class FlatArray extends ObjArray {
Q: Changing the inherited class from `Array` to `ObjArray` is suspisious.
The following methods may need overriding: `isObjArray()`, `getOopHandleAt()`,
and `getObjAt()`.
Also, you may need to check `instanceof ObjArray` use-sites to correctly handle
`FlatArray` cases:
public Object readObject(Oop oop) throws ClassNotFoundException {
. . .
} else if (oop instanceof ObjArray){
return readObjectArray((ObjArray)oop);
} else {
return null;
}
}
. . .
public boolean doObj(Oop oop) {
try {
writeHeapRecordPrologue(calculateOopDumpRecordSize(oop));
if (oop instanceof TypeArray) {
writePrimitiveArray((TypeArray)oop);
} else if (oop instanceof ObjArray) {
Klass klass = oop.getKlass();
ObjArrayKlass oak = (ObjArrayKlass) klass;
Klass bottomType = oak.getBottomKlass();
if (bottomType instanceof InstanceKlass ||
bottomType instanceof TypeArrayKlass) {
writeObjectArray((ObjArray)oop);
} else {
writeInternalObject(oop);
}
. . .
if (oop instanceof Instance || oop instanceof TypeArray) {
return true;
} else if (oop instanceof ObjArray) {
ObjArrayKlass oak = (ObjArrayKlass) oop.getKlass();
Klass bottomKlass = oak.getBottomKlass();
return bottomKlass instanceof InstanceKlass ||
bottomKlass instanceof TypeArrayKlass;
} else {
return false;
}
}
. . .
protected int calculateOopDumpRecordSize(Oop oop) throws IOException {
if (oop instanceof TypeArray taOop) {
return calculatePrimitiveArrayDumpRecordSize(taOop);
} else if (oop instanceof ObjArray oaOop) {
Klass klass = oop.getKlass();
ObjArrayKlass oak = (ObjArrayKlass) klass;
Klass bottomType = oak.getBottomKlass();
if (bottomType instanceof InstanceKlass ||
bottomType instanceof TypeArrayKlass) {
return calculateObjectArrayDumpRecordSize(oaOop);
} else {
// Internal object, nothing to write.
return 0;
}
. . .
-------------
PR Review: https://git.openjdk.org/jdk/pull/32849#pullrequestreview-5453696227
PR Review Comment: https://git.openjdk.org/jdk/pull/32849#discussion_r4216593554
