Skip to content

feat(nativemem): categorized native-memory accounting — first cut#669

Open
rkennke wants to merge 8 commits into
mainfrom
feat/native-mem-accounting
Open

feat(nativemem): categorized native-memory accounting — first cut#669
rkennke wants to merge 8 commits into
mainfrom
feat/native-mem-accounting

Conversation

@rkennke

@rkennke rkennke commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What

Adds a NativeMem facility to measure the profiler's own native memory usage — currently we only observe whole-process RSS. Per-category live gauge, moving-window average, and a precise per-category peak, wired through the existing counter/JFR path.

Why

We measure the agent's native footprint only as an RSS delta (~150–200 MB) with no breakdown. This gives an in-process, attributable, categorized number we can trend, and a foundation for later pinpointing which code is responsible.

How

  • NativeMem (nativeMem.h / nativeMem.cpp): a NativeMemCategory enum whose per-category live gauges partition the total — each backing allocation belongs to exactly one category, so no double-counting.
  • Always-on, independent of the COUNTERS build flag: record() is a single relaxed atomic add. Most sites are off the signal path (malloc/new is not async-signal-safe anyway); the exception is CALLTRACE, whose arena allocates inside the sampling signal handler (via OS::safeAlloc), so record() is kept async-signal-safe there.
  • Precise per-category peak: record() maintains each category's high-water mark at allocation time via a relaxed CAS (common path is load+compare; CAS only on a new peak; frees skip it). Spikes between sample ticks are still captured.
  • sample() refreshes the moving-window averages and the observed total, ticked once per JFR chunk finish.
  • Exposure: totals mirror into NATIVE_MEM_{LIVE,AVG,MAX}_BYTES (JFR T_DATADOG_COUNTER + JNI debug counters); per-category native_mem_{live,avg,max}_bytes.<category> values reuse the same event format — no new event type.

Total peak: bracketed, no shared hotspot

A precise total peak would need a single global atomic hit by every allocation (a contention hotspot). Instead the total peak is bracketed:

  • NATIVE_MEM_MAX_BYTES = upper bound = sum of the precise per-category peaks.
  • native_mem_max_observed_total_bytes = lower bound = the largest instantaneous total seen at a sampling tick.

Instrumented categories

Tagged via record() at their backing alloc/free sites (precise, always-on):

  • CALLTRACELinearAllocator chunks (call-trace arena) + per-shard calltrace buffers.
  • DICTIONARY — both implementations: the older per-key-malloc Dictionary (symbols/packages — root/overflow DictTables + key strings, whose size is recovered via strlen at bulk-free), and the arena StringDictionary (arena chunks + root/overflow SBTables; keys live inside chunks so they're not counted separately). Counted unconditionally, independent of the diagnostic counters' offset gate. Single category — a per-role split, if ever wanted, belongs in a nested dimension.
  • THREAD_LOCALProfiledThread per thread.
  • JFR_BUFFERS — the Recording object (embeds the JFR buffer array).
  • LINE_TABLES — malloc'd JVMTI line-number table copies.
  • PERF — perf ring mmap (2 × page_size).
  • THREAD_FILTERChunkStorage chunks (bounded) + free-list array.

Gauge-mirrored (NativeMem::setLive() at a recompute site):

  • NATIVE_SYMBOLS — the profiler's per-native-library symbol tables used to symbolicate native frames (async-profiler's CodeCache; not the JVM code cache). Shares the existing CODECACHE_NATIVE_SIZE_BYTES counter's accuracy caveats (under-counts symbol-name strings, DWARF tables, and the blob array; stale snapshot); those are fixed in fix(codecache): make memoryUsage() accurate and live #677, which this gauge picks up automatically once both land.

Not yet covered (reads 0):

  • The long tail — scattered new/malloc across the remaining files → best captured by malloc interception, not hand-tagging.

(CONTEXT was removed: the OTel context record is embedded in ProfiledThread, already counted under THREAD_LOCAL; a separate category would double-count or read a permanent 0.)

So the total is still a subset of the RSS delta until interception lands, but now covers the major consumers.

Avoiding double-counting

Call-trace bytes appear at three layers (safeAlloc mmap, LINEAR_ALLOCATOR_BYTES reserved chunks, CALLTRACE_STORAGE_BYTES used-within-chunks); summing counters would double/triple-count. NativeMem counts backing memory once per category; the existing reserved/used/waste counters remain an independent nested diagnostic dimension. The dictionary tagging follows the same rule — e.g. arena keys are inside chunks and are not counted separately.

Performance

Designed to be off the hot path. Measured in operations, not observed as a regression in this workload:

  • Per sample (signal handler): zero in the common case. A sample's CallTraceStorage::put normally just bump-allocates within an existing arena chunk and touches no record(). record() runs only when the call-trace arena grows a new chunk (8 MiB apart), and there it's a lock-free relaxed atomic add + high-water update — async-signal-safe.
  • Per backing allocation: ~1–2 relaxed atomic ops. record() is one relaxed fetch_add; on allocation it also does a relaxed load + compare, with a CAS only when a new per-category peak is set (rare, since the peak is monotonic). This sits next to a malloc/calloc/mmap/new that dominates the cost, so it's negligible. These sites (dictionary inserts, chunk/table growth, thread/lib/recording creation) are not per-sample.
  • Per JFR chunk finish: a fixed, tiny cost. sample() is O(categories × window) ≈ 9 × 64 folds; writeNativeMem() emits ~28 counter events. This runs once per chunk rotation (seconds–minutes), off the sampling path.
  • One extra pass at dictionary clear(): the per-key free now also does a strlen to recover the freed size, so clear() cost is O(sum of key lengths) rather than O(key count). clear() already walks every key; this is a marginal constant-factor increase, at dictionary rotation only.

No new locks, no shared global counter on the hot path (the total peak is bracketed precisely to avoid one), and the always-on atomics are relaxed. sample()/writeNativeMem() run within the existing recording flush.

Roadmap

  1. This PR — facility + precise per-category max (bracketed total) + the major consumers, including DICTIONARY.
  2. Malloc interception — capture the untagged long tail so categories sum toward the RSS delta, plus a reconciliation check to quantify what's still unattributed.

Testing

  • nativeMem_ut (8 tests): live tracking + total partition, single-sample avg==live, moving average, max high-water, precise per-category max catching an inter-sample spike (with the bracket), negative-live clamping, setLive gauge + peak retention, category names.
  • Lifecycle invariants for both dictionary implementations (dictionary_ut, stringDictionary_ut): accounting grows on insert, returns exactly to the construction baseline after clear(), and to zero after destruction — directly proving inc/dec pairing (incl. the strlen-at-free path and arena chunk growth).
  • Full :ddprof-lib:gtestDebug suite green — 359 tests, 0 failures (rebased onto latest main).

🤖 Generated with Claude Code

@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #29833525967 | Commit: 1fe3714 | Duration: 13m 52s (longest job)

All 32 test jobs passed

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

Summary: Total: 32 | Passed: 32 | Failed: 0


Updated: 2026-07-21 13:31:57 UTC

@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit e095203)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125289326 Commit: e095203a546caadc5ac26755383d0b2880957cf7

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 21): runtime -3.6% (2121→2045 ms)
  • 🔴 future-genetic (JDK 25): runtime +5.6% (1976→2086 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10287 ms (21 iters) ✅ 10260 ms (21 iters) ≈ -0.3% (±10.9%) — / —
akka-uct 25 ✅ 8867 ms (24 iters) ✅ 8805 ms (24 iters) ≈ -0.7% (±9.8%) — / —
finagle-chirper 21 ✅ 5933 ms (33 iters) ✅ 5935 ms (33 iters) ≈ +0% (±24.9%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5475 ms (36 iters) ✅ 5453 ms (36 iters) ≈ -0.4% (±24.5%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2648 ms (71 iters) ✅ 2700 ms (69 iters) ≈ +2% (±2.7%) — / —
fj-kmeans 25 ✅ 2825 ms (66 iters) ✅ 2836 ms (66 iters) ≈ +0.4% (±2.6%) — / —
future-genetic 21 ✅ 2121 ms (88 iters) ✅ 2045 ms (90 iters) 🟢 -3.6% — / —
future-genetic 25 ✅ 1976 ms (94 iters) ✅ 2086 ms (89 iters) 🔴 +5.6% — / —
naive-bayes 21 ✅ 1230 ms (139 iters) ✅ 1259 ms (137 iters) ≈ +2.4% (±32.9%) — / —
naive-bayes 25 ✅ 1015 ms (168 iters) ✅ 1011 ms (169 iters) ≈ -0.4% (±31.6%) — / —
reactors 21 ✅ 15811 ms (16 iters) ✅ 16445 ms (15 iters) ≈ +4% (±7.6%) — / —
reactors 25 ✅ 18243 ms (15 iters) ✅ 18764 ms (15 iters) ≈ +2.9% (±4.1%) — / —
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 ✅ / ✅ ✅ / ✅ 3 / 1 2094 / 1933 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 3 2216 / 2195 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 4 / 2 8790 / 8777 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 1 / 3 8165 / 8254 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 2 / 2 1283 / 1240 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 1 / 2 1289 / 1281 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / 2 2935 / 2926 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 1 / 2 2884 / 2927 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 4 / 2 3486 / 3580 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 5 / ✅ 3448 / 3533 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / ✅ 1512 / 1844 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / 1 1866 / 1889 ✅ / ✅ ✅ / ✅

@datadog-prod-us1-5

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Reliability & Chaos Results

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

@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 62b6f2c)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125313150 Commit: 62b6f2cdc90e727ad72f0bcc5a37ef184201f0f7

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 21): runtime -2.6% (2122→2066 ms)
  • 🟢 future-genetic (JDK 25): runtime -5.4% (2071→1960 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10303 ms (21 iters) ✅ 10356 ms (21 iters) ≈ +0.5% (±10.7%) — / —
akka-uct 25 ✅ 9028 ms (24 iters) ✅ 8895 ms (24 iters) ≈ -1.5% (±9.8%) — / —
finagle-chirper 21 ✅ 5921 ms (33 iters) ✅ 5936 ms (33 iters) ≈ +0.3% (±25.2%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5537 ms (36 iters) ✅ 5535 ms (36 iters) ≈ -0% (±24.4%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2705 ms (70 iters) ✅ 2667 ms (69 iters) ≈ -1.4% (±2.6%) — / —
fj-kmeans 25 ✅ 2829 ms (66 iters) ✅ 2824 ms (66 iters) ≈ -0.2% (±2.6%) — / —
future-genetic 21 ✅ 2122 ms (88 iters) ✅ 2066 ms (90 iters) 🟢 -2.6% — / —
future-genetic 25 ✅ 2071 ms (90 iters) ✅ 1960 ms (94 iters) 🟢 -5.4% — / —
naive-bayes 25 ✅ 1022 ms (167 iters) ✅ 1012 ms (169 iters) ≈ -1% (±31.6%) — / —
reactors 21 ✅ 16191 ms (15 iters) ✅ 15952 ms (15 iters) ≈ -1.5% (±6.7%) — / —
reactors 25 ✅ 18581 ms (15 iters) ✅ 18426 ms (15 iters) ≈ -0.8% (±3.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 ✅ / ✅ ✅ / ✅ 2 / ✅ 1975 / 2029 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ ✅ / ✅ 2406 / 2283 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 7 / 7 8749 / 8443 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 1 / 2 8539 / 8755 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 5 / 3 1305 / 1244 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 4 / 1 1284 / 1292 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / 1 3065 / 2961 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 2 / 4 2938 / 2818 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 5 / 4 3483 / 3485 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / ✅ 1495 / 1554 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 1874 / 1984 ✅ / ✅ ✅ / ✅

@dd-octo-sts

dd-octo-sts Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit bdf9f55)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125648678 Commit: bdf9f55c6eff51a319914246b6bc15f6307be7da

✅ 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 ✅ 10249 ms (21 iters) ✅ 10281 ms (21 iters) ≈ +0.3% (±10.9%) — / —
akka-uct 25 ✅ 8849 ms (24 iters) ✅ 8841 ms (24 iters) ≈ -0.1% (±10.1%) — / —
finagle-chirper 21 ✅ 5980 ms (33 iters) ✅ 5982 ms (33 iters) ≈ +0% (±25.9%) ⚠️ W:3 / ⚠️ W:4
finagle-chirper 25 ✅ 5494 ms (36 iters) ✅ 5505 ms (36 iters) ≈ +0.2% (±24.7%) ⚠️ W:4 / ⚠️ W:3
fj-kmeans 21 ✅ 2819 ms (66 iters) ✅ 2825 ms (66 iters) ≈ +0.2% (±2.5%) — / —
future-genetic 21 ✅ 2141 ms (87 iters) ✅ 2100 ms (88 iters) ≈ -1.9% (±2.7%) — / —
future-genetic 25 ✅ 2082 ms (89 iters) ✅ 2128 ms (87 iters) ≈ +2.2% (±2.6%) — / —
naive-bayes 21 ✅ 1229 ms (139 iters) ✅ 1235 ms (138 iters) ≈ +0.5% (±32.9%) — / —
naive-bayes 25 ✅ 984 ms (173 iters) ✅ 1018 ms (168 iters) ≈ +3.5% (±32.6%) — / —
reactors 21 ✅ 15877 ms (15 iters) ✅ 15755 ms (15 iters) ≈ -0.8% (±8.7%) — / —
reactors 25 ✅ 18486 ms (15 iters) ✅ 18630 ms (15 iters) ≈ +0.8% (±4.4%) — / —
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 ✅ / ✅ ✅ / ✅ 3 / 1 2044 / 1900 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 2 / 3 2166 / 2375 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 3 / 4 8177 / 8568 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ ✅ / 1 8459 / 8227 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 1 / 1 1274 / 1269 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ ✅ / 2 1275 / 1300 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 4 / 1 3074 / 2943 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ ✅ / 1 2833 / 2770 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 2 / 2 3517 / 3483 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 3 / 2 3468 / 3482 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ 1646 / 1662 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / 1 1906 / 1963 ✅ / ✅ ✅ / ✅

@dd-octo-sts

dd-octo-sts Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit f1a84c6)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125658666 Commit: f1a84c60a8b99c21a4d57835fc7da23b96871e0a

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 21): runtime -4.8% (2141→2039 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10309 ms (21 iters) ✅ 10323 ms (21 iters) ≈ +0.1% (±10.8%) — / —
akka-uct 25 ✅ 8961 ms (24 iters) ✅ 8884 ms (24 iters) ≈ -0.9% (±9.8%) — / —
finagle-chirper 21 ✅ 6042 ms (33 iters) ✅ 5955 ms (33 iters) ≈ -1.4% (±25.3%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5468 ms (36 iters) ✅ 5531 ms (36 iters) ≈ +1.2% (±25%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2649 ms (70 iters) ✅ 2695 ms (70 iters) ≈ +1.7% (±2.7%) — / —
fj-kmeans 25 ✅ 2800 ms (67 iters) ✅ 2807 ms (66 iters) ≈ +0.3% (±2.6%) — / —
future-genetic 21 ✅ 2141 ms (87 iters) ✅ 2039 ms (90 iters) 🟢 -4.8% — / —
future-genetic 25 ✅ 2088 ms (89 iters) ✅ 2127 ms (87 iters) ≈ +1.9% (±2.5%) — / —
naive-bayes 21 ✅ 1227 ms (139 iters) ✅ 1280 ms (134 iters) ≈ +4.3% (±33%) — / —
naive-bayes 25 ✅ 1024 ms (167 iters) ✅ 1022 ms (167 iters) ≈ -0.2% (±32%) — / —
reactors 21 ✅ 16285 ms (15 iters) ✅ 16003 ms (15 iters) ≈ -1.7% (±5.7%) — / —
reactors 25 ✅ 18050 ms (15 iters) ✅ 18143 ms (15 iters) ≈ +0.5% (±6%) — / —
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 ✅ / ✅ ✅ / ✅ ✅ / 5 2017 / 2009 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 1 2344 / 2264 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 3 / 5 8749 / 8648 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 3 / 4 8234 / 8352 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 2 / 2 1239 / 1269 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 2 / 2 1267 / 1274 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 3 / 1 3072 / 2901 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ ✅ / 1 2974 / 2854 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 2 / 2 3509 / 3545 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 3 / ✅ 3490 / 3444 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / ✅ 1756 / 1751 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / 1 1854 / 1753 ✅ / ✅ ✅ / ✅

@dd-octo-sts

dd-octo-sts Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 277e2d8)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125665685 Commit: 277e2d80bb9893784ce7e4ca60240217faeaaa71

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 21): runtime -5.6% (2159→2039 ms)
  • 🔴 future-genetic (JDK 25): runtime +4% (2030→2111 ms)
  • 🟢 reactors (JDK 21): runtime -9.6% (16429→14845 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10393 ms (21 iters) ✅ 10229 ms (21 iters) ≈ -1.6% (±11%) — / —
akka-uct 25 ✅ 8931 ms (24 iters) ✅ 8766 ms (24 iters) ≈ -1.8% (±9.8%) — / —
finagle-chirper 21 ✅ 6009 ms (33 iters) ✅ 5981 ms (33 iters) ≈ -0.5% (±24.6%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5507 ms (36 iters) ✅ 5496 ms (36 iters) ≈ -0.2% (±24.7%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2761 ms (68 iters) ✅ 2694 ms (70 iters) ≈ -2.4% (±2.7%) — / —
fj-kmeans 25 ✅ 2757 ms (68 iters) ✅ 2798 ms (67 iters) ≈ +1.5% (±2.7%) — / —
future-genetic 21 ✅ 2159 ms (86 iters) ✅ 2039 ms (92 iters) 🟢 -5.6% — / —
future-genetic 25 ✅ 2030 ms (91 iters) ✅ 2111 ms (88 iters) 🔴 +4% — / —
naive-bayes 21 ✅ 1283 ms (133 iters) ✅ 1258 ms (136 iters) ≈ -1.9% (±32.4%) — / —
naive-bayes 25 ✅ 1019 ms (168 iters) ✅ 1020 ms (168 iters) ≈ +0.1% (±31.8%) — / —
reactors 21 ✅ 16429 ms (15 iters) ✅ 14845 ms (17 iters) 🟢 -9.6% — / —
reactors 25 ✅ 18219 ms (15 iters) ✅ 18034 ms (15 iters) ≈ -1% (±4.2%) — / —
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 ✅ / ✅ ✅ / ✅ 8 / 1 1988 / 2062 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 9 2080 / 2084 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 4 / 3 8485 / 8415 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 3 / 2 8560 / 8615 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 4 / 2 1282 / 1286 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 6 / 1 1278 / 1295 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 2 / 1 3002 / 3031 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 2 / ✅ 2944 / 2864 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 2 / 3 3504 / 3524 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 2 / 6 3485 / 3488 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ 1536 / 1691 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 1780 / 1837 ✅ / ✅ ✅ / ✅

@dd-octo-sts

dd-octo-sts Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit ac8c128)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125736830 Commit: ac8c128f639c2eaf1c7d2b6526a1106e2d09ca0f

⚠️ Significant outliers

  • 🟢 future-genetic (JDK 25): runtime -3.7% (2130→2051 ms)
Runtime details (per benchmark × JDK)
Benchmark JDK Latest Dev Δ (dev vs latest) Issues L/D
akka-uct 21 ✅ 10383 ms (21 iters) ✅ 10256 ms (21 iters) ≈ -1.2% (±11.4%) — / —
akka-uct 25 ✅ 8923 ms (24 iters) ✅ 8855 ms (24 iters) ≈ -0.8% (±10%) — / —
finagle-chirper 21 ✅ 5980 ms (33 iters) ✅ 6028 ms (33 iters) ≈ +0.8% (±25.1%) ⚠️ W:4 / ⚠️ W:4
finagle-chirper 25 ✅ 5483 ms (36 iters) ✅ 5498 ms (36 iters) ≈ +0.3% (±24.1%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2749 ms (68 iters) ✅ 2717 ms (69 iters) ≈ -1.2% (±2.7%) — / —
fj-kmeans 25 ✅ 2826 ms (66 iters) ✅ 2762 ms (67 iters) ≈ -2.3% (±2.6%) — / —
future-genetic 21 ✅ 2042 ms (91 iters) ✅ 2044 ms (90 iters) ≈ +0.1% (±2.6%) — / —
future-genetic 25 ✅ 2130 ms (87 iters) ✅ 2051 ms (90 iters) 🟢 -3.7% — / —
naive-bayes 25 ✅ 1008 ms (170 iters) ✅ 1009 ms (169 iters) ≈ +0.1% (±31.7%) — / —
reactors 21 ✅ 16229 ms (15 iters) ✅ 16543 ms (15 iters) ≈ +1.9% (±7.5%) — / —
reactors 25 ✅ 18332 ms (15 iters) ✅ 18379 ms (15 iters) ≈ +0.3% (±3.4%) — / —
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 2019 / 2112 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 2 / ✅ 2418 / 2216 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 5 / 4 8420 / 8855 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ ✅ / 3 8622 / 8796 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 6 / 2 1266 / 1280 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 1307 / 1266 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ ✅ / 1 3019 / 2888 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 2890 / 2833 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 4 / 3 3505 / 3444 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ ✅ / ✅ 1626 / 1497 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / 1 1806 / 1830 ✅ / ✅ ✅ / ✅

Copilot AI review requested due to automatic review settings July 21, 2026 09:23

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

Introduces a new NativeMem facility to attribute the profiler’s own native memory usage into a small set of categories, tracking per-category live bytes, a moving-window average, and a precise per-category peak; totals are exported via existing counter/JFR pathways.

Changes:

  • Added NativeMem core implementation (nativeMem.h/.cpp) with per-category live/avg/max tracking and tests.
  • Instrumented major native allocation sites (calltrace arena/buffers, dictionaries, thread-local data, thread filter, perf mmap, line tables, JFR recording buffers, native symbols gauge) to record alloc/free deltas.
  • Emitted totals through the counters table and per-category metrics through T_DATADOG_COUNTER events on chunk finish.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ddprof-lib/src/test/cpp/stringDictionary_ut.cpp Adds lifecycle accounting-balance test for StringDictionaryBuffer NM_DICTIONARY instrumentation.
ddprof-lib/src/test/cpp/nativeMem_ut.cpp New unit tests covering live/avg/max semantics, peaks between samples, clamping behavior, and category names.
ddprof-lib/src/test/cpp/dictionary_ut.cpp Adds lifecycle accounting-balance test for Dictionary NM_DICTIONARY instrumentation.
ddprof-lib/src/main/cpp/threadLocalData.h Records NM_THREAD_LOCAL on ProfiledThread creation via forTid.
ddprof-lib/src/main/cpp/threadLocalData.cpp Records NM_THREAD_LOCAL decrements on TLS destruction/release paths.
ddprof-lib/src/main/cpp/threadFilter.cpp Accounts for thread-filter chunk and freelist backing allocations under NM_THREAD_FILTER.
ddprof-lib/src/main/cpp/stringDictionary.h Accounts arena chunks + SBTable allocations/frees under NM_DICTIONARY.
ddprof-lib/src/main/cpp/profiler.cpp Accounts per-shard calltrace buffer allocations/frees under NM_CALLTRACE; mirrors native symbol gauge via setLive.
ddprof-lib/src/main/cpp/perfEvents_linux.cpp Accounts perf ring mmap/unmap under NM_PERF.
ddprof-lib/src/main/cpp/nativeMem.h Defines categories and NativeMem API (record/setLive/sample/live/avg/max/totals).
ddprof-lib/src/main/cpp/nativeMem.cpp Implements totals, sampling window averaging, reset, and category naming.
ddprof-lib/src/main/cpp/linearAllocator.cpp Accounts calltrace arena chunk alloc/free under NM_CALLTRACE.
ddprof-lib/src/main/cpp/flightRecorder.h Declares Recording helpers to sample/export native-mem metrics.
ddprof-lib/src/main/cpp/flightRecorder.cpp Samples native-mem each chunk and emits totals + per-category counter events.
ddprof-lib/src/main/cpp/dictionary.h Accounts root table allocation under NM_DICTIONARY.
ddprof-lib/src/main/cpp/dictionary.cpp Accounts key-string alloc/free and overflow/root table alloc/free under NM_DICTIONARY.
ddprof-lib/src/main/cpp/counters.h Adds total native-mem counters (live/avg/max) to the counter table.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1788 to +1802
void Recording::updateNativeMemStats() {
// Refresh the moving-window averages and the observed total peak. Per-category
// peaks are maintained precisely at allocation time, so they are not sampled
// here; the total peak is bracketed instead (see writeNativeMem).
NativeMem::sample();

// Mirror the totals into the flat counter table so they flow out through the
// existing counter path (JFR T_DATADOG_COUNTER events and the JNI debug
// counters). NATIVE_MEM_MAX_BYTES carries the upper bound on the total peak
// (sum of precise per-category peaks); the observed lower bound and the
// per-category values are emitted by writeNativeMem().
Counters::set(NATIVE_MEM_LIVE_BYTES, NativeMem::liveTotal());
Counters::set(NATIVE_MEM_AVG_BYTES, NativeMem::avgTotal());
Counters::set(NATIVE_MEM_MAX_BYTES, NativeMem::maxTotal());
}
Comment on lines +1818 to +1830
// Per-category live/avg/max, named "<metric>.<category>". The max here is the
// precise per-category peak tracked at allocation time.
for (int c = 0; c < NM_NUM_CATEGORIES; c++) {
NativeMemCategory cat = (NativeMemCategory)c;
const char *name = NativeMem::categoryName(cat);
const struct {
const char *prefix;
long long value;
} metrics[] = {
{"native_mem_live_bytes.", NativeMem::live(cat)},
{"native_mem_avg_bytes.", NativeMem::avg(cat)},
{"native_mem_max_bytes.", NativeMem::max(cat)},
};
@dd-octo-sts

dd-octo-sts Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 8a9567f)

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

✅ 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 ✅ 10186 ms (21 iters) ✅ 10281 ms (21 iters) ≈ +0.9% (±11.6%) — / —
akka-uct 25 ✅ 8926 ms (24 iters) ✅ 8870 ms (24 iters) ≈ -0.6% (±10.2%) — / —
finagle-chirper 21 ✅ 5952 ms (33 iters) ✅ 6019 ms (33 iters) ≈ +1.1% (±25.7%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5541 ms (35 iters) ✅ 5513 ms (36 iters) ≈ -0.5% (±24.8%) ⚠️ W:4 / ⚠️ W:3
fj-kmeans 21 ✅ 2722 ms (69 iters) ✅ 2779 ms (67 iters) ≈ +2.1% (±2.7%) — / —
fj-kmeans 25 ✅ 2791 ms (67 iters) ✅ 2820 ms (66 iters) ≈ +1% (±2.6%) — / —
future-genetic 21 ✅ 2089 ms (89 iters) ✅ 2091 ms (89 iters) ≈ +0.1% (±2.7%) — / —
future-genetic 25 ✅ 2068 ms (89 iters) ✅ 2054 ms (90 iters) ≈ -0.7% (±2.7%) — / —
naive-bayes 21 ✅ 1290 ms (133 iters) ✅ 1228 ms (139 iters) ≈ -4.8% (±32%) — / —
naive-bayes 25 ✅ 1016 ms (169 iters) ✅ 1025 ms (167 iters) ≈ +0.9% (±31.6%) — / —
reactors 21 ✅ 16308 ms (15 iters) ✅ 16584 ms (15 iters) ≈ +1.7% (±7.9%) — / —
reactors 25 ✅ 17591 ms (15 iters) ✅ 18412 ms (15 iters) ≈ +4.7% (±5.9%) — / —
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 / ✅ 1948 / 1908 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 3 / ✅ 2298 / 2155 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 2 / 2 8534 / 8361 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 8731 / 8511 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 3 / 6 1249 / 1252 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 1 / 2 1265 / 1305 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 3 / 2 2953 / 2951 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 3 / 5 2919 / 2902 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 4 / 3 3496 / 3510 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 3 / 7 3506 / 3485 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / 1 1655 / 1670 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / 1 1735 / 1903 ✅ / ✅ ✅ / ✅

rkennke and others added 6 commits July 21, 2026 13:42
Add a NativeMem facility that tracks the profiler's own native memory
usage per category, with a moving-window average and a running peak.

- NativeMemCategory enum whose per-category live gauges partition the
  total (each backing allocation belongs to exactly one category, so
  there is no double counting).
- Live accounting is always-on and independent of the COUNTERS build
  flag: record() is a single relaxed atomic add, async-signal-safe and
  usable from signal handlers.
- sample() folds the live gauges into a moving-window average and a
  high-water max; it is ticked once per JFR chunk finish.
- Totals mirror into the existing NATIVE_MEM_{LIVE,AVG,MAX}_BYTES
  counters (JFR + JNI counter path); per-category values are emitted as
  native_mem_{live,avg,max}_bytes.<category> counter events, reusing the
  existing counter event format (no new event type).

Instrumented sites (tagged CALLTRACE, their sole use today): the
LinearAllocator chunk alloc/free (the call-trace arena) and the
per-shard calltrace buffers. Accounting lives with the semantic owner
rather than OS::safeAlloc, which stays category-agnostic.

First-cut limits, documented in code: only CALLTRACE sites are
instrumented so far (other categories read 0 until tagged); the max is
sampled rather than spike-accurate; transient negative live is clamped
to 0. The existing reserved/used/waste counters (CALLTRACE_STORAGE_BYTES,
DICTIONARY_ARENA_WASTE_BYTES) remain an independent nested dimension and
are intentionally not summed into the per-category total.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Track each category's peak at allocation time instead of sampling it at
chunk finish, so a spike that rises and falls between two ticks is still
captured. record() updates the per-category high-water mark on positive
deltas via a relaxed CAS: the common (no new peak) path is a single load
plus a compare, and the CAS fires only when a genuinely higher peak is
set, which is rare since the peak is monotonic. Frees skip the check.

The total peak avoids a shared global counter (a contention hotspot on a
single cache line hit by every allocation) and is instead reported as a
bracket:
- NATIVE_MEM_MAX_BYTES = upper bound = sum of the precise per-category
  peaks (exact when the peaks coincide, otherwise an overestimate).
- native_mem_max_observed_total_bytes = lower bound = the largest
  instantaneous total seen at a sampling tick.

sample() no longer touches the per-category peaks; it only refreshes the
moving averages and the observed total.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend per-category accounting beyond CALLTRACE to the other large,
cleanly-paired backing allocations:

- THREAD_LOCAL: `new`/`delete ProfiledThread` (sizeof, per thread).
- JFR_BUFFERS: `new`/`delete Recording` (embeds the RecordingBuffer array
  and cpu-monitor buffer).
- LINE_TABLES: the malloc'd JVMTI line-number table copy, tracked beside
  the existing LINE_NUMBER_TABLES counter and freed in
  ~SharedLineNumberTable (byte size recovered from the stored entry count).
- PERF: the perf ring mmap (2 * page_size), paired with its munmap.
- THREAD_FILTER: ChunkStorage chunks (bounded, tagged only on successful
  CAS install) and the FreeListNode array.
- CODECACHE: a recomputed gauge, mirrored via the new NativeMem::setLive()
  at the existing CodeCache size-set site. setLive() overwrites live and
  still advances the peak.

Not tagged in this commit:
- CONTEXT has no separate allocation — the OTel context record is embedded
  in ProfiledThread, so it is already counted under THREAD_LOCAL; tagging
  it again would double-count. The category stays 0 by design.
- DICTIONARY is deferred to its own commit: it has two implementations
  (the older per-key-malloc Dictionary used for symbols/packages, with
  set()-based counters and bulk key-frees that lose per-item size, and the
  arena-based StringDictionary where keys live inside chunks). A partial
  number that tagged only one would mislead, so it needs dedicated
  per-implementation handling.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add DICTIONARY to the per-category accounting, and remove the CONTEXT
category.

CONTEXT had no separate backing allocation — the OTel context record is
embedded in ProfiledThread and already counted under THREAD_LOCAL — so it
would have been a permanently-zero line. Removed rather than left dangling.

DICTIONARY is tagged across both implementations, counting physical
backing allocations once each (single category; per-role breakdown, if
ever wanted, belongs in a nested dimension, not extra top-level
categories):

- Dictionary (older, per-key malloc; used for symbols/packages): root and
  overflow DictTables (constant size) and key strings. Key frees are bulk
  and lose per-item sizes, but keys are null-terminated so the malloc'd
  size (strlen + 1) is recovered at free — no running total needed.
- StringDictionary / StringArena (arena-based): arena chunks and root /
  overflow SBTables. Keys are bump-allocated inside chunks, so they are
  NOT counted separately (that would double-count). Chunk and SBTable
  accounting is unconditional, independent of the diagnostic counters'
  _counter_offset gate, so anonymous dictionaries are covered too.

Tests: lifecycle invariants for both implementations assert the accounting
grows on insert, returns exactly to the construction baseline after
clear(), and to zero after destruction — directly proving inc/dec pairing
(including the strlen-at-free path and arena chunk growth). Full
gtestDebug suite green (356 tests).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The class comment implied every record() call needs to be
async-signal-safe. In fact most categories allocate via malloc/new off
the signal path, where the property is irrelevant. Only the CALLTRACE
arena allocates from within the sampling signal handler (via
OS::safeAlloc's raw mmap syscall), so that is where record() staying a
relaxed atomic add actually matters. Reword accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "CodeCache" name is async-profiler's, and collides with the JVM's
JIT code cache. This category measures something unrelated: the
profiler's own per-native-library symbol tables (used to symbolicate
native frames), not JVM-managed code. Rename the NativeMem category and
its JFR label to native_symbols so the metric is unambiguous.

The existing CODECACHE_NATIVE_SIZE_BYTES counter keeps its name for
continuity; only the new NativeMem category is renamed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 21, 2026 11:52
@rkennke
rkennke force-pushed the feat/native-mem-accounting branch from 8a9567f to 3c82245 Compare July 21, 2026 11:52

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 17 out of 17 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ddprof-lib/src/main/cpp/flightRecorder.cpp:1815

  • writeNativeMem() serializes values with Buffer::putVar64(u64). If any native-mem gauge goes negative (possible until all free sites are instrumented), the implicit signed->unsigned conversion will emit a huge varint. Clamp to 0 before encoding to keep the JFR/counter stream valid.
  auto emit = [&](const char *label, long long value) {
    int start = buf->skip(1);
    buf->putVar64(T_DATADOG_COUNTER);
    buf->putVar64(_start_ticks);
    buf->putUtf8(label);
    buf->putVar64(value);
    writeEventSizePrefix(buf, start);
    flushIfNeeded(buf);

Comment on lines +28 to +34
long long NativeMem::liveTotal() {
long long total = 0;
for (int c = 0; c < NM_NUM_CATEGORIES; c++) {
total += load(_live[c]);
}
return total;
}
Comment on lines 1374 to 1387
@@ -1383,13 +1385,17 @@ Error Profiler::start(Arguments &args, bool reset) {
return Error("Not enough memory to allocate stack trace buffers (try "
"smaller jstackdepth)");
}
@dd-octo-sts

dd-octo-sts Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 3c82245)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125929510 Commit: 3c82245cff2102b58ef2e0ca5e28df5faf67d3b7

✅ 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 ✅ 10196 ms (21 iters) ✅ 10303 ms (21 iters) ≈ +1% (±11%) — / —
akka-uct 25 ✅ 8881 ms (24 iters) ✅ 8870 ms (24 iters) ≈ -0.1% (±10%) — / —
finagle-chirper 21 ✅ 5949 ms (33 iters) ✅ 5963 ms (33 iters) ≈ +0.2% (±25.5%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5429 ms (36 iters) ✅ 5455 ms (36 iters) ≈ +0.5% (±23.8%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2755 ms (68 iters) ✅ 2758 ms (68 iters) ≈ +0.1% (±2.8%) — / —
fj-kmeans 25 ✅ 2822 ms (66 iters) ✅ 2830 ms (66 iters) ≈ +0.3% (±2.6%) — / —
future-genetic 21 ✅ 2080 ms (89 iters) ✅ 2046 ms (90 iters) ≈ -1.6% (±2.5%) — / —
future-genetic 25 ✅ 2064 ms (90 iters) ✅ 2114 ms (88 iters) ≈ +2.4% (±2.5%) — / —
naive-bayes 21 ✅ 1270 ms (135 iters) ✅ 1278 ms (134 iters) ≈ +0.6% (±32.9%) — / —
naive-bayes 25 ✅ 1011 ms (169 iters) ✅ 1015 ms (168 iters) ≈ +0.4% (±31.7%) — / —
reactors 21 ✅ 17509 ms (15 iters) ✅ 16616 ms (15 iters) ≈ -5.1% (±7.9%) — / —
reactors 25 ✅ 18621 ms (15 iters) ✅ 18322 ms (15 iters) ≈ -1.6% (±3.9%) — / —
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 / 3 1953 / 1993 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 7 / 3 2318 / 2195 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 3 / 3 8547 / 8631 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ ✅ / 2 8658 / 8388 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ 1 / 4 1268 / 1277 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ 2 / 1 1267 / 1274 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 4 / 1 2930 / 2911 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 2 / ✅ 2869 / 2933 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 1 / 3 3541 / 3482 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 3 / 4 3499 / 3480 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / ✅ 1717 / 1831 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ ✅ / ✅ 1914 / 1928 ✅ / ✅ ✅ / ✅

The class comment still described record() as "a single relaxed atomic
add" from before precise per-category max was added. record() now also
does a conditional lock-free high-water update on allocation. Correct the
wording; the async-signal-safety guarantee still holds (lock-free atomics).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 21, 2026 13:01

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 17 out of 17 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

ddprof-lib/src/main/cpp/flightRecorder.cpp:1801

  • NativeMem::sample() explicitly clamps negative per-category live values to 0 when computing totals/averages, but the exported total live counter uses NativeMem::liveTotal(), which can go negative if any category is temporarily negative (the scenario sample() is already designed to tolerate). This can produce a nonsensical negative "native_mem_live_bytes" in the counter stream and makes the totals inconsistent with the sampled/clamped window.
  // per-category values are emitted by writeNativeMem().
  Counters::set(NATIVE_MEM_LIVE_BYTES, NativeMem::liveTotal());
  Counters::set(NATIVE_MEM_AVG_BYTES, NativeMem::avgTotal());
  Counters::set(NATIVE_MEM_MAX_BYTES, NativeMem::maxTotal());

Two invariants the accounting relies on are now asserted (stripped under
NDEBUG, so no release cost and never in a real signal handler):

- Per-category live bytes never go negative: record() asserts the
  post-update value >= 0. A negative means an unbalanced/oversized free.
  This lets liveTotal() sum without clamping, since each term is >= 0.
- Dictionary keys are NUL-free strings of exactly `length`: allocateKey()
  asserts strlen == length. The NM_DICTIONARY free path recovers a key's
  size via strlen at clear(), so an embedded NUL would under-count; the
  assert trips in debug/gtest instead of silently drifting.

The sample() clamp is retained as a release-mode safety net for the
asserted-impossible negative case, and its comment updated to say so. The
old NegativeLiveClampedInSample test is removed: it deliberately drove a
category negative, which now (correctly) trips the record() assert.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 21, 2026 13:14
@rkennke
rkennke marked this pull request as ready for review July 21, 2026 13:14
@rkennke
rkennke requested a review from a team as a code owner July 21, 2026 13:14

@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: 19db9d3172

ℹ️ 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".

if (rec != nullptr) {
// NULL first, deallocate later
_rec = nullptr;
NativeMem::record(NM_JFR_BUFFERS, -(long long)sizeof(Recording));

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 Keep JFR buffer bytes counted through final chunk

When stopping a recording, delete rec invokes Recording::~Recording(), which calls finishChunk(true) and emits the native-memory counters. This decrement runs before that destructor, so the final JFR chunk reports jfr_buffers and the total as if the Recording buffers were already freed even though they are still live during serialization; move the decrement after delete rec or after finishChunk to keep stop output accurate.

Useful? React with 👍 / 👎.

// CodeCache here is the profiler's native-symbol tables (not the JVM code
// cache). Its size is a recomputed gauge (not alloc/free deltas), so mirror
// it as an absolute. It shares the accuracy caveats of the counter above.
NativeMem::setLive(NM_NATIVE_SYMBOLS, native_libs.memoryUsage());

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 Refresh native-symbol gauge on stop as well

When users stop without a prior dump, execution follows ACTION_STOP into Profiler::stop, which refreshes Libraries and then calls _jfr.stop() without executing this dump-only NativeMem::setLive. As a result the final recording emits native_mem_*_bytes.native_symbols as 0 or stale for the normal stop path; mirror native_libs.memoryUsage() before _jfr.stop() too, or centralize this refresh in the chunk-finish path.

Useful? React with 👍 / 👎.

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 17 out of 17 changed files in this pull request and generated 2 comments.

Comment on lines 1745 to +1751
Counters::set(CODECACHE_NATIVE_SIZE_BYTES, native_libs.memoryUsage());
Counters::set(CODECACHE_RUNTIME_STUBS_SIZE_BYTES,
native_libs.memoryUsage());
// CodeCache here is the profiler's native-symbol tables (not the JVM code
// cache). Its size is a recomputed gauge (not alloc/free deltas), so mirror
// it as an absolute. It shares the accuracy caveats of the counter above.
NativeMem::setLive(NM_NATIVE_SYMBOLS, native_libs.memoryUsage());
Comment on lines +95 to +96

// Every category exposes a distinct, non-empty name.
@dd-octo-sts

dd-octo-sts Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Benchmark Results (commit 19db9d3)

Pipeline: https://gitlab.ddbuild.io/DataDog/apm-reliability/benchmarking-platform/-/pipelines/125953166 Commit: 19db9d3172a3b825b2a3c2d9cde16d419f2a16ab

✅ 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 ✅ 10184 ms (21 iters) ✅ 10222 ms (21 iters) ≈ +0.4% (±10.7%) — / —
akka-uct 25 ✅ 8849 ms (24 iters) ✅ 8836 ms (24 iters) ≈ -0.1% (±9.6%) — / —
finagle-chirper 21 ✅ 5977 ms (33 iters) ✅ 5959 ms (33 iters) ≈ -0.3% (±25.1%) ⚠️ W:3 / ⚠️ W:3
finagle-chirper 25 ✅ 5517 ms (36 iters) ✅ 5496 ms (36 iters) ≈ -0.4% (±24.5%) ⚠️ W:3 / ⚠️ W:3
fj-kmeans 21 ✅ 2786 ms (67 iters) ✅ 2784 ms (67 iters) ≈ -0.1% (±2.7%) — / —
fj-kmeans 25 ✅ 2840 ms (66 iters) ✅ 2827 ms (66 iters) ≈ -0.5% (±2.6%) — / —
future-genetic 21 ✅ 2062 ms (91 iters) ✅ 2079 ms (89 iters) ≈ +0.8% (±2.7%) — / —
future-genetic 25 ✅ 2115 ms (87 iters) ✅ 2078 ms (89 iters) ≈ -1.7% (±2.6%) — / —
naive-bayes 21 ✅ 1252 ms (136 iters) ✅ 1266 ms (135 iters) ≈ +1.1% (±33.2%) — / —
naive-bayes 25 ✅ 988 ms (173 iters) ✅ 1018 ms (168 iters) ≈ +3% (±32.6%) — / —
reactors 21 ✅ 16944 ms (15 iters) ✅ 15943 ms (15 iters) ≈ -5.9% (±7.3%) — / —
reactors 25 ✅ 18369 ms (15 iters) ✅ 18723 ms (15 iters) ≈ +1.9% (±5.7%) — / —
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 ✅ / ✅ ✅ / ✅ 3 / ✅ 1939 / 1856 ✅ / ✅ ✅ / ✅
akka-uct 25 ✅ / ✅ ✅ / ✅ 1 / 1 2331 / 2274 ✅ / ✅ ✅ / ✅
finagle-chirper 21 ✅ / ✅ ✅ / ✅ 2 / 1 8422 / 8566 ✅ / ✅ ✅ / ✅
finagle-chirper 25 ✅ / ✅ ✅ / ✅ 4 / 1 8696 / 8393 ✅ / ✅ ✅ / ✅
fj-kmeans 21 ✅ / ✅ ✅ / ✅ ✅ / 1 1268 / 1272 ✅ / ✅ ✅ / ✅
fj-kmeans 25 ✅ / ✅ ✅ / ✅ ✅ / 5 1299 / 1292 ✅ / ✅ ✅ / ✅
future-genetic 21 ✅ / ✅ ✅ / ✅ 2 / 2 3012 / 2985 ✅ / ✅ ✅ / ✅
future-genetic 25 ✅ / ✅ ✅ / ✅ 4 / 2 2914 / 2832 ✅ / ✅ ✅ / ✅
naive-bayes 21 ✅ / ✅ ✅ / ✅ 1 / 4 3478 / 3535 ✅ / ✅ ✅ / ✅
naive-bayes 25 ✅ / ✅ ✅ / ✅ 3 / 1 3463 / 3510 ✅ / ✅ ✅ / ✅
reactors 21 ✅ / ✅ ✅ / ✅ 1 / 2 1715 / 1726 ✅ / ✅ ✅ / ✅
reactors 25 ✅ / ✅ ✅ / ✅ 1 / ✅ 1873 / 1930 ✅ / ✅ ✅ / ✅

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