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

[Albert Mingkun Yang]

On Mon, 29 Jan 2024 08:34:37 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.

> Please check src/hotspot/os/linux/os_linux.cpp os::pd_pretouch_memory. In this function if HugePages::thp_mode() equals THPMode::always, page_size is set to os::vm_page_size();

Thank you for the clarification.

> For numa_setup_pages, because numa_setup_pages sets memory for from/to/eden regions, I think alignment() is sufficient here.

I dug into the `numa_setup_pages`, and it seems that `page_size` is treated as alignment, not os-page-size... (The naming here is quite misleading.)

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 os.family == "linux"

I believe ` * @requires vm.gc.Parallel` is also required here.

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

> 77:     Runtime runtime = Runtime.getRuntime();
> 78:     long committedMemory = (runtime.totalMemory()) / 1024; // in kb
> 79:     long base = (long)(committedMemory * 0.9);

What's the meaning of `0.9` here? Why can one use `committedMemory` directly?

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

> 92:         System.out.println("Passed RSS = " + rss + " base value " + base);
> 93:     }
> 94:     Asserts.assertTrue(rss >= base, "heap rss should be bigger than committed heap mem");

Could this be just `Asserts.assertTrue(rss >= base, "RSS: " + rss + " should be >= Committed Heap Mem: " + base);`?

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

PR Review: https://git.openjdk.org/jdk/pull/17610#pullrequestreview-1851164334
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1471159376
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1471163133
PR Review Comment: https://git.openjdk.org/jdk/pull/17610#discussion_r1471162120