On Thu, 21 May 2026 13:14:10 GMT, Robert Toyonaga <[email protected]> wrote:
>> ### Summary >> `os::committed_in_range` is used by NMT to account for thread stacks. The >> name is misleading, it actually is meant to find **live** pages not just >> **committed** pages. On POSIX (except AIX) this works correctly using >> `mincore`. On Windows this worked incorrectly using `VirtualQuery` (only >> finds committed, not live regions). This means that the values reported on >> Windows were inaccurate. >> >> This PR fixes the Windows implementation by using `QueryWorkingSetEx` >> instead of `VirtualQuery`. This properly identifies pages that are truly >> live in the working set, not just committed. I also renamed >> `committed_in_range` to `resident_in_range` so that the intention is >> clearer. I have tried to structure the Windows implementation as similarly >> to Posix as possible to help with maintainability. >> >> ### Testing >> - Tested on windows and linux and64 >> - `make test TEST=gtest:NMTCommittedVirtualMemory` (this tests >> `live_in_range` directly) >> - `make test >> TEST=test/hotspot/jtreg/runtime/Thread/TestAlwaysPreTouchStacks.java` (this >> tests `resident_in_range` indirectly through NMT thread stack accounting) >> - `make test TEST=gtest:os` >> - `make test TEST=hotspot_nmt` >> - tier 1 >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Robert Toyonaga has updated the pull request incrementally with one > additional commit since the last revision: > > Revert "renaming" > > This reverts commit c58a65577d446e92dc68d59227312f6135bcb32f. I think this starts looking good. There is a semantic disconnect I described earlier: Up in NMT, we deal with "committed" regions and have no concept of residency, but count resident stack pages as committed. But until we have residency numbers in NMT, this is difficult to disentangle. There is also a question of interface cutting: seems weird to have an API for resident region iteration in os:: instead of just an API that, given a range, returns size of residency. After all, this is what the sole user in NMT does. Might as well do that iteration down in os scope. But thats for a future RFE. Question, what happens if we have multiple disjunct resident regions in a stack? src/hotspot/share/runtime/os.hpp line 447: > 445: // The search begins at the provided start address. > 446: // Returns true after the first contiguous resident region is found, > otherwise false if none found. > 447: static bool resident_in_range(address start, size_t size, address& > resident_start, size_t& resident_size); I realized just now (again) that this API does not count resident space in a range, but finds the first resident region within a range. I remember that this name clash confounded me also back when I looked at NMT the first time. I therefore would rename it to "first_resident_in_range" and to adjust the comment accordingly. ------------- PR Review: https://git.openjdk.org/jdk/pull/31124#pullrequestreview-4337365762 PR Review Comment: https://git.openjdk.org/jdk/pull/31124#discussion_r3281556625
