On Mon, 19 Jun 2023 06:55:52 GMT, Axel Boldt-Christmas <abold...@openjdk.org> 
wrote:

>> The current implementation for testing generational ZGC with jtreg is 
>> implemented with a filter on the mode flag `ZGenerational`. Because of this 
>> only environments which set this flag explicitly will run most of the tests. 
>> So they get missed in Github Actions and for developers running jtreg 
>> locally without supplying the `ZGenerational` flag.
>> 
>> The proposed change here is to introduce two new jtreg requirement 
>> properties, `vm.gc.ZGenerational` and `vm.gc.ZSingelgen`. These flags will 
>> effectively behave the same as the existing `vm.gc.<GC>` flags but also take 
>> the specific ZGC mode in account.
>> 
>> If no gc flags are supplied to jtreg and the `vm.gc.Z` is true (the build 
>> includes ZGC) both `vm.gc.ZGenerational` and `vm.gc.ZSinglegen` will be true.
>> 
>> If `-XX:+UseZGC` is supplied then both `vm.gc.ZGenerational` and 
>> `vm.gc.ZSinglegen` will also be true.
>> 
>> If `-XX:{+,-}ZGenerational` is supplied then either  `vm.gc.ZGenerational` 
>> or `vm.gc.ZSinglegen` be true depending on the flags value.
>> 
>> And if `vm.gc.Z` is false both `vm.gc.ZGenerational` and `vm.gc.ZSinglegen` 
>> will be false.
>> 
>> This change also splits the relevant tests into two distinct runs for the 
>> two modes. And the respective test ids are set to `ZGenerational` or 
>> `ZSinglegen` to make it easier to distinguish the runs.
>> 
>> This also solves the issue that some compiler tests will never run with 
>> generational ZGC unless the `TEST_VM_FLAGLESS` is set. This is because the 
>> current filter `vm.opt.final.ZGenerational` requires the flag to be 
>> explicit, but these compiler tests uses `vm.flagless`. 
>> 
>> The introduction of  `vm.gc.ZGenerational` and `vm.gc.ZSinglegen` harmonizes 
>> the way you specify generational / single gen ZGC test with the way it is 
>> done for other gcs with `vm.gc.<GC>`
>> 
>> To support this feature the Whitebox API is extended with `isDefaultVMFlag` 
>> to enable checking if `ZGenerational` is default or not.
>> 
>> `vm.opt.final.ZGenerational` is still kept because we still have some 
>> reasons to filter based on the supplied flags. 
>> - `test/hotspot/jtreg/gc/cslocker/TestCSLocker.java` is disabled when 
>> running with ZGenerational
>> - `test/jdk/java/lang/ProcessBuilder/CloseRace.java` is ran with a different 
>> max heap size for ZGenerational, but it is not the intent to dispatch the 
>> test to both G1 and generational ZGC if Generational ZGC is not specified.  
>> 
>> `test/jdk/java/lang/management/MemoryMXBean/MemoryTest.java` was also 
>> changed to not filter but instead dispatch. However u...
>
> Axel Boldt-Christmas has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add explicit id to all Skynet.java @test

Thanks for all the reviews.

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

PR Comment: https://git.openjdk.org/jdk/pull/14509#issuecomment-1598489642

Reply via email to