On Fri, 13 Feb 2026 08:26:34 GMT, Leonid Mesnik <[email protected]> wrote:
>> Please review following fix that add Byte[], Boolean[] and Integer[] arrays
>> to garbageProducers and use them in random(arrays) producers and added few
>> configurations to ArrayJuggle3 using these new producers.
>>
>> Also change some memory objects from primitive arrays to Integer/Byte arrays
>> if preview is enabled to stress flattened arrays.
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fixed gp name
Thank you for this work, it'd be great to have more robust value class support!
I've left some comments from the perspective of a VM developer. I can't comment
on how appropriate these changes are for the vmTestbase codebase, but the
changes seem reasonable to me.
test/hotspot/jtreg/vmTestbase/gc/ArrayJuggle/Juggle3.java line 27:
> 25: * VM Testbase keywords: [gc, stress, stressopt, nonconcurrent]
> 26: */
> 27: /* @test id=BooleanObjArr_medium @key stress randomness @library
> /vmTestbase /test/lib @run main/othervm -XX:+HeapDumpOnOutOfMemoryError
> -Xlog:gc=debug:gc.log gc.ArrayJuggle.Juggle3 -gp BooleanObjArr -ms
> medium */
Do we have the resources to run these boxed arrays when not in preview mode? If
not, we might want to `@require java.enablePreview` and `@require
!java.enablePreview`.
test/hotspot/jtreg/vmTestbase/nsk/share/gc/Memory.java line 40:
> 38: private static int objectExtraSize = 0;
> 39:
> 40: private static final boolean previewEnabled =
I think there are some more robust ways to check for preview.
* The best way is probably `jdk.internal.misc.PreviewFeatures.isEnabled` which
checks if the current VM is preview-enabled. Needs `@modules
java.base/jdk.internal.misc`.
* Alternatively, if the above is not possible,
`jdk.test.lib.Utils.getTestJavaOpts` which you can check in a Stream for
`--enable-preview`. This consolidates all of the `test.vm.opts` and
`test.java.opts` for you.
test/hotspot/jtreg/vmTestbase/nsk/share/gc/Memory.java line 139:
> 137: return getReferenceSize();
> 138: }
> 139: return 8;
This comment applies to all `getXXXArrayElementSize` methods. The element size
depends on many things, such as the properties of the array (controls
flattening decisions and/or layout) and the VM (flattening can be disabled).
For JEP 401, we only consider the nullable atomic layout. Even for this simple
case, we should, in my opinion, do the following:
- Check if flattening is enabled. For this, we can use the `WhiteBox` API to
get the VM flag values. Some relevant flags may be: `UseArrayFlattening`,
`UseFieldFlattening`, `UseNullableValueFlattening`, `UseAtomicValueFlattening`.
- File an RFE to track hardcoding the size, such that we can change it once
there are more layouts available.
A more robust solution would be to use
`test/hotspot/jtreg/runtime/valhalla/inlinetypes/field_layout/FieldLayoutAnalyzer.java`
in order to get more comprehensive layout information.
test/hotspot/jtreg/vmTestbase/nsk/share/gc/Memory.java line 208:
> 206: int length = (int) Math.min((memory - arrayExtraSize) /
> objectSize,
> 207: Integer.MAX_VALUE);
> 208: return Math.max(length, 0);
In which scenarios do you anticipate to get a negative `length`?
test/hotspot/jtreg/vmTestbase/nsk/share/gc/MemoryObject.java line 49:
> 47: }
> 48: } else {
> 49: storage = new int[0];
This else case wasn't here before right? I'm not sure I follow why we need an
empty array.
test/hotspot/jtreg/vmTestbase/nsk/share/gc/gp/GarbageUtils.java line 402:
> 400: return new BooleanObjArrayProducer();
> 401: else if (id.equals("IntegerObjArr"))
> 402: return new IntegerObjArrayProducer();
Sanity question: do we also need to consider `h_IntegerObjArr`?
-------------
PR Review:
https://git.openjdk.org/valhalla/pull/2099#pullrequestreview-3797580365
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804490843
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804610132
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804658453
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804661188
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804664777
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2099#discussion_r2804548441
