On Fri, 27 Mar 2026 05:01:52 GMT, Chris Plummer <[email protected]> wrote:

>> Although SA has some FlatArrayKlass support, it is mostly incomplete. This 
>> is causing heap walking to abort when a flattened array of value objects is 
>> encountered. 4 SA tests are failing in non-obvious ways because of this.
>> 
>> [JDK-8380769](https://bugs.openjdk.org/browse/JDK-8380769) [lworld] 
>> resourcehogs/serviceability/sa/TestHeapDumpForLargeArray.java fails with 
>> 'truncating to' missing from stdout/stderr with --enable-preview
>> [JDK-8380851](https://bugs.openjdk.org/browse/JDK-8380851) [lworld] 
>> serviceability/sa/ClhsdbJstackWithConcurrentLock.java fails with "', (a 
>> java/util/concurrent/locks/ReentrantLock$NonfairSync)' not found in jstack 
>> output" when using --enable-preview
>> [JDK-8380852](https://bugs.openjdk.org/browse/JDK-8380852) [lworld] 
>> serviceability/sa/CDSJMapClstats.java fails with "'BootClassLoader' missing 
>> from stdout/stderr" with "-XX:-UseCompressedOops --enable-preview"
>> [JDK-8380853](https://bugs.openjdk.org/browse/JDK-8380853) [lworld] 
>> serviceability/sa/ClhsdbJhisto.java fails with "'java.nio.HeapByteBuffer' 
>> missing from stdout/stderr" with "-XX:-UseCompressedOops --enable-preview"
>> 
>> The main shortcoming is that there is no FlatArray type in SA yet, and also 
>> the code to instantiate it is missing in the following:
>> 
>> 
>>   public Oop newOop(OopHandle handle) {
>> ...
>>     Klass klass = Oop.getKlassForOopHandle(handle);
>>     if (klass != null) {
>>       if (klass instanceof TypeArrayKlass) return new TypeArray(handle, 
>> this);
>>  + if (klass instanceof FlatArrayKlass) return new FlatArray(handle, this);
>>       if (klass instanceof ObjArrayKlass) return new ObjArray(handle, this);
>>       if (klass instanceof InstanceKlass) return new Instance(handle, this);
>>     }
>>     throw new UnknownOopException(handle.toString());
>>   }
>> 
>> 
>> Without the added code, this method throws UnknownOopException when it 
>> encounters a flattened array. That in turn causes the heap walking to stop 
>> scanning the region it is in, thinking that it has reached the top of the 
>> used part of the region. See ObjectHeap.iterateLiveRegions(). 
>> 
>> Tested tier1, tier2 svc, and tier5 svc CI testing with and w/o 
>> --enable-preview. Did tier5, tier6, and tier7 with --enable-preview on 
>> linux-x64-debug only.
>
> Chris Plummer has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - fix comment typo
>  - get rid of unused getXXX() methods

Looks good.
I added a note about comment for the class, also `Array` class has
`// Array is an abstract superclass for TypeArray and ObjArray`
FlatArray can be added there, or the comment can be generalized, smth like 
'super class for different array classes'

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/FlatArray.java line 35:

> 33: import sun.jvm.hotspot.utilities.Observer;
> 34: 
> 35: // A FlatArray is an array containing flattened value types.

Suggestion:

// A FlatArray is an array containing flattened value objects.

-------------

Marked as reviewed by amenkov (Committer).

PR Review: 
https://git.openjdk.org/valhalla/pull/2271#pullrequestreview-4023328697
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2271#discussion_r3002901160

Reply via email to