feat(nativemem): emit post-serialization counters so flush cost is visible - #754
Merged
Merged
Conversation
…sible A chunk's NM_* counters are necessarily sampled before it serializes: the chunk header records cpool_offset as the boundary between the event section and the constant pool, so no event may be appended once writeCpool() has run. But writeCpool() is where the method map is built and the dictionary grows, so a consumer reading only the in-chunk values never sees what serialization costs. Measured by forcing a mid-run dump() and reading NativeMem::_live[]/_max[] directly out of process memory either side of it, one flush takes NM_DICTIONARY from 4.55 MiB to a 216.62 MiB peak, settling at 162.46 MiB that it keeps, and builds NM_METHOD_MAP from nothing to 13.32 MiB. native_mem_max_bytes does eventually reflect that, since record() raises the peak at allocation time and nothing resets it -- but only one chunk late, and only as a lifetime maximum, so after several flushes it cannot say which one was responsible; in a single-chunk recording it is never emitted at all. Recording::capturePostFlushNativeMem() snapshots per-category live and max immediately after writeCpool(), and the following chunk emits them as native_mem_post_flush_live_bytes.<category> and native_mem_post_flush_max_bytes.<category>. The first chunk emits neither, since no flush has happened. The same capture point refreshes the JNI-visible Counters:: mirrors so a live process reading getDebugCounters0() after a dump() sees post-serialization values rather than pre-. It deliberately does not call NativeMem::sample() a second time: that advances a 64-tick moving-average window and would silently redefine avg() as a 32-chunk mean. Verified end to end on a two-chunk recording, cross-checked against the direct memory read: post-flush dictionary 161.53 live / 215.27 max via JFR against 162.46 / 216.62 from the probe, and calltrace post-flush max matching exactly. Without these, the following chunk reports dictionary live and max both at 240.02 with no way to attribute the preceding spike. MemSweepMain gains an opt-in -Dmemsweep.dumpAfterMs mid-run dump, which is what makes a flush observable while the process is still alive; default behaviour is unchanged. The memsweep probe now also reads NativeMem::_max[]. Not covered: the final chunk's own serialization, which has no following chunk to carry it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2db0c6ae6d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
CI Test ResultsRun: #36587093291 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-29 15:49:55 UTC |
Contributor
|
🔗 Commit SHA: 74e88c2 | Docs | View more details | Give us feedback! |
rkennke
commented
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NM_*counters are sampled beforewriteCpool()runs in that same chunk, so the in-chunk values can never reflect what serialization itself costs (e.g. dictionary/method-map growth during constant-pool writing). There was previously no way to observe this cost from a live JFR recording at all.Recording::capturePostFlushNativeMem(), called immediately afterwriteCpool()'s patching completes (so it captures state after serialization, beforechunk_end). Snapshots per-category live/max bytes into_post_flush_live[]/_post_flush_max[], which the following chunk emits asnative_mem_post_flush_live_bytes.<category>/native_mem_post_flush_max_bytes.<category>counter events, alongside a refresh of theCounters::JNI-visible mirrors.NativeMem::sample()a second time per chunk — that would advance a 64-tick moving-average window, silently redefiningavg()as a 32-chunk mean.Motivated by a native-memory overhead investigation that found a chunk-flush burst (dictionary/method-map growth during constant-pool serialization) invisible to any existing counter — the spike shows up in the per-category
maxcounters of the chunk after it happens, once this lands.Test plan
ddprof-lib:assembleReleasebuilds cleannative_mem_post_flush_*counters appear from the second chunk onward and reflect the prior chunk's post-writeCpool()state🤖 Generated with Claude Code