8325890: NMT: Arena Chunk value correction causes multiple errors in reporting#32044
8325890: NMT: Arena Chunk value correction causes multiple errors in reporting#32044roberttoyonaga wants to merge 1 commit into
Conversation
|
👋 Welcome back rtoyonaga! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@roberttoyonaga The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
The Problem
Historically, NMT needed special handling for
mtChunk. Free heap chunks in the chunk pool were tagged withmtChunk, and they remained asmtChunkeven after being pulled from the pool and assigned to arenas bearing other tags. For example, a malloc'd heap chunk block assigned to a mtCompiler arena continues to contribute “size” and “count” toward themtChunktag. This is problematic because that chunk memory is also accounted for in the arena’s “size” – effectively double counting the chunk’s memory.From a user’s perspective,
mtChunkmeans “free” heap chunks (not assigned to arenas). The internal NTM accounting had a different meaning:mtChunk= free chunks and arena chunks. This mismatch resulted in several problems:mtChunkvalues to the user, NMT had to subtract the total size of all arenas frommtChunksize.mtChunk.mtChunkmalloc count was wrong. This is because arena chunk count was not tracked anywhere, so could not be deducted frommtChunkcount. SomtChunk"size" is the free chunk size, butmtChunk"count" is the total chunk count. That's a conflict.mtChunkpeak size and peak count are wrong. These values are inflated by chunks assigned to arenas.mtChunk. This conflicts with the NMT summary data which has removed the chunk size from themtChunkcategory.Solution
This PR makes
mtChunkinternally only represent free chunks. This aligns the internal meaning with what is presented externally to the user. This naturally fixes all the problems listed above.This is done by removing the chunk memory from
mtChunkwhen the chunk is assigned to an arena. The assigned arena then assumes responsibility for tracking that chunk’s memory. This change meshes nicely with the existing way that NMT treats arena memory explained more in the next section.Invariants maintained
The existing way NMT treats malloc, arena, and “total” malloc memory (both internally/externally) is as follows:
The approach in this PR does not change any of the above rules. Free chunks are counted in per-tag malloc memory, but when they are assigned to arenas they switch to being counted in per-tag arena memory. A chunk’s contribution to total malloc memory remains the same for its entire lifetime.
Breakdown of operations
os::mallocoperation.mtChunksize and #mtChunksize and #os::freeoperation.Notes
Mem tags needed to be incorporated into the MST hashing function. This ensures that a heap chunk in the free pool and a chunk backing an arena correctly show up as two separate allocations in an NMT detail report. Even though they have the same allocation site and stack, their tags will differ and allow them to be treated as two unique allocation blocks. Otherwise they'd be lumped together and presented to the user as a single allocation.
The "total" malloc counters are only ever touched when a chunk is created/destroyed, not when it changes hands between arenas and the free pool.
When chunks are added/removed from arenas, I also account for the chunk header size. This is a small amount but it gets excluded from the arena size (which only includes the chunk payload). The chunk header gets counted toward the arena's tag malloc size.
Updating the MST upon heap chunk ownership changes also fixes the NMT summary vs detail inconsistency.
Other Approaches considered
I also implemented an alternate solution that changes the internal meaning of malloc vs arena memory. We can consider arena memory as a subset of malloc memory such that per-tag arena size <= per-tag malloc size. And "total" malloc size is simply a sum of all per-tag malloc sizes. In this alternate solution, when a heap chunk is assigned to an arena, the allocation changes tags to the arena tag (like in my current solution) AND the new tag's malloc counters are incremented. This means that the chunk memory would show up in per-tag malloc and per-tag arena amounts. The memory is double counted like before this PR's fix, but the semantics have changed so that the per-tag arena amount is interpreted as a subset of the per-tag malloc amount.
I decided against this solution because it requires report-time patching of size/counts in order to maintain the existing meanings of values in the NMT report. It also requires extra logic to track "exclusive" malloc peaks, since now the per-tag malloc peaks include arena amounts.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32044/head:pull/32044$ git checkout pull/32044Update a local copy of the PR:
$ git checkout pull/32044$ git pull https://git.openjdk.org/jdk.git pull/32044/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 32044View PR using the GUI difftool:
$ git pr show -t 32044Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32044.diff