RFR: 8317228: GC: Make TestXXXHeapSizeFlags use createTestJvm [v2]

Leo Korinth lkorinth at openjdk.org
Fri Oct 13 12:31:56 UTC 2023


On Fri, 13 Oct 2023 10:05:31 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> Leo Korinth has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add @key flag-sensitive
>
> test/jtreg-ext/requires/VMProps.java line 143:
> 
>> 141:         map.put("vm.flagless", this::isFlagless);
>> 142:         map.put("jdk.foreign.linker", this::jdkForeignLinker);
>> 143:         map.putAll(xOptFlags()); // -Xmx4g -> @requires vm.opt.x.Xmx == "4g" )
> 
> Does it have to be `vm.opt.x.*` or could we just add those to `vm.opt` and use for example: `vm.opt.Xmx == null`? Or do we have -X and -XX: options that collide?

I know of no colliding options ATM. They could be colliding in the future (especially if we chose to parse extra options not starting with a `-X`). Also, I think it is incredibly confusing to reuse the `vm.opt` namespace that is used by vanilla JTREG by our extension code in VMProps. I would object to put both in the `vm.opt` namespace.

> test/jtreg-ext/requires/VMProps.java line 728:
> 
>> 726:         return allFlags()
>> 727:             .filter(s -> s.startsWith("-X") && !s.startsWith("-XX:") && !s.equals("-X"))
>> 728:             .map(s -> s.replaceFirst("-*", ""))
> 
> Is there a need to replace "-*", should you be able to just do:
>  ```suggestion
>              .map(s -> s.replaceFirst("-", ""))

I will remove it, it is needed if I want to parse extra options starting with two dashes, but I chose to keep it simple and just parse extra options starting with `-X`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15986#discussion_r1358193037
PR Review Comment: https://git.openjdk.org/jdk/pull/15986#discussion_r1358195607


More information about the hotspot-gc-dev mailing list