On Tue, 12 May 2026 07:02:10 GMT, Stefan Karlsson <[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). > >> The name is misleading, it actually is meant to find live pages not just >> committed pages > > Neither the mincore man pages nor the Windows the QueryWorkingSetEx talks > about "live" pages. I don't think NMT or the rest of the JVM does either. > This leads me to think that this terminology might not be the one we want. > Also, if I see a call to `os::live_in_range` I don't know what that function > is trying to convey. Is there a better name for the property we are looking > for? > > 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. Hi @stefank, when you get the chance, can you please have another look at this? ------------- PR Comment: https://git.openjdk.org/jdk/pull/31124#issuecomment-4606976890
