fix(codecache): report the real runtime-stubs cache size#681
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the CODECACHE_RUNTIME_STUBS_SIZE_BYTES counter to report the HotSpot JIT runtime-stubs cache size (instead of incorrectly duplicating the native-libraries code cache size), improving the accuracy of codecache-related diagnostics emitted by Profiler::dump().
Changes:
- Wire
CODECACHE_RUNTIME_STUBS_SIZE_BYTEStoJitCodeCache::_runtime_stubs.memoryUsage()instead ofnative_libs.memoryUsage(). - Add
JitCodeCache::runtimeStubsMemoryUsage()that reads under the shared stubs lock to avoid races with concurrent JVMTIDynamicCodeGenerated()updates. - Include the necessary
hotspot/jitCodeCache.hdependency fromprofiler.cpp.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ddprof-lib/src/main/cpp/profiler.cpp | Sets CODECACHE_RUNTIME_STUBS_SIZE_BYTES from JitCodeCache::runtimeStubsMemoryUsage() instead of duplicating native libs size. |
| ddprof-lib/src/main/cpp/hotspot/jitCodeCache.h | Declares runtimeStubsMemoryUsage() with concurrency rationale. |
| ddprof-lib/src/main/cpp/hotspot/jitCodeCache.cpp | Implements runtimeStubsMemoryUsage() using _stubs_lock.lockShared() around _runtime_stubs.memoryUsage(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CI Test ResultsRun: #29932498669 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Failed Testsglibc-aarch64/debug / 25-graalJob: View logs No detailed failure information available. Check the job logs. Summary: Total: 32 | Passed: 31 | Failed: 1 Updated: 2026-07-22 15:58:54 UTC |
Benchmark Results (commit 8c422a3)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126245147 Commit: ✅ Within expected boundariesNo significant runtime deltas (all within run-to-run noise) and no internal-counter outliers. Runtime details (per benchmark × JDK)
Internal counter details (ddprof)ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c422a3043
ℹ️ 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".
| // Live heap footprint of the runtime-stubs code cache. Read under the | ||
| // shared stubs lock because DynamicCodeGenerated() mutates _runtime_stubs | ||
| // (add()/expand()) concurrently. | ||
| static long long runtimeStubsMemoryUsage(); |
There was a problem hiding this comment.
Use real heap accounting for the stub counter
This wires codecache_runtime_stubs_size_bytes to a value described as the live heap footprint, but CodeCache::memoryUsage() currently charges _capacity * sizeof(CodeBlob*) + _count * sizeof(NativeFunc) while the cache actually allocates new CodeBlob[_capacity] plus variable-sized NativeFunc::create(name, ...) allocations for every stub name. For any dump after DynamicCodeGenerated entries arrive, the JFR counter still materially underreports the runtime-stub cache, so please use/introduce accounting that includes the actual CodeBlob array and name allocations before exposing it as the real stub-cache size.
AGENTS.md reference: AGENTS.md:L456-L456
Useful? React with 👍 / 👎.
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/126276516 |
|
Correct that the magnitude is still approximate here — but that's by design of the split, not something to fix in this PR:
So #681 + #677 together give an accurate runtime-stubs figure; keeping the accuracy fix in one place (#677) avoids duplicating/rebasing the same |
CODECACHE_RUNTIME_STUBS_SIZE_BYTES was set to native_libs.memoryUsage() — the same value as CODECACHE_NATIVE_SIZE_BYTES — a copy-paste since the counter was introduced (d0b0d1e). It never reflected the runtime-stubs cache. The runtime stubs live in JitCodeCache::_runtime_stubs (populated by DynamicCodeGenerated as the JVM emits stubs), a different cache from the native libraries. Add JitCodeCache::runtimeStubsMemoryUsage(), which reads that cache's size under its shared _stubs_lock (the cache is mutated concurrently by the JVMTI callback), and set the counter from it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
8c422a3 to
bb2147f
Compare
| // Live heap footprint of the runtime-stubs code cache. Read under the | ||
| // shared stubs lock because DynamicCodeGenerated() mutates _runtime_stubs | ||
| // (add()/expand()) concurrently. | ||
| static long long runtimeStubsMemoryUsage(); |
…ootprint" CodeCache::memoryUsage() is a capacity/count-based estimate on main, so "live heap footprint" overstated it. Reword to "approximate heap usage" and note #677 makes memoryUsage() exact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
🔄 Datadog auto-retried 1 job - 1 passed on retry 🔗 Commit SHA: 2886d41 | Docs | Datadog PR Page | Give us feedback! |
Benchmark Results (commit 2886d41)Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126276523 Commit:
|
| Benchmark | JDK | Latest | Dev | Δ (dev vs latest) | Issues L/D |
|---|---|---|---|---|---|
| akka-uct | 21 | ✅ 10301 ms (21 iters) | ✅ 10255 ms (21 iters) | ≈ -0.4% (±11.7%) | — / — |
| akka-uct | 25 | ✅ 8866 ms (24 iters) | ✅ 8851 ms (24 iters) | ≈ -0.2% (±10.2%) | — / — |
| finagle-chirper | 21 | ✅ 6010 ms (33 iters) | ✅ 5981 ms (33 iters) | ≈ -0.5% (±25%) | |
| finagle-chirper | 25 | ✅ 5450 ms (36 iters) | ✅ 5463 ms (36 iters) | ≈ +0.2% (±24.9%) | |
| fj-kmeans | 21 | ✅ 2695 ms (70 iters) | ✅ 2780 ms (67 iters) | 🔴 +3.2% | — / — |
| fj-kmeans | 25 | ✅ 2827 ms (66 iters) | ✅ 2809 ms (66 iters) | ≈ -0.6% (±2.6%) | — / — |
| future-genetic | 25 | ✅ 2115 ms (88 iters) | ✅ 2071 ms (90 iters) | ≈ -2.1% (±2.6%) | — / — |
| naive-bayes | 21 | ✅ 1322 ms (130 iters) | ✅ 1262 ms (136 iters) | ≈ -4.5% (±31.6%) | — / — |
| naive-bayes | 25 | ✅ 1020 ms (168 iters) | ✅ 999 ms (172 iters) | ≈ -2.1% (±31.3%) | — / — |
| reactors | 21 | ✅ 16568 ms (15 iters) | ✅ 17173 ms (15 iters) | ≈ +3.7% (±7.6%) | — / — |
| reactors | 25 | ✅ 18482 ms (15 iters) | ✅ 18271 ms (15 iters) | ≈ -1.1% (±5.5%) | — / — |
Internal counter details (ddprof)
ddprof internal counters, latest / dev (✅ = 0, · = unavailable):
| Benchmark | JDK | Dropped rec | Dropped jvmti | Dropped trace | Skipped WC | AGCT fail | Unwind fail |
|---|---|---|---|---|---|---|---|
| akka-uct | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 1960 / 2005 | ✅ / ✅ | ✅ / ✅ |
| akka-uct | 25 | ✅ / ✅ | ✅ / ✅ | 3 / 3 | 2153 / 2334 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 21 | ✅ / ✅ | ✅ / ✅ | 2 / 1 | 8594 / 8571 | ✅ / ✅ | ✅ / ✅ |
| finagle-chirper | 25 | ✅ / ✅ | ✅ / ✅ | 1 / 1 | 8022 / 8344 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / 1 | 1273 / 1304 | ✅ / ✅ | ✅ / ✅ |
| fj-kmeans | 25 | ✅ / ✅ | ✅ / ✅ | 3 / ✅ | 1278 / 1268 | ✅ / ✅ | ✅ / ✅ |
| future-genetic | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 3 | 2864 / 2825 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 21 | ✅ / ✅ | ✅ / ✅ | 4 / 2 | 3542 / 3487 | ✅ / ✅ | ✅ / ✅ |
| naive-bayes | 25 | ✅ / ✅ | ✅ / ✅ | 2 / ✅ | 3473 / 3500 | ✅ / ✅ | ✅ / ✅ |
| reactors | 21 | ✅ / ✅ | ✅ / ✅ | ✅ / ✅ | 1574 / 1749 | ✅ / ✅ | ✅ / ✅ |
| reactors | 25 | ✅ / ✅ | ✅ / ✅ | ✅ / 2 | 1873 / 1701 | ✅ / ✅ | ✅ / ✅ |
What
Fixes
CODECACHE_RUNTIME_STUBS_SIZE_BYTESto report the JIT runtime-stubs code cache, instead of duplicating the native-libraries size.Why
The counter was set to
native_libs.memoryUsage()— the same expression asCODECACHE_NATIVE_SIZE_BYTESon the line above — a copy-paste present since it was introduced (d0b0d1e44 "add counters for codecaches"). So it never reflected the runtime stubs; it just double-reported the native-libs total.The runtime stubs are a separate cache:
JitCodeCache::_runtime_stubs(name"[stubs]"), populated viaJitCodeCache::DynamicCodeGenerated()→add()as the JVM emits VM stubs. (There is also an unrelated, unusedLibraries::_runtime_stubsmember — same name, different thing; the live one is inJitCodeCache.)How
JitCodeCache::runtimeStubsMemoryUsage(), which reads_runtime_stubs.memoryUsage()under_stubs_lock.lockShared(). That cache is mutated concurrently by theDynamicCodeGeneratedJVMTI callback (add()/expand()under_stubs_lock), so the read must hold the shared lock — mirroringfindRuntimeStub().CODECACHE_RUNTIME_STUBS_SIZE_BYTESfrom it inProfiler::dump().hotspot/jitCodeCache.*is compiled for all VM targets; on non-HotSpot VMs the callback never fires, so the cache is empty and the counter reports its (small, constant) empty-cache baseline rather than the wrong native-libs figure.Notes
CodeCache::memoryUsage()itself. OnmaintodaymemoryUsage()still uses the naive_capacity*sizeof(CodeBlob*) + _count*sizeof(NativeFunc)formula, so until fix(codecache): make memoryUsage() accurate and live #677 lands this counter carries that same approximation as the siblingCODECACHE_NATIVE_SIZE_BYTES— now merely attributed to the correct cache.memoryUsage()to count the realCodeBlobarray + variable-length names + DWARF table; once it lands, everymemoryUsage()-based counter (this one included, plus theNM_NATIVE_SYMBOLSbreakdown) becomes accurate — no duplicate accuracy fix needed here._runtime_stubsis private and populated only by the JVMTI callback, so it can't be driven from gtest. Exercised by the JVM integration tests in CI (dump populates the counter).Testing
:ddprof-lib:gtestDebuggreen — 349 tests, 0 failures.🤖 Generated with Claude Code