RFR: 8330647: Two CDS tests fail with -UseCompressedOops and UseSerialGC/UseParallelGC [v3]
Stefan Karlsson
stefank at openjdk.org
Wed May 22 07:59:04 UTC 2024
On Tue, 21 May 2024 20:57:29 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
>> The following two tests contain `@requires vm.cds.write.archived.java.heap` which implies `UseG1GC && UseCompressedClassPointers`
>>
>> cds/appcds/cacheObject/ArchiveHeapTestClass.java
>> cds/serviceability/ReplaceCriticalClassesForSubgraphs.java
>>
>> The tests would fail if options conflicting with `vm.cds.write.archived.java.heap` are specified via the ` -Dtest.cds.runtime.options` property because the options would be added after the `@require` check.
>>
>> A fix is to check if the ` -Dtest.cds.runtime.options` property contains non-null value and throws a `SkippedException`.
>>
>> Update:
>> The checking of the ` -Dtest.cds.runtime.options` property is now performed in VMProps.java.
>
> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>
> comments from David H. and @turbanoff
This is not a review of the functional changes of this fix, but since this patch does change the shared VMProps.java file I have some style requests.
test/jtreg-ext/requires/VMProps.java line 337:
> 335:
> 336: final String GC_PREFIX = "-XX:+Use";
> 337: final String GC_SUFFIX = "GC";
Maybe move them to the beginning of the file and make them `private static final`? At least add a blank line after them and the following comment.
test/jtreg-ext/requires/VMProps.java line 473:
> 471: * property is compatible with writing Java heap objects into the CDS archive
> 472: */
> 473: protected boolean isCDSRuntimeOptionsCompatible() {
Please add braces for all the added `if` statements.
-------------
Changes requested by stefank (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/19274#pullrequestreview-2070416670
PR Review Comment: https://git.openjdk.org/jdk/pull/19274#discussion_r1609473577
PR Review Comment: https://git.openjdk.org/jdk/pull/19274#discussion_r1609474959
More information about the hotspot-runtime-dev
mailing list