RFR: 8324817: Parallel GC does not pre-touch all heap pages when AlwaysPreTouch enabled and large page disabled [v4]

[Albert Mingkun Yang]

On Wed, 31 Jan 2024 11:15:15 GMT, Wang Zhuo <wzhuo at openjdk.org> wrote:

>> AlwaysPreTouch requires all freshly committed pages to be pre-touched. While currently PS GC does not pre-touch heap pages with -XX:+AlwaysPreTouch.
>> It is related to [JDK-8315923](https://bugs.openjdk.org/browse/JDK-8315923), which fixes the issue when huge pages are used. But the bug still stands if regular page are used. On linux we can reproduce this bug when /sys/kernel/mm/transparent_hugepage/enabled is madvise or never, but cannot reproduce when it is always.
>> 
>> A simple way to reproduce, please make sure large pages are NOT used.
>> Just run a test with the following parameters
>> -XX:+AlwaysPreTouch -Xms31g -Xmx31g -XX:+UseParallelGC
>> There is no pre-touching during heap initialing stage. After initialization, RSS of the test process is much smaller than memory committed.
>> On the contrary, using -XX:+UseG1GC and run the test again
>> -XX:+AlwaysPreTouch -Xms31g -Xmx31g -XX:+UseG1GC
>> it takes several seconds to pre-touch heap pages during initialization, and RSS usage after initialization is similar to memory committed. This is the expected behavior of AlwaysPreTouch
>> 
>> This issue related to [JDK-8283935](https://bugs.openjdk.org/browse/JDK-8283935), which uses alignment() instead of os::vm_page_size() as pre-touching step size. The value of alignment() is usually bigger than OS page size, which causes most heap pages are not pre-touched.
>
> Wang Zhuo has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove printings and move necessary information into assertion.

Marked as reviewed by ayang (Reviewer).

test/hotspot/jtreg/gc/parallel/TestAlwaysPreTouchBehavior.java line 29:

> 27:  * @test TestAlwaysPreTouchBehavior
> 28:  * @summary Tests AlwaysPreTouch Bahavior, pages of java heap should be pretouched with AlwaysPreTouch enabled. This test reads RSS of test process, which should be bigger than heap size(1g) with AlwaysPreTouch enabled.
> 29:  * @requires vm.gc.Serial & os.family == "linux" & os.maxMemory > 2G

It should be Parallel, not Serial. The GC requirement should probably be on its own line to distinguish btw `vm` and `os` constraint..

test/hotspot/jtreg/gc/parallel/TestAlwaysPreTouchBehavior.java line 72:

> 70:         } catch (Exception e) {
> 71:            return EXCEPTION_VALUE;
> 72:         }

This is identical to what caller does. I believe try-catch can be omitted in this method (callee), and keep the only the try-catch in the caller.

test/hotspot/jtreg/gc/parallel/TestAlwaysPreTouchBehavior.java line 78:

> 76:     long rss = 0;
> 77:     Runtime runtime = Runtime.getRuntime();
> 78:     long committedMemory = (runtime.totalMemory()) / 1024; // in kb

The `()` seems not needed. Also, one doesn't need introduce a tmp `runtime` var.

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

PR Review: https://git.openjdk.org/jdk/pull/17610#pullrequestreview-1853635548
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1472769150
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1472765469
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1472766915