On Thu, 2 Apr 2026 22:59:07 GMT, Leonid Mesnik <[email protected]> wrote:

>> The GCbasher si a GC stress test that create a lot of different objects and 
>> arrays.
>> 
>> It would be makes sense to update it to create a lot of different value 
>> classes and arrays of value types. It helps to ensure that GC works with 
>> different type of classes.
>> 
>> Please, note that after this fix GCbasher is going to enable preview always. 
>> Do we want to keep original test while value classes are still preview 
>> feature? 
>> I think that it is not needed, however if someone thinks it is good idea, I 
>> can add gcbasher-nopreview and keep until we start removing `@enablePreview` 
>> from our tests.
>> 
>> In the long term the problem might be that some structure are flattened, and 
>> test start generating less objects. However they are much more diverse now.
>> 
>> UPDATE. There are 3 possible approaches to maintain value/non-value classes
>> 1) keep old version of gcbahser
>> 2) keep value/non-value classes in different directories and have 2 tests 
>> with different `@compile` commands to choose different files
>> 3) add `@ValueClass` annotation and support javac plugin that change value 
>> class attribute during compilation. In this case we need to add 
>> 'PREVEW_MODE' when tests are compiles with --enable-preview and this pluign.
>> 
>> The 2) and 3) might work well to maintain value/non-value variants of tests. 
>> While it looks overcompilcated for gcbasher. So I prefer to either have 
>> 'gcbasher-novalue' or use only variant with `--enable-preview`.
>
> Leonid Mesnik has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains three additional 
> commits since the last revision:
> 
>  - updated gcbasher to use annotation for value classes
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into 8379695
>  - 8379695: [lworld] Update GCBasher to use value classes

Looks fine. My understanding is that `javac` will automatically run the 
required plugin when `--enable-preview` is provided so nothing changes on the 
test side of things.

test/hotspot/jtreg/gc/stress/gcbasher/ClassInfo.java line 30:

> 28: import java.util.Set;
> 29: 
> 30: @jdk.test.lib.valueclass.AsValueClass

While it is good to have as many value classes as possible, I am not sure what 
we gain here. But I guess it could be worth testing nonetheless.

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

Marked as reviewed by phubner (Committer).

PR Review: 
https://git.openjdk.org/valhalla/pull/2215#pullrequestreview-4067750429
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2215#discussion_r3044709426

Reply via email to