8332237: [nmt] Remove the need for ThreadStackTracker::track_as_vm()

[tstuefe]

This PR simplifies NMT by removing a bunch of code needed only to track thread stacks on AIX (ThreadStackTracker::track_as_vm() is actually #ifndef AIX in disguise).

NMT has two ways to track thread stacks:
A) piggybacking on the virtual memory accounting
B) piggybacking on malloc accounting

(A) is done on all platforms except AIX. (B) is AIX.

This is because on AIX, thread stacks (more precisely, the boundaries of thread stacks as the JVM knows them) are not page-aligned. Therefore we track these stacks as if they had been allocated with malloc. This special handling adds a lot of complex coding to NMT.

We (@zhengyu123 and me) originally decided on this approach to keep NMT versatile and able to deal with platforms that place thread stacks wherever. Or with the hypothetical that the JVM manages its own thread stacks. The latter never manifested - for a number of reasons, the thread library tends to be better at managing thread stacks than the application.

In practice, we only ever did this for AIX. The irony is that with this method, we track stacks as if they were fully alive. So, arguably, this method is just a rather complicated way of iterating all live threads and adding up their thread stack sizes.

On all other platforms, we track thread stacks as virtual memory. For every thread stack tracked, we also query memory liveness via os::committed_in_range (on closer inspection, another misnomer since this function does not query commit state but liveness). os::committed_in_range is implemented on Linux and Windows, and a fallback variant exists that just acts as if the whole range were committed.

Solving the thread-stacks-not-page-aligned problem can be done in a much simpler way by aligning thread stack boundaries - for the purpose of NMT - inward to the next inner page boundaries. We pay a very small loss in fidelity. But we can now use the same method all other platforms use. We can throw away a bunch of code. And, if we ever implement os::committed_in_range for AIX, we get real thread stack liveness tracking for free.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8332237: [nmt] Remove the need for ThreadStackTracker::track_as_vm() (Enhancement - P4)

Reviewers

• Johan Sjölen (@jdksjolen - Reviewer)
• Afshin Zafari (@afshin-zafari - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19231/head:pull/19231
$ git checkout pull/19231

Update a local copy of the PR:
$ git checkout pull/19231
$ git pull https://git.openjdk.org/jdk.git pull/19231/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 19231

View PR using the GUI difftool:
$ git pr show -t 19231

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19231.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back stuefe! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@tstuefe This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8332237: [nmt] Remove the need for ThreadStackTracker::track_as_vm()

Reviewed-by: jsjolen, azafari

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 23 new commits pushed to the master branch:

• a33cb90: 8332098: Add missing @ since tags to jdk.jdi
• 910d77b: 8331953: ubsan: metaspaceShared.cpp:1305:57: runtime error: applying non-zero offset 12849152 to null pointer
• f9f8d0b: 8332101: Add an @since to StandardOperation:REMOVE in jdk.dynalink
• f398cd2: 8331575: C2: crash when ConvL2I is split thru phi at LongCountedLoop
• 96c5c3f: 8329998: Remove double initialization for parts of small TypeArrays in ZObjArrayAllocator
• ee4a9d3: 8321622: ClassFile.verify(byte[] bytes) throws unexpected ConstantPoolException, IAE
• ab8d7b0: 8324517: C2: crash in compiled code because of dependency on removed range check CastIIs
• fe8a2af: 8307778: com/sun/jdi/cds tests fail with jtreg's Virtual test thread factory
• 95f79c6: 8332253: Linux arm32 build fails after 8292591
• b687aa5: 8332176: Refactor ClassListParser::parse()
• ... and 13 more: https://git.openjdk.org/jdk/compare/1a944478a26a766f5a573a1236b642d8e7b0685c...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@tstuefe this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8332237-nmt-Thread-stack-tracking-remove-track_as_vm
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[openjdk]

@tstuefe To determine the appropriate audience for reviewing this pull request, one or more labels corresponding to different subsystems will normally be applied automatically. However, no automatic labelling rule matches the changes in this pull request. In order to have an "RFR" email sent to the correct mailing list, you will need to add one or more applicable labels manually using the /label pull request command.

Applicable Labels

• build
• client
• compiler
• core-libs
• graal
• hotspot
• hotspot-compiler
• hotspot-gc
• hotspot-jfr
• hotspot-runtime
• i18n
• ide-support
• javadoc
• jdk
• jmx
• kulla
• net
• nio
• security
• serviceability
• shenandoah

[tstuefe]

Ping @jdksjolen @afshin-zafari @gerard-ziemski

[tstuefe]

Thanks to @JoKern65 for running this through the AIX CI at SAP

[jdksjolen]

Hi,

I'm very happy to see this simplification being made in NMT, thank you for doing this! Code looks good to me, have you run this through any non-AIX CI?

[tstuefe]

/label hotspot-runtime

[tstuefe]

Hi,

I'm very happy to see this simplification being made in NMT, thank you for doing this! Code looks good to me, have you run this through any non-AIX CI?

Yes, it ran the full gamut of tests over at SAP. They test on a ton of platforms.

[openjdk]

@tstuefe
The hotspot-runtime label was successfully added.

[mlbridge]

Webrevs

• 00: Full (5abeaaa5)

[jdksjolen]

Hi,
I'm very happy to see this simplification being made in NMT, thank you for doing this! Code looks good to me, have you run this through any non-AIX CI?

Yes, it ran the full gamut of tests over at SAP. They test on a ton of platforms.

Ship it! (after receiving another review)

[afshin-zafari]

Nice cleanup. All OK. Thanks.

[tstuefe]

Thanks @jdksjolen and @afshin-zafari . I'll wait the obligatory 24, and maybe for @zhengyu123 to chime in in case I got some of the history wrong.

[tstuefe]

/integrate

[openjdk]

Going to push as commit 9160ef8.
Since your change was applied there have been 30 commits pushed to the master branch:

• 7c750fd: 8331746: Create a test to verify that the cmm id is not ignored
• de57d4b: 8332257: Shenandoah: Move evacuation methods to implementation file
• da9c23a: 8325384: sun/security/ssl/SSLSessionImpl/ResumptionUpdateBoundValues.java failing intermittently when main thread is a virtual thread
• dc184f1: 8324649: Shenandoah: replace implementation of free set
• 2599151: 8331202: Support for Duration until another Instant
• 6f7ddbe: 8260633: [macos] java/awt/dnd/MouseEventAfterStartDragTest/MouseEventAfterStartDragTest.html test failed
• fa3e94d: 8332393: Problemlist compiler/rangechecks/TestArrayAccessAboveRCAfterRCCastIIEliminated.java
• a33cb90: 8332098: Add missing @ since tags to jdk.jdi
• 910d77b: 8331953: ubsan: metaspaceShared.cpp:1305:57: runtime error: applying non-zero offset 12849152 to null pointer
• f9f8d0b: 8332101: Add an @since to StandardOperation:REMOVE in jdk.dynalink
• ... and 20 more: https://git.openjdk.org/jdk/compare/1a944478a26a766f5a573a1236b642d8e7b0685c...master

Your commit was automatically rebased without conflicts.

[openjdk]

@tstuefe Pushed as commit 9160ef8.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.