RFR: 8332362: Implement os::committed_in_range for MacOS and AIX

Thomas Stuefe stuefe at openjdk.org
Tue Jun 11 13:54:14 UTC 2024


On Wed, 29 May 2024 14:04:37 GMT, Robert Toyonaga <duke at openjdk.org> wrote:

> ### Summary
> This change adds `os::committed_in_range` to `src/hotspot/os/posix/os_posix.cpp` so that AIX and MacOS can use it. It is mostly the same as the prior implementation in `os_linux.cpp`, but takes into account AIX may have unaligned stacks and variable page sizes, and that mincore uses a different argument type on linux.  This will allow AIX and MacOS to report true liveness information for thread stacks reported by NMT. Previously this was only possible on Windows and Linux.
> 
> **Testing**
> - `TestAlwaysPreTouchStacks.java` is updated to also test the case when stacks are large but mostly not paged in. 
> - A few new unit tests in `test_committed_virtualmemory.cpp` to check`os::committed_in_range`. I've excluded these tests on Windows because it reports the committed region as fully paged in. 
> - tier1
> 
> I've tested on Linux and Windows. I don't have a Mac to test on, but the Mac GHA tests are passing. Not yet tested on AIX.

First cursory review.

src/hotspot/os/posix/os_posix.cpp line 102:

> 100: typedef char mincore_vec_t;
> 101: #endif
> 102: 

Proposal, shorter

typedef LINUX_ONLY(unsigned) char mincore_vec_t;

src/hotspot/os/posix/os_posix.cpp line 157:

> 155: 
> 156: bool os::committed_in_range(address start, size_t size, address& committed_start, size_t& committed_size) {
> 157:   log_info(os)("test _____ %s", "in os_posix");

Please remove debug output

test/hotspot/gtest/runtime/test_committed_virtualmemory.cpp line 207:

> 205:   // The gtestLauncher are called with and without -XX:NativeMemoryTracking during jtreg-controlled
> 206:   //  gtests.
> 207: 

Please remove stray delete

test/hotspot/gtest/runtime/test_committed_virtualmemory.cpp line 217:

> 215: }
> 216: 
> 217: #ifndef _WINDOWS

Test should work on Windows, too

test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java line 43:

> 41:  * @test id=preTouch
> 42:  * @summary Test AlwaysPreTouchThreadStacks
> 43:  * @requires os.family != "aix"

Should work on aix now, no?

test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java line 53:

> 51:  * @test id=noPreTouch
> 52:  * @summary Test that only touched committed memory is reported as thread stack usage.
> 53:  * @requires os.family != "aix"

ditto

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

PR Review: https://git.openjdk.org/jdk/pull/19455#pullrequestreview-2110533903
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634928661
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634929258
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634931558
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634932004
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634932571
PR Review Comment: https://git.openjdk.org/jdk/pull/19455#discussion_r1634932834


More information about the hotspot-runtime-dev mailing list