On Wed, 20 May 2026 19:59:45 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: > > renaming and fix uintx type Hi @stefank, thank you for the review! I have renamed "live" to "resident" now. > Also, the NMT function that calls os::live_in_range is called > RegionIterator::next_committed, so there's still a naming skew there, I think. Yes, that's true. I could rename "committed" to "resident" here as well, but that would unfortunately also require renaming all the code related to accounting thread stacks. And ultimately we'd still have to eventually call: `VirtualMemoryTracker::Instance::add_committed_region(resident_start, resident_size, ncs);` So we still wouldn't have eliminated the naming skew, just shifted it further along the code path. Like Thomas mentions above, the root problem is that NMT reports thread stack resident memory as "committed". ------------- PR Comment: https://git.openjdk.org/jdk/pull/31124#issuecomment-4502415085
