RFR: 8334513: New test gc/TestAlwaysPreTouchBehavior.java is failing [v2]

Albert Mingkun Yang ayang at openjdk.org
Mon Jul 1 12:33:20 UTC 2024


On Fri, 28 Jun 2024 19:22:32 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> See JBS issue.
>> 
>> It is not completely obvious what the problem is in Oracle's CI, but the current assumption is that RSS of the testee VM gets reduced after it started and before we measured due to memory pressure.
>> 
>> The patch:
>> - exposes os::available_memory via Whitebox
>> - For the test to count as failed, we require a certain minimum size of available memory both before and during the start of the testee JVM. Otherwise, we throw a `SkippedException`
>> 
>> I have some misgivings about this solution, though:
>> 1) obviously, it is not bullet-proof either, since it is vulnerable to fast changes in machine memory load. 
>> 2) On MacOS, we have the problem that 'os::available_memory()' totally underreports how much memory is available. Therefore, as an estimate of whether the test is valid, it is too conservative. I opened https://bugs.openjdk.org/browse/JDK-8334767 to track that issue. As long as it is not fixed, the tests will likely fall below the threshold on MacOS and, therefore, be skipped. Still, this is somewhat better than outright excluding the test for MacOS (or is it? Open to opinions)
>> 3) `SkippedException` leads to the test counting as "passed", not "skipped". I think that is a usability issue with jtreg. I cannot easily see which tests had been skipped due to SkippedException.
>> 
>> Despite my doubts, I think this is the best we can come up with if we want to have such a test.
>> 
>> Note: One way to go about (3) would be to make "minimum available memory" a `@requires` tag, similar to os.maxMemory. However, I fear that this may be easily misused and cause many tests to be excluded without notice.
>
> Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update TestAlwaysPreTouchBehavior.java

test/hotspot/jtreg/gc/TestAlwaysPreTouchBehavior.java line 101:

> 99:  * @build jdk.test.whitebox.WhiteBox
> 100:  * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
> 101:  * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -Xmx64m gc.TestAlwaysPreTouchBehavior -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC

Why the explicit `-Xmx64m`? As I understand this is essentially the launcher, whose heap-size is of little importance.
Also, why does the launch require `WhiteBoxAPI`?

test/hotspot/jtreg/gc/TestAlwaysPreTouchBehavior.java line 161:

> 159:         System.out.println("RSS: " + rss + " available: " + avail + " committed " + committed);
> 160: 
> 161:         if (args[0].equals("run")) {

When will this branch be taken? I can't find where the `run` arg is specified.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19803#discussion_r1660980458
PR Review Comment: https://git.openjdk.org/jdk/pull/19803#discussion_r1660970079


More information about the hotspot-gc-dev mailing list