Skip to content

fix(codecache): report the real runtime-stubs cache size#681

Open
rkennke wants to merge 2 commits into
mainfrom
fix/codecache-runtime-stubs-counter
Open

fix(codecache): report the real runtime-stubs cache size#681
rkennke wants to merge 2 commits into
mainfrom
fix/codecache-runtime-stubs-counter

Conversation

@rkennke

@rkennke rkennke commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What

Fixes CODECACHE_RUNTIME_STUBS_SIZE_BYTES to 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 as CODECACHE_NATIVE_SIZE_BYTES on 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 via JitCodeCache::DynamicCodeGenerated()add() as the JVM emits VM stubs. (There is also an unrelated, unused Libraries::_runtime_stubs member — same name, different thing; the live one is in JitCodeCache.)

How

  • Add JitCodeCache::runtimeStubsMemoryUsage(), which reads _runtime_stubs.memoryUsage() under _stubs_lock.lockShared(). That cache is mutated concurrently by the DynamicCodeGenerated JVMTI callback (add()/expand() under _stubs_lock), so the read must hold the shared lock — mirroring findRuntimeStub().
  • Set CODECACHE_RUNTIME_STUBS_SIZE_BYTES from it in Profiler::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

Testing

  • :ddprof-lib:gtestDebug green — 349 tests, 0 failures.

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings July 22, 2026 13:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_BYTES to JitCodeCache::_runtime_stubs.memoryUsage() instead of native_libs.memoryUsage().
  • Add JitCodeCache::runtimeStubsMemoryUsage() that reads under the shared stubs lock to avoid races with concurrent JVMTI DynamicCodeGenerated() updates.
  • Include the necessary hotspot/jitCodeCache.h dependency from profiler.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.

@dd-octo-sts

dd-octo-sts Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #29932498669 | Commit: 7220252 | Duration: 38m 22s (longest job)

1 of 32 test jobs failed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - - -
8-ibm - - -
8-j9 - -
8-librca - -
8-orcl - - -
11 - - -
11-j9 - -
11-librca - -
17 - -
17-graal - -
17-j9 - -
17-librca - -
21 - -
21-graal - -
21-librca - -
25 - -
25-graal - -
25-librca - -

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Failed Tests

glibc-aarch64/debug / 25-graal

Job: 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

@dd-octo-sts

dd-octo-sts Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 8c422a3)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126245147 Commit: 8c422a30432280584f389c4aa4427007f1385c03

✅ Within expected boundaries

No significant runtime deltas (all within run-to-run noise) and no internal-counter outliers.

Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10332 ms (21 iters) ✅ 10170 ms (21 iters) ≈ -1.6% (±11.1%) — / —
akka-uct 25 ✅ 8849 ms (24 iters) ✅ 8865 ms (24 iters) ≈ +0.2% (±11%) — / —
finagle-chirper 21 ✅ 5998 ms (33 iters) ✅ 5981 ms (33 iters) ≈ -0.3% (±25.2%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2752 ms (68 iters) ✅ 2686 ms (70 iters) ≈ -2.4% (±2.7%) — / —
fj-kmeans 25 ✅ 2818 ms (66 iters) ✅ 2749 ms (69 iters) ≈ -2.4% (±2.6%) — / —
future-genetic 21 ✅ 2091 ms (88 iters) ✅ 2073 ms (90 iters) ≈ -0.9% (±2.6%) — / —
future-genetic 25 ✅ 2064 ms (90 iters) ✅ 2048 ms (91 iters) ≈ -0.8% (±2.6%) — / —
naive-bayes 21 ✅ 1257 ms (136 iters) ✅ 1265 ms (135 iters) ≈ +0.6% (±32.4%) — / —
naive-bayes 25 ✅ 983 ms (173 iters) ✅ 1014 ms (169 iters) ≈ +3.2% (±32%) — / —
reactors 21 ✅ 16352 ms (15 iters) ✅ 16136 ms (15 iters) ≈ -1.3% (±6.9%) — / —
reactors 25 ✅ 18512 ms (15 iters) ✅ 18450 ms (15 iters) ≈ -0.3% (±5.8%) — / —
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 ✅ / ✅ ✅ / ✅ 1 / 2 1941 / 2072 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ ✅ / 1 2346 / 2182 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 2 / 5 8580 / 8611 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 6 / 2 1262 / 1255 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ ✅ / 2 1290 / 1308 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 2870 / 2900 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 3 / 3 3516 / 3537 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 4 / 5 3421 / 3449 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / 1 1627 / 1622 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / 1 1936 / 1909 ✅ / ✅ ✅ / ✅

@rkennke
rkennke marked this pull request as ready for review July 22, 2026 14:21
@rkennke
rkennke requested a review from a team as a code owner July 22, 2026 14:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +42 to +45
// 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();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@dd-octo-sts

dd-octo-sts Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/126276516

@rkennke

rkennke commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Correct that the magnitude is still approximate here — but that's by design of the split, not something to fix in this PR:

  • This PR fixes which cache the counter reflects: CODECACHE_RUNTIME_STUBS_SIZE_BYTES was a copy-paste of native_libs.memoryUsage() and never pointed at the runtime-stubs cache at all. After this, it reads JitCodeCache::_runtime_stubs (under its lock).
  • The accuracy of memoryUsage() — the exact _capacity*sizeof(CodeBlob*) + _count*sizeof(NativeFunc) undercount you flagged — is fixed in fix(codecache): make memoryUsage() accurate and live #677, which rewrites CodeCache::memoryUsage() to count the real CodeBlob array + variable-length NativeFunc name allocations + DWARF table. That applies to every memoryUsage()-based counter, including this one and CODECACHE_NATIVE_SIZE_BYTES.

So #681 + #677 together give an accurate runtime-stubs figure; keeping the accuracy fix in one place (#677) avoids duplicating/rebasing the same memoryUsage() change across PRs. Until #677 lands, this counter carries the same naive approximation as the sibling native_libs counter — no worse, and now at least attributed to the correct cache. I've made that explicit in the PR description.

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>
@rkennke
rkennke force-pushed the fix/codecache-runtime-stubs-counter branch from 8c422a3 to bb2147f Compare July 22, 2026 14:58
Copilot AI review requested due to automatic review settings July 22, 2026 14:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +42 to +45
// 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>
@rkennke

rkennke commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Reworded the comment to "approximate heap usage" (from "live heap footprint") and noted that CodeCache::memoryUsage() is a capacity/count estimate today, made exact by #677. Same underlying point as the codex P2 above — this PR fixes which cache the counter reflects; #677 fixes the magnitude.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@datadog-datadog-prod-us1

datadog-datadog-prod-us1 Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Tests

🔄 Datadog auto-retried 1 job - 1 passed on retry View in Datadog

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 2886d41 | Docs | Datadog PR Page | Give us feedback!

@dd-octo-sts

dd-octo-sts Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 2886d41)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/126276523 Commit: 2886d411b833bf8b8e12ed533ac8b62139ea982b

⚠️ Significant outliers

  • 🔴 fj-kmeans (JDK 21): runtime +3.2% (2695→2780 ms)
Runtime details (per benchmark × JDK)
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%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5450 ms (36 iters) ✅ 5463 ms (36 iters) ≈ +0.2% (±24.9%) ⚠️ W:3 / ⚠️ W:3
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 ✅ / ✅ ✅ / ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants