From b738055464fff558e244441862d52fd964073a29 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 30 Oct 2026 16:33:39 +0100 Subject: [PATCH 1/4] fix: bound reference-chain pass rate and root-enum frequency Add a minimum pass spacing gate ahead of every shouldRunPass authorization lane (GC epoch, canary back-to-back, fallback cadence), so pass rate tracks the pause controller's cadence instead of the young-GC rate. End a pass immediately on a truncated or errored root enumeration instead of retrying on the next pass, and gate retries on an adaptive interval: 2s floor, extended by 20ns per millisecond of measured in-safepoint cost, capped at 30s. IterateOverReachableObjects deoptimizes, on every thread, every compiled frame whose scope holds a scalar-replaced object, so root-enum frequency is the deopt-all storm's lever. Add debug-only *ForTest seams over the scheduling state and remove the unused setReferenceChainSearchStartedForTest0 seam. --- ddprof-lib/src/main/cpp/javaApi.cpp | 117 ++++++++++++++++++ .../src/main/cpp/referenceChainTraversal.cpp | 60 +++++---- ddprof-lib/src/main/cpp/referenceChains.cpp | 85 ++++++++++--- ddprof-lib/src/main/cpp/referenceChains.h | 115 +++++++++++++++-- .../com/datadoghq/profiler/JavaProfiler.java | 68 ++++++++++ .../profiler/JavaProfilerApiSurfaceTest.java | 6 +- .../profiler/ReferenceChainTestHooks.java | 32 +++++ 7 files changed, 424 insertions(+), 59 deletions(-) diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index f506215f41..53d6f98045 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -1454,6 +1454,84 @@ Java_com_datadoghq_profiler_JavaProfiler_urgentLatchedForTest0(JNIEnv *env, return tracker->urgentLatchedForTest() ? JNI_TRUE : JNI_FALSE; } +// Places the previous pass a controlled distance in the past and sets the effective cadence +// shouldRunPass()'s minimum-spacing gate enforces - see setPassSpacingForTest(). +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainPassSpacingForTest0( + JNIEnv *env, jclass unused, jlong ms_since_last_pass, jlong cadence_ms) { + ProfiledThread::initCurrentThreadSignalSafe(); + ReferenceChainTracker::instance()->setPassSpacingForTest( + (u64)ms_since_last_pass, (u64)cadence_ms); +} + +// Shifts the tracker's last-pass GC-epoch snapshot relative to the live epoch counter - see +// shiftLastPassGcEpochForTest(). +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_shiftReferenceChainLastGcEpochForTest0( + JNIEnv *env, jclass unused, jlong delta) { + ProfiledThread::initCurrentThreadSignalSafe(); + ReferenceChainTracker::instance()->shiftLastPassGcEpochForTest((long)delta); +} + +// Sets the root-enumeration scheduling state (last attempt's time and measured in-safepoint +// cost) - see setRootEnumStateForTest(). +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainRootEnumStateForTest0( + JNIEnv *env, jclass unused, jlong ms_since_last_enum, + jlong last_enum_cost_ms) { + ProfiledThread::initCurrentThreadSignalSafe(); + ReferenceChainTracker::instance()->setRootEnumStateForTest( + (long)ms_since_last_enum, (u64)last_enum_cost_ms); +} + +// Evaluates shouldRunRootEnum() with a synthetic clock so a test can place the last attempt a +// controlled distance in the past. +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_shouldRunRootEnumForTest0( + JNIEnv *env, jclass unused, jboolean manual_first_pass) { + ProfiledThread::initCurrentThreadSignalSafe(); + return ReferenceChainTracker::instance()->shouldRunRootEnumForTest( + OS::nanotime(), manual_first_pass == JNI_TRUE) + ? JNI_TRUE + : JNI_FALSE; +} + +// Forces a started, RUNNING search with fresh pain budgets so shouldRunPassForTest0() evaluates +// the running-search gates deterministically - see forceRunningSearchForTest(). +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_forceRunningReferenceChainSearchForTest0( + JNIEnv *env, jclass unused) { + ProfiledThread::initCurrentThreadSignalSafe(); + ReferenceChainTracker::instance()->forceRunningSearchForTest(); +} + +// Sets the canary chase's candidate count and found-bits mask so shouldRunPassForTest0() +// evaluates the canary lane - see setCanaryChaseStateForTest(). +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainCanaryStateForTest0( + JNIEnv *env, jclass unused, jint candidate_count, jlong found_bits) { + ProfiledThread::initCurrentThreadSignalSafe(); + ReferenceChainTracker::instance()->setCanaryChaseStateForTest( + (int)candidate_count, (u64)found_bits); +} + +// Current adaptive root-enumeration interval floor - see rootEnumIntervalFloorNs(). +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_rootEnumIntervalFloorNsForTest0( + JNIEnv *env, jclass unused) { + ProfiledThread::initCurrentThreadSignalSafe(); + return (jlong)ReferenceChainTracker::instance() + ->rootEnumIntervalFloorNsForTest(); +} + +// Live GC-finish epoch - see gcFinishEpoch() (relaxed atomic load, no lock needed). +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_referenceChainGcFinishEpochForTest0( + JNIEnv *env, jclass unused) { + ProfiledThread::initCurrentThreadSignalSafe(); + return (jlong)ReferenceChainTracker::instance()->gcFinishEpoch(); +} + #else // !DEBUG // Release builds: no-op stubs so the Java natives still link. @@ -1576,6 +1654,45 @@ Java_com_datadoghq_profiler_JavaProfiler_backdateLeakSignalActivationForTest0( return 0; } +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainPassSpacingForTest0( + JNIEnv *env, jclass unused, jlong ms_since_last_pass, jlong cadence_ms) {} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_shiftReferenceChainLastGcEpochForTest0( + JNIEnv *env, jclass unused, jlong delta) {} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainRootEnumStateForTest0( + JNIEnv *env, jclass unused, jlong ms_since_last_enum, + jlong last_enum_cost_ms) {} + +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_shouldRunRootEnumForTest0( + JNIEnv *env, jclass unused, jboolean manual_first_pass) { + return JNI_FALSE; +} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_forceRunningReferenceChainSearchForTest0( + JNIEnv *env, jclass unused) {} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setReferenceChainCanaryStateForTest0( + JNIEnv *env, jclass unused, jint candidate_count, jlong found_bits) {} + +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_rootEnumIntervalFloorNsForTest0( + JNIEnv *env, jclass unused) { + return 0; +} + +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_referenceChainGcFinishEpochForTest0( + JNIEnv *env, jclass unused) { + return 0; +} + extern "C" DLLEXPORT void JNICALL Java_com_datadoghq_profiler_JavaProfiler_setLeakSignalActivatedNsForTest0( JNIEnv *env, jclass unused, jlong activatedNs) {} diff --git a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp index ceb9fb1292..92ec6c937f 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -171,8 +171,8 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, // Shared wall-clock ceiling for this whole call's static-field sweep, expandFrontier(), and // rotation sub-calls below (see _pass_deadline_ns's own comment) - deliberately NOT applied to - // root/stack-ref enumeration itself, which is instead cadence-gated by run_root_enum/ - // ROOT_ENUM_MIN_INTERVAL_NS. + // root/stack-ref enumeration itself, which is instead interval-gated by shouldRunRootEnum() + // (ROOT_ENUM_MIN_INTERVAL_NS and the adaptive extension). _pass_deadline_ns = _effective_pause_target_ms > 0 ? OS::nanotime() + (u64)_effective_pause_target_ms * 1000000ULL : 0; @@ -213,7 +213,8 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, jvmtiError root_err = jvmti->IterateOverReachableObjects( heapRootCallback, stackRefCallback, /*object_ref_callback=*/nullptr, &ctx); - *safepoint_ns += OS::nanotime() - root_enum_start_ns; + u64 root_enum_safepoint_ns = OS::nanotime() - root_enum_start_ns; + *safepoint_ns += root_enum_safepoint_ns; // expand_budget is spent independently of root_enum_budget below (see // ROOT_ENUM_MIN_INTERVAL_NS's own comment) - ctx.edges_admitted is written straight into @@ -221,23 +222,28 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, // whatever root enumeration admitted. *edges_admitted = ctx.edges_admitted; _last_root_enum_ns = OS::nanotime(); + // Every attempt - completed, truncated, or errored - re-pays the JVM's unconditional + // all-thread EA deoptimization (see ROOT_ENUM_MIN_INTERVAL_NS's comment), so the measured + // in-safepoint cost extends the next attempt's interval floor regardless of the outcome. + _last_root_enum_safepoint_ms = root_enum_safepoint_ns / 1000000; if (root_err != JVMTI_ERROR_NONE) { *truncated = true; *frontier_cap_hit = false; - _root_enum_truncated_last_time = false; return; } if (ctx.truncated) { + // A truncated attempt is retried after the normal adaptive interval (shouldRunRootEnum()), + // not on the very next pass: with a first-pass budget too small for the live heap every + // attempt truncates, and a next-pass retry would turn every pass into a deopt-all + // enumeration (see ROOT_ENUM_MIN_INTERVAL_NS's comment). Root admission is best-effort - + // meanwhile this pass ends here (an incomplete enumeration would leave the frontier's root + // picture inconsistent with the expansion phases below) and the next pass's expansion + // phases proceed on the roots already admitted. *truncated = true; *frontier_cap_hit = ctx.frontier_cap_hit; - // Only a budget-exhausted truncation (not a frontier-cap-hit, which abandons the search - // outright) is grounds to retry root enumeration on the very next pass - see - // _root_enum_truncated_last_time's own comment. - _root_enum_truncated_last_time = !ctx.frontier_cap_hit; return; } - _root_enum_truncated_last_time = false; } int expand_phase_edges_admitted = 0; @@ -1050,8 +1056,7 @@ bool ReferenceChainTracker::runPass(jvmtiEnv *jvmti, JNIEnv *jni, // (runPassManualWalk()'s own comment) - there is no "first pass walks the whole graph inline" // shortcut here, so every pass (first or resumed) takes the same expand-frontier shape. u64 now_ns = OS::nanotime(); - bool run_root_enum = manual_first_pass || _root_enum_truncated_last_time || - (now_ns - _last_root_enum_ns >= ROOT_ENUM_MIN_INTERVAL_NS); + bool run_root_enum = shouldRunRootEnum(now_ns, manual_first_pass); int frontier_size_before_pass = _frontier != nullptr ? _frontier->size() : 0; @@ -1073,17 +1078,14 @@ bool ReferenceChainTracker::runPass(jvmtiEnv *jvmti, JNIEnv *jni, _last_pass_gc_finish_epoch = gcFinishEpoch(); store(_last_pass_ns, OS::nanotime()); if (!run_root_enum) { - // A pass that ran root/stack-ref enumeration spends _first_pass_budget, not _effective_budget - - // its duration is not a signal about the per-pass cost updatePacing() is trying to regulate - // (expandFrontier()'s cheap, per-node expansion calls), so feeding it in here would throttle - // _effective_budget down for every one of those unrelated later passes based on a single, - // deliberately oversized outlier. + // Only non-root-enum passes feed the budget/cadence controller - root-enum passes are + // excluded (see the else branch for why). updatePacing(safepoint_ns); } else { - // Excluded from the budget/cadence controller above, but not from the borrow ceiling's - // revocation check (see maybeRevokeBorrowForRootEnumPass()'s own comment) - a root-enum pass's - // in-safepoint cost is real pause time and must still be able to revoke a borrowed-budget grant - // the pacing controller would otherwise keep believing is safe. + // A root-enum pass's duration is not a per-pass cost signal for updatePacing(): it spends + // _first_pass_budget, not _effective_budget, and feeding the outlier in would throttle every + // later expansion pass. The cost is still real pause time, so it can still revoke a + // borrowed-budget grant (maybeRevokeBorrowForRootEnumPass()). maybeRevokeBorrowForRootEnumPass(safepoint_ns); } // accumulate this pass's own in-safepoint cost toward the running total restartSearch() will @@ -1225,7 +1227,7 @@ bool ReferenceChainTracker::runPass(jvmtiEnv *jvmti, JNIEnv *jni, "candidateFound=%d/%d discoveredCounts=[%d,%d,%d,%d,%d]", (int)err, edges_admitted, truncated, frontier_cap_hit, (int)load(_search_state), (int)_abandon_reason, _frontier->size(), _effective_budget, - (unsigned long long)_effective_cadence_ns, + (unsigned long long)load(_effective_cadence_ns), _pending_expand.size(), _priority_expand.size(), (int)__builtin_popcountll(_candidate_found_bits), _candidate_count, _candidate_count > 0 ? _candidate_discovered_count[0] : 0, @@ -1284,20 +1286,24 @@ void ReferenceChainTracker::updatePacing(u64 pass_wall_ns) { int64_t overflow = desired - clamped; _effective_budget = (int)clamped; + // One load() and one store() of _effective_cadence_ns: threadLoop()'s urgency ramp writes it + // outside _engine_lock, so the access must be atomic. The read-modify-write as a whole is not + // atomic - it races the urgency ramp by design (while urgent the ramp owns the field) - the + // load()/store() only removes the mixed plain/atomic data race. if (overflow < 0) { // Still over the pause-time ceiling even at the minimum budget - widen the fallback interval // instead of shrinking the budget further. u64 step = (u64)(-overflow) * CADENCE_NS_PER_EDGE_OVERFLOW; - _effective_cadence_ns = - std::min(_effective_cadence_ns + step, MAX_EFFECTIVE_CADENCE_NS); + u64 cur = load(_effective_cadence_ns); + store(_effective_cadence_ns, std::min(cur + step, MAX_EFFECTIVE_CADENCE_NS)); } else if (overflow > 0) { // Comfortably under the ceiling even at the maximum (config) budget - relax the fallback // interval. u64 step = (u64)overflow * CADENCE_NS_PER_EDGE_OVERFLOW; - _effective_cadence_ns = - step >= _effective_cadence_ns - ? MIN_EFFECTIVE_CADENCE_NS - : std::max(_effective_cadence_ns - step, MIN_EFFECTIVE_CADENCE_NS); + u64 cur = load(_effective_cadence_ns); + store(_effective_cadence_ns, + step >= cur ? MIN_EFFECTIVE_CADENCE_NS + : std::max(cur - step, MIN_EFFECTIVE_CADENCE_NS)); } // overflow == 0: the budget clamp alone fully absorbed this pass's correction - leave the cadence // at its current value. diff --git a/ddprof-lib/src/main/cpp/referenceChains.cpp b/ddprof-lib/src/main/cpp/referenceChains.cpp index 82c6c5b7c2..28337fc06d 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.cpp +++ b/ddprof-lib/src/main/cpp/referenceChains.cpp @@ -304,7 +304,7 @@ Error ReferenceChainTracker::start(Arguments &args) { _pause_target_ms = args._reference_chains_pause_target_ms; _effective_pause_target_ms = _pause_target_ms; _effective_budget = _budget; - _effective_cadence_ns = PASS_CADENCE_NS; + _effective_cadence_ns = PASS_CADENCE_NS; // plain: the BFS thread is not running yet _candidate_count = 0; _candidate_found_bits = 0; memset(_candidate_discovered_count, 0, sizeof(_candidate_discovered_count)); @@ -517,7 +517,7 @@ void ReferenceChainTracker::threadLoop() { return; } DEBUG_ONLY(rcDebugLevelRefresh(true)); // apply the override file before the first log line - TEST_LOG_SUMMARY("ReferenceChainTracker::threadLoop started, cadence=%lluns rc_debug_level=%d", (unsigned long long)_effective_cadence_ns, rcDebugLevel()); + TEST_LOG_SUMMARY("ReferenceChainTracker::threadLoop started, cadence=%lluns rc_debug_level=%d", (unsigned long long)load(_effective_cadence_ns), rcDebugLevel()); int iteration = 0; while (_running.load(std::memory_order_acquire)) { @@ -526,7 +526,7 @@ void ReferenceChainTracker::threadLoop() { double seconds_to_oom = LivenessTracker::instance()->secondsToOOM(); bool urgent = seconds_to_oom >= 0 && seconds_to_oom < OOM_RAMP_START_S; long target_ms = _pause_target_ms; - u64 cadence_ns = _effective_cadence_ns; + u64 cadence_ns = load(_effective_cadence_ns); if (urgent) { double x = 1.0 - seconds_to_oom / OOM_RAMP_START_S; // 0 at 30min out, 1 at OOM target_ms = std::lround(_pause_target_ms * @@ -538,7 +538,7 @@ void ReferenceChainTracker::threadLoop() { std::pow((double)URGENT_CADENCE_NS / (double)PASS_CADENCE_NS, x)); // While urgent, the ramp owns _effective_cadence_ns outright so shouldRunPass()'s cadence // gate and the per-pass log actually reflect it. - _effective_cadence_ns = cadence_ns; + store(_effective_cadence_ns, cadence_ns); } if (target_ms != _effective_pause_target_ms) { _effective_pause_target_ms = target_ms; @@ -615,9 +615,9 @@ void ReferenceChainTracker::threadLoop() { "passesRun=%d effectiveCadenceNs=%llu effectiveBudget=%d gcFinishEpoch=%llu " "lastPassGcFinishEpoch=%llu nowMinusLastPassNs=%llu", ++iteration, should_run, (int)_search_state, _passes_run, - (unsigned long long)_effective_cadence_ns, _effective_budget, + (unsigned long long)load(_effective_cadence_ns), _effective_budget, (unsigned long long)gcFinishEpoch(), (unsigned long long)_last_pass_gc_finish_epoch, - (unsigned long long)(now_ns - _last_pass_ns)); + (unsigned long long)(now_ns - load(_last_pass_ns))); runPassSerialized(jvmti, jni); } finishLoopIterationSerialized(jvmti, jni); @@ -705,6 +705,18 @@ bool ReferenceChainTracker::shouldRunPass(u64 now_ns) { // _candidate_found_bits. bool canary_active = _candidate_count > 0 && __builtin_popcountll(_candidate_found_bits) < (u64)_candidate_count; + // Minimum pass spacing on every authorization lane (GC epoch, canary back-to-back, fallback + // cadence): a completed GC must not bypass the pacing, and the canary lane's mult==1 + // back-to-back spacing would otherwise run passes with no gap. No OOM-ramp exemption: while + // urgent the ramp owns _effective_cadence_ns outright (threadLoop()) and lowers this floor. + u64 cadence_ns = load(_effective_cadence_ns); + if (now_ns - load(_last_pass_ns) < cadence_ns) { + TEST_LOG_SUMMARY("ReferenceChainTracker::shouldRunPass held off by minimum pass spacing " + "since_last_pass_ms=%llu effective_cadence_ns=%llu", + (unsigned long long)((now_ns - load(_last_pass_ns)) / 1000000ULL), + (unsigned long long)cadence_ns); + return false; + } // Adaptive CPU budget: 100x refill while a canary chase is open - NOT a rate control (the canary // lane's rate is bounded by _canary_backoff_ns's progress-driven exponential backoff, see its own // comment) but a double-throttle guard: the base refill rate is tuned for the ordinary ~1 pass/s @@ -749,26 +761,50 @@ bool ReferenceChainTracker::shouldRunPass(u64 now_ns) { } u64 gc_finish_epoch = gcFinishEpoch(); if (gc_finish_epoch != _last_pass_gc_finish_epoch) { - // Triggering section: "a GC just happened, a pass may be worth running soon". + // Triggering section: "a GC just happened, a pass may be worth running soon". Spacing is + // already enforced by the minimum-pass-spacing gate above - the epoch only selects this pass + // over waiting for the fallback cadence. TEST_LOG_SUMMARY("ReferenceChainTracker::shouldRunPass -> true (gcFinishEpoch=%llu != " "lastPassGcFinishEpoch=%llu)", (unsigned long long)gc_finish_epoch, (unsigned long long)_last_pass_gc_finish_epoch); return true; } - // Pause-time pacing controller: compares against _effective_cadence_ns, not the fixed - // PASS_CADENCE_NS - see that field's own comment (referenceChains.h) for how updatePacing() - // widens or relaxes it from the measured pause-time signal. - bool cadence_elapsed = now_ns - _last_pass_ns >= _effective_cadence_ns; - // Only log when the cadence actually elapsed (a pass will run). - if (cadence_elapsed) { - TEST_LOG_SUMMARY("ReferenceChainTracker::shouldRunPass -> true (now_ns=%llu last_pass_ns=%llu " - "delta=%llu effectiveCadenceNs=%llu)", - (unsigned long long)now_ns, (unsigned long long)_last_pass_ns, - (unsigned long long)(now_ns - _last_pass_ns), - (unsigned long long)_effective_cadence_ns); - } - return cadence_elapsed; + // The fallback cadence has elapsed - the minimum-spacing gate above guarantees the gap, and this + // branch authorizes a pass even with no new GC epoch. updatePacing() widens or relaxes + // _effective_cadence_ns from the measured pause-time signal - see its own comment. + TEST_LOG_SUMMARY("ReferenceChainTracker::shouldRunPass -> true (now_ns=%llu last_pass_ns=%llu " + "delta=%llu effectiveCadenceNs=%llu)", + (unsigned long long)now_ns, (unsigned long long)load(_last_pass_ns), + (unsigned long long)(now_ns - load(_last_pass_ns)), + (unsigned long long)cadence_ns); + return true; +} + +// Wall-clock interval the next root/stack-ref enumeration attempt must wait out - see the +// ROOT_ENUM_* constants' comments (referenceChains.h) for why the floor extends with the last +// attempt's measured in-safepoint cost and why it is capped. +u64 ReferenceChainTracker::rootEnumIntervalFloorNs() const { + u64 interval_ns = ROOT_ENUM_MIN_INTERVAL_NS; + if (_last_root_enum_safepoint_ms > 0) { + u64 scaled_ns = std::min( + _last_root_enum_safepoint_ms * ROOT_ENUM_INTERVAL_NS_PER_COST_MS, + ROOT_ENUM_MAX_INTERVAL_NS); + interval_ns = std::max(interval_ns, scaled_ns); + } + return interval_ns; +} + +// See the declaration's comment (referenceChains.h). +bool ReferenceChainTracker::shouldRunRootEnum(u64 now_ns, + bool manual_first_pass) const { + // manual_first_pass is checked ahead of the interval: _last_root_enum_ns is 0 before the first + // attempt, and an agent starting early in the JVM's lifetime would otherwise see + // now_ns - 0 fall below the interval floor and skip the search's only mandatory enumeration. + if (manual_first_pass) { + return true; + } + return now_ns - _last_root_enum_ns >= rootEnumIntervalFloorNs(); } // Search restart gate (this class's own header comment). Deliberately a probe (max=1) rather than @@ -1065,6 +1101,15 @@ void ReferenceChainTracker::resetSearchStateForTest(jvmtiEnv *jvmti, _last_resolved_class_count = 0; _last_static_field_class_count = -1; _static_field_sweep_cursor = 0; + // Same contract for the pacing/enumeration state the scheduling gates read: an earlier test's + // setPassSpacingForTest() cadence (possibly stretched to tens of seconds) or root-enumeration + // cost would otherwise throttle every later test's BFS thread for that test's whole lifetime. + // The gate's other input, _last_pass_ns, is already zeroed above next to _passes_run, so after + // this reset now - _last_pass_ns >= _effective_cadence_ns holds immediately and the + // minimum-spacing gate is open. Plain write: the BFS thread is stopped here. + _effective_cadence_ns = PASS_CADENCE_NS; + _last_root_enum_ns = 0; + _last_root_enum_safepoint_ms = 0; _static_field_sweep_cycle_truncated = false; _candidate_count = 0; _candidate_found_bits = 0; diff --git a/ddprof-lib/src/main/cpp/referenceChains.h b/ddprof-lib/src/main/cpp/referenceChains.h index 9e2ba53637..c4c09ab099 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -270,7 +270,11 @@ class ReferenceChainTracker { // against, replacing the fixed PASS_CADENCE_NS constant below in that role once updatePacing() // starts adjusting it - see PASS_CADENCE_NS's own comment for why that constant survives as this // field's starting value rather than being deleted outright. - u64 _effective_cadence_ns; + // Written by the BFS thread (threadLoop()'s urgency ramp, outside _engine_lock) and by + // runPass()/updatePacing() and test seams on other threads: every access that can overlap + // another thread's goes through arch.h load()/store(). Plain access only where the BFS thread + // is not running (start(), resetSearchStateForTest()). + volatile u64 _effective_cadence_ns; // Budget-borrowing: extra headroom updatePacing() has temporarily granted above _budget's own // ceiling, earned by a sustained run of comfortably- under-target passes (see @@ -666,10 +670,22 @@ class ReferenceChainTracker { static constexpr int AUTO_FIRST_PASS_BUDGET_MULTIPLIER = 50; static constexpr int AUTO_FIRST_PASS_BUDGET_CAP = 200000; - // Minimum wall-clock gap between root/stack-ref enumeration attempts (runPassManualWalk()'s - // IterateOverReachableObjects call) after the search's own first pass. + // Minimum wall-clock gap between root/stack-ref enumeration attempts + // (runPassManualWalk()'s IterateOverReachableObjects call). Every attempt deoptimizes, on + // every thread, each compiled frame whose scope holds a scalar-replaced (EA-eliminated) + // object - one VM_DeoptimizeFrame per frame, and the JIT re-creates those frames between + // attempts - so a truncated attempt must not retry on the very next pass: that would turn + // every pass into a deopt-all pass. static constexpr u64 ROOT_ENUM_MIN_INTERVAL_NS = 2000000000ULL; // 2s + // Interval extension per millisecond of measured in-safepoint cost of the last attempt: an + // attempt that held the VM for 100ms pushes the next one out by a further 2s; the cap bounds it. + static constexpr u64 ROOT_ENUM_INTERVAL_NS_PER_COST_MS = 20000000ULL; // 20ms per cost ms + + // Upper bound: only enumeration admits new threads and JNI globals, so an expensive attempt + // stretches the interval but never disables it. + static constexpr u64 ROOT_ENUM_MAX_INTERVAL_NS = 30000000000ULL; // 30s + // Pause-time pacing controller: bounds and conversion constants for updatePacing()'s // budget/cadence adjustment - see that method's own comment for the full mechanism. static constexpr int MIN_EFFECTIVE_BUDGET = 2000; @@ -726,12 +742,11 @@ class ReferenceChainTracker { // first pass). u64 _last_root_enum_ns = 0; - // Set true when the most recent root/stack-ref enumeration attempt ended via BUDGET_EXHAUSTED - // (not FRONTIER_CAP_HIT, which stops admitting new frontier entries but leaves the search RUNNING - // - see runPass()'s frontier_cap_hit handling) - runPass() treats this as grounds to retry root - // enumeration on the very next pass regardless of ROOT_ENUM_MIN_INTERVAL_NS, so a - // still-incomplete attempt is not left waiting out the full interval before continuing. - bool _root_enum_truncated_last_time = false; + // In-safepoint milliseconds spent inside the most recent root/stack-ref enumeration attempt, + // including the unconditional all-thread EA deoptimization the JVM performs before its heap + // walk (see ROOT_ENUM_MIN_INTERVAL_NS's comment). Extends the next attempt's interval floor + // (rootEnumIntervalFloorNs()); 0 until the first attempt completes. + u64 _last_root_enum_safepoint_ms = 0; ReferenceChainTracker() : _enabled(false), @@ -777,9 +792,21 @@ class ReferenceChainTracker { } void threadLoop(); - // Runs a pass when the GC-finish epoch advanced or the cadence elapsed. + // Runs a pass when the GC-finish epoch advanced or the cadence elapsed. Both authorizations are + // subject to the minimum pass spacing in shouldRunPass() - see its own comment. bool shouldRunPass(u64 now_ns); + // Wall-clock interval the next root/stack-ref enumeration attempt must wait out since + // _last_root_enum_ns: ROOT_ENUM_MIN_INTERVAL_NS extended by the measured in-safepoint cost of + // the last attempt (ROOT_ENUM_INTERVAL_NS_PER_COST_MS), capped at ROOT_ENUM_MAX_INTERVAL_NS. + u64 rootEnumIntervalFloorNs() const; + + // Whether runPassManualWalk() should run root/stack-ref enumeration this pass: always on the + // search's first pass, otherwise only once rootEnumIntervalFloorNs() above has elapsed since + // _last_root_enum_ns, the last attempt (truncated attempts included - see + // ROOT_ENUM_MIN_INTERVAL_NS's comment for why retries get no fast lane). + bool shouldRunRootEnum(u64 now_ns, bool manual_first_pass) const; + // Cheap probe (max=1, not the real poll pollWatchedTargets() makes) into LivenessTracker's // population-trend table: true if at least one klass shows a positive population slope worth // chasing. Always false when no leak-signal source (generations or liveheap tracking) is @@ -1166,7 +1193,73 @@ class ReferenceChainTracker { int passesSinceLastCandidateProgressForTest() const { return _passes_since_last_candidate_progress; } int canaryStuckRestartCountForTest() const { return _canary_stuck_restart_count; } void setCandidateFoundBitsForTest(u64 bits) { _candidate_found_bits = bits; } - void setSearchStartedForTest(bool v) { _search_started = v; } + // Engine lock serializes with shouldRunPass()/runPass(); the BFS thread also touches + // _effective_cadence_ns outside the lock (threadLoop()'s urgency ramp and logs), so both fields + // use store()/load() everywhere they can overlap. While urgent the ramp overwrites a seeded + // cadence, which is why tests retry their seeds. + void setPassSpacingForTest(u64 ms_since_last_pass, u64 cadence_ms) { + MutexLocker engine_guard(_engine_lock); + store(_last_pass_ns, OS::nanotime() - ms_since_last_pass * 1000000ULL); + store(_effective_cadence_ns, cadence_ms * 1000000ULL); + } + // Raw-value sibling of setPassSpacingForTest() (same lock/store discipline - see its comment): + // places the previous pass's end at an absolute OS::nanotime() value. + void setLastPassNsForTest(u64 ns) { + MutexLocker engine_guard(_engine_lock); + store(_last_pass_ns, ns); + } + // Shifts _last_pass_gc_finish_epoch relative to the live gcFinishEpoch() counter: negative + // looks advanced, positive looks stale, without racing the real GC callbacks. A negative delta + // larger than the live counter clamps to 0 instead of wrapping - a wrap would look stale. + void shiftLastPassGcEpochForTest(long delta) { + u64 epoch = gcFinishEpoch(); + _last_pass_gc_finish_epoch = + delta < 0 ? (epoch >= (u64)(-delta) ? epoch - (u64)(-delta) : 0) + : epoch + (u64)delta; + } + // Seeds shouldRunRootEnum()'s inputs. Engine lock held: both fields are otherwise + // BFS-thread-owned (runPassManualWalk()). Negative ms_since_last_enum clamps to 0 (never ran). + void setRootEnumStateForTest(long ms_since_last_enum, u64 last_enum_cost_ms) { + MutexLocker engine_guard(_engine_lock); + _last_root_enum_ns = ms_since_last_enum < 0 + ? 0 + : OS::nanotime() - (u64)ms_since_last_enum * 1000000ULL; + _last_root_enum_safepoint_ms = last_enum_cost_ms; + } + bool shouldRunRootEnumForTest(u64 now_ns, bool manual_first_pass) const { + MutexLocker engine_guard(_engine_lock); + return shouldRunRootEnum(now_ns, manual_first_pass); + } + u64 rootEnumIntervalFloorNsForTest() const { + MutexLocker engine_guard(_engine_lock); + return rootEnumIntervalFloorNs(); + } + // Engine lock is the happens-before edge: every reader of _candidate_count/_candidate_found_bits + // runs under _engine_lock (shouldRunPass(), runPass()/runPassSerialized(), pollWatchedTargets()), + // so the plain writes need no atomics. + void setCanaryChaseStateForTest(int candidate_count, u64 found_bits) { + MutexLocker engine_guard(_engine_lock); + _candidate_count = candidate_count; + _candidate_found_bits = found_bits; + } + void setSearchStartedForTest(bool v) { + MutexLocker engine_guard(_engine_lock); + _search_started = v; + } + // Forces a started, RUNNING search with fresh pain budgets so shouldRunPassForTest() evaluates + // the running-search gates deterministically; a concurrently passing BFS thread can still drive + // the real search terminal in the seed-to-call window - tests retry the seed. + // _tags_released is deliberately left untouched: it is only consulted once the search is + // terminal, and the production RUNNING->terminal transition recomputes it from + // releaseSearchTags(). Forcing it true could let restartSearch() rewind _next_tag over live tags. + void forceRunningSearchForTest() { + MutexLocker engine_guard(_engine_lock); + _search_started = true; + store(_search_state, (u8)SearchState::RUNNING); + store(_abandon_reason, (u8)SearchAbandonReason::NONE); + _cpu_pain_budget = PainBudget(_pain_budget_refill_rate); + _search_pain_ms = 0; + } // OOM-urgency hysteresis state (isUrgent()'s latch/release machine, // referenceChains.cpp): tests drive it via isUrgentForTest() and assert on // the raw latch/tick reads. diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index 1f91d50f0c..b429f66c9e 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -791,6 +791,74 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l */ static native boolean urgentLatchedForTest0(); + /** + * Test seam (debug native builds only): places the previous reference-chain pass + * {@code msSinceLastPass} milliseconds in the past and sets the effective cadence to + * {@code cadenceMs}, so {@link #shouldRunPassForTest0()} evaluates its minimum-pass-spacing + * gate against a controlled spacing without sleeping. Values persist until the next call or + * the next real BFS pass overwrites them. + */ + static native void setReferenceChainPassSpacingForTest0(long msSinceLastPass, + long cadenceMs); + + /** + * Test seam (debug native builds only): shifts ReferenceChainTracker's last-pass GC-epoch + * snapshot by {@code delta} relative to the live epoch counter, so a test can make the epoch + * look advanced (negative delta - "a GC happened since the last pass") or stale (positive + * delta) for {@link #shouldRunPassForTest0()} without racing the real GC callbacks. + */ + static native void shiftReferenceChainLastGcEpochForTest0(long delta); + + /** + * Test seam (debug native builds only): sets ReferenceChainTracker's root-enumeration + * scheduling state - the last attempt {@code msSinceLastEnum} milliseconds in the past + * (negative means "never ran") and its measured in-safepoint cost + * {@code lastEnumCostMs}, which the adaptive interval floor extends. Feeds + * {@link #shouldRunRootEnumForTest0(boolean)}. + */ + static native void setReferenceChainRootEnumStateForTest0(long msSinceLastEnum, + long lastEnumCostMs); + + /** + * Test seam (debug native builds only): evaluates whether ReferenceChainTracker's next pass + * would run root/stack-ref enumeration ({@code shouldRunRootEnum()}) with the current + * scheduling state; {@code manualFirstPass} forces the search's always-enumerate first pass. + * Pure predicate - no JVMTI call is made. + */ + static native boolean shouldRunRootEnumForTest0(boolean manualFirstPass); + + /** + * Test seam (debug native builds only): the current adaptive root-enumeration interval floor + * in nanoseconds ({@code ROOT_ENUM_MIN_INTERVAL_NS} extended by the last attempt's measured + * in-safepoint cost, capped) - the value {@link #shouldRunRootEnumForTest0(boolean)} compares + * against. Zero in release builds. + */ + static native long rootEnumIntervalFloorNsForTest0(); + + /** + * Test seam (debug native builds only): the live GC-finish epoch ({@code gcFinishEpoch()}, + * advanced by every JVMTI GarbageCollectionFinish), so a test can verify a real GC was + * observed before relying on {@link #shiftReferenceChainLastGcEpochForTest0(long)}. Zero in + * release builds. + */ + static native long referenceChainGcFinishEpochForTest0(); + + /** + * Test seam (debug native builds only): forces the tracker into a started, RUNNING search + * with fresh pain budgets, so {@link #shouldRunPassForTest0()} evaluates the running-search + * scheduling gates deterministically even when the live BFS thread is concurrently running + * (and possibly terminating) its own real search. + */ + static native void forceRunningReferenceChainSearchForTest0(); + + /** + * Test seam (debug native builds only): sets the canary chase's candidate count and + * found-bits mask so {@link #shouldRunPassForTest0()} evaluates the canary lane - with at + * least one candidate and not all found, the gate takes the candidate-chase backoff path. + */ + static native void setReferenceChainCanaryStateForTest0(int candidateCount, + long foundBits); + // ---- Test-only reads of the current thread's OTEP record ---------------------------------- // Each resolves the current carrier's record directly (like the write primitives above) with // no cached buffer and no per-thread Java object; introspection/test use only. diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java index 6dde526f50..8f9c19bbbe 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -40,7 +40,11 @@ public void ownedBlockHooksAreNotPublicApiBeforeTaskBlockInstrumentation() throw "heapFloorResetForTest0", "heapFloorRisingForTest0", "shouldRunPassForTest0", "referenceChainPassesRunForTest0", "backdateLeakSignalActivationForTest0", "setLeakSignalActivatedNsForTest0", - "secondsToOOMForTest0", "urgentLatchedForTest0")); + "secondsToOOMForTest0", "urgentLatchedForTest0", + "setReferenceChainPassSpacingForTest0", "shiftReferenceChainLastGcEpochForTest0", + "setReferenceChainRootEnumStateForTest0", "shouldRunRootEnumForTest0", + "rootEnumIntervalFloorNsForTest0", "referenceChainGcFinishEpochForTest0", + "setReferenceChainCanaryStateForTest0", "forceRunningReferenceChainSearchForTest0")); @Test public void referenceChainTestSeamsAreNotPublicApi() { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java index 285894c7a5..8f5b616aa6 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java @@ -50,6 +50,38 @@ public static boolean runReferenceChainPass0() { return JavaProfiler.runReferenceChainPass0(); } + public static void setReferenceChainPassSpacingForTest0(long msSinceLastPass, long cadenceMs) { + JavaProfiler.setReferenceChainPassSpacingForTest0(msSinceLastPass, cadenceMs); + } + + public static void shiftReferenceChainLastGcEpochForTest0(long delta) { + JavaProfiler.shiftReferenceChainLastGcEpochForTest0(delta); + } + + public static void setReferenceChainRootEnumStateForTest0(long msSinceLastEnum, long lastEnumCostMs) { + JavaProfiler.setReferenceChainRootEnumStateForTest0(msSinceLastEnum, lastEnumCostMs); + } + + public static boolean shouldRunRootEnumForTest0(boolean manualFirstPass) { + return JavaProfiler.shouldRunRootEnumForTest0(manualFirstPass); + } + + public static long rootEnumIntervalFloorNsForTest0() { + return JavaProfiler.rootEnumIntervalFloorNsForTest0(); + } + + public static long referenceChainGcFinishEpochForTest0() { + return JavaProfiler.referenceChainGcFinishEpochForTest0(); + } + + public static void forceRunningReferenceChainSearchForTest0() { + JavaProfiler.forceRunningReferenceChainSearchForTest0(); + } + + public static void setReferenceChainCanaryStateForTest0(int candidateCount, long foundBits) { + JavaProfiler.setReferenceChainCanaryStateForTest0(candidateCount, foundBits); + } + public static void pollReferenceChainTargets0() { JavaProfiler.pollReferenceChainTargets0(); } From 674de1e6c4010fe41cc1e7bb9b612ac7e795f4a9 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 30 Oct 2026 16:33:51 +0100 Subject: [PATCH 2/4] test: pin reference-chain scheduling gates Add ReferenceChainSchedulingTest over the spacing and root-enum gates (open and closed directions, positive controls for the negated probes, verified GC-epoch advancement). Backdate pass spacing in the canary-lane and refill gtests for the new gate; add boundary tests for the root-enum interval edge and the GC-epoch shift clamp. Share assumeDebugBuild() via ReferenceChainAssertions. --- .../src/test/cpp/referenceChainsBfsTests.inc | 189 +++++++- .../src/test/cpp/referenceChainsMockHeap.h | 11 + .../src/test/cpp/referenceChainsOomTests.inc | 4 + .../test/cpp/referenceChainsTestAccessors.h | 49 ++ .../AggressiveLeakReferenceChainTest.java | 8 +- .../ReferenceChainAssertions.java | 12 + .../ReferenceChainSchedulingTest.java | 432 ++++++++++++++++++ .../ReferenceChainTestSeamsTest.java | 41 +- 8 files changed, 695 insertions(+), 51 deletions(-) create mode 100644 ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainSchedulingTest.java diff --git a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc index 96c82590e2..d282148a2d 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc @@ -850,6 +850,11 @@ TEST_F(ReferenceChainsBfsTest, CanaryLaneBacksOffWithoutProgressAndResetsOnProgr // progress and the multiplier stays at 1 - back-to-back. ASSERT_TRUE(tracker->runPass(&mock_jvmti, &mock_jni, &truncated)); EXPECT_EQ(1, ReferenceChainsTestAccessor::canaryBackoffMult()); + // Backdate the previous pass's end past the minimum-spacing gate (shouldRunPass() enforces one + // effective cadence between passes on every lane) so the assertions below exercise the canary + // lane's own backoff, not the spacing gate. + ReferenceChainsTestAccessor::setLastPassNsForTest( + OS::nanotime() - ReferenceChainsTestAccessor::baselineCadenceNs() - 1); EXPECT_TRUE(ReferenceChainsTestAccessor::shouldRunPass(OS::nanotime())) << "a fresh chase must be allowed to run back-to-back"; @@ -858,16 +863,26 @@ TEST_F(ReferenceChainsBfsTest, CanaryLaneBacksOffWithoutProgressAndResetsOnProgr EXPECT_EQ(2, ReferenceChainsTestAccessor::canaryBackoffMult()); ReferenceChainsTestAccessor::setCanaryBackoffForTest( /*mult=*/2, /*ema_ms=*/100, OS::nanotime()); + ReferenceChainsTestAccessor::setLastPassNsForTest( + OS::nanotime() - ReferenceChainsTestAccessor::baselineCadenceNs() - 1); EXPECT_FALSE(ReferenceChainsTestAccessor::shouldRunPass(OS::nanotime())) << "a no-progress canary pass must hold off the next one"; // Beyond the spacing, the chase is allowed again - the backoff paces, it never abandons. - EXPECT_TRUE(ReferenceChainsTestAccessor::shouldRunPass( - ReferenceChainsTestAccessor::lastCanaryPassNs() + - 2ULL * 100ULL * 1000000ULL + 1)) + u64 resume_ns = ReferenceChainsTestAccessor::lastCanaryPassNs() + + 2ULL * 100ULL * 1000000ULL + 1; + ReferenceChainsTestAccessor::setLastPassNsForTest( + resume_ns - ReferenceChainsTestAccessor::baselineCadenceNs() - 1); + EXPECT_TRUE(ReferenceChainsTestAccessor::shouldRunPass(resume_ns)) << "elapsed spacing must re-admit the canary pass"; - // The OOM urgency ramp overrides the backoff gate entirely. + // The OOM urgency ramp overrides the backoff gate entirely. Mirror the ramp's production + // effect on the cadence (threadLoop() drives _effective_cadence_ns down with urgency) so the + // minimum-spacing gate opens the way it does under a live ramp. ReferenceChainsTestAccessor::setOomRampActive(true); + ReferenceChainsTestAccessor::setEffectiveCadenceNs( + ReferenceChainsTestAccessor::minEffectiveCadenceNs()); + ReferenceChainsTestAccessor::setLastPassNsForTest( + OS::nanotime() - ReferenceChainsTestAccessor::minEffectiveCadenceNs() - 1); EXPECT_TRUE(ReferenceChainsTestAccessor::shouldRunPass(OS::nanotime())) << "urgency must bypass the canary backoff"; ReferenceChainsTestAccessor::setOomRampActive(false); @@ -890,11 +905,177 @@ TEST_F(ReferenceChainsBfsTest, CanaryLaneBacksOffWithoutProgressAndResetsOnProgr ASSERT_TRUE(tracker->runPass(&mock_jvmti, &mock_jni, &truncated)); EXPECT_EQ(1, ReferenceChainsTestAccessor::canaryBackoffMult()) << "candidate progress must reset the spacing multiplier to 1"; + ReferenceChainsTestAccessor::setLastPassNsForTest( + OS::nanotime() - ReferenceChainsTestAccessor::baselineCadenceNs() - 1); EXPECT_TRUE(ReferenceChainsTestAccessor::shouldRunPass(OS::nanotime())); tracker->stop(); } +// forceRunningSearchForTest() forces RUNNING for the scheduling tests but must leave +// _tags_released at its real value: that flag is only consulted once the search is terminal, and +// a fabricated "true" would hide a still-pending release and let restartSearch() rewind _next_tag +// over live tags in this process-wide singleton. +TEST_F(ReferenceChainsBfsTest, ForceRunningSearchLeavesTagsReleasedUntouched) { + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + + // Fixture default (_tags_released = true): the seam must not clear it either. + ASSERT_TRUE(ReferenceChainsTestAccessor::tagsReleased()); + tracker->forceRunningSearchForTest(); + EXPECT_TRUE(ReferenceChainsTestAccessor::tagsReleased()); + EXPECT_EQ(SearchState::RUNNING, tracker->searchState()); + + // A RUNNING search with unreleased tags. + ReferenceChainsTestAccessor::setInFlightSearchForTest(OS::nanotime()); + ASSERT_FALSE(ReferenceChainsTestAccessor::tagsReleased()); + tracker->forceRunningSearchForTest(); + EXPECT_FALSE(ReferenceChainsTestAccessor::tagsReleased()) + << "a test seam must never fabricate a completed tag release"; + EXPECT_EQ(SearchState::RUNNING, tracker->searchState()); + // The next test's fixture reset restores _tags_released / the search state. +} + +// _effective_cadence_ns and _last_pass_ns are written by the spacing seams from a test thread +// while shouldRunPass() and updatePacing() read (and write) them on another thread without +// _engine_lock - the shape of the JNI/gtest seam paths. Every such access must be an atomic +// load()/store(); under the tsan native config a plain access reports a data race and fails the +// run. Without TSan this is a smoke test (the cadence must stay sane). +TEST_F(ReferenceChainsBfsTest, PassSpacingSeamsDoNotRaceSchedulingReaders) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); // start() spawns no BFS thread + tracker->forceRunningSearchForTest(); + ReferenceChainsTestAccessor::setCandidateCountForTest(0); + const u64 saved_cadence = ReferenceChainsTestAccessor::effectiveCadenceNs(); + const int saved_budget = ReferenceChainsTestAccessor::effectiveBudget(); + + const u64 deadline = OS::nanotime() + 200ULL * 1000000ULL; + std::atomic writer_done(false); + std::thread writer([&]() { + for (u64 i = 0; i < 20000 && OS::nanotime() < deadline; i++) { + if ((i & 1) == 0) { + tracker->setPassSpacingForTest(i % 3, 1 + i % 5); + } else { + ReferenceChainsTestAccessor::setLastPassNsForTest(OS::nanotime() - i); + } + } + writer_done.store(true); + }); + for (u64 i = 0; !writer_done.load() || i < 100; i++) { + ReferenceChainsTestAccessor::shouldRunPass(OS::nanotime()); + ReferenceChainsTestAccessor::updatePacing(1000000ULL * (i % 50)); + if (OS::nanotime() >= deadline && writer_done.load()) { + break; + } + } + writer.join(); + + EXPECT_GT(ReferenceChainsTestAccessor::effectiveCadenceNs(), 0u); + + ReferenceChainsTestAccessor::setEffectiveCadenceNs(saved_cadence); + ReferenceChainsTestAccessor::setEffectiveBudget(saved_budget); + ReferenceChainsTestAccessor::resetPacingController(); + tracker->stop(); +} + +// runPassManualWalk() converts the root-enumeration call's measured OS::nanotime() delta to ms and +// stores it as the next attempt's cost (_last_root_enum_safepoint_ms); rootEnumIntervalFloorNs() +// extends ROOT_ENUM_MIN_INTERVAL_NS by ROOT_ENUM_INTERVAL_NS_PER_COST_MS per cost-ms, capped at +// ROOT_ENUM_MAX_INTERVAL_NS. A 150ms enumeration must therefore land the floor in [3s, 30s): an +// unscaled ns value saturates the cap, a wrong scale or a dropped assignment leaves it at MIN. +TEST_F(ReferenceChainsBfsTest, RootEnumMeasuredCostExtendsIntervalFloor) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + // The fixture reset does not touch the root-enumeration state: clear any residue. + std::pair saved = ReferenceChainsTestAccessor::setRootEnumRawStateForTest(0, 0); + // Vacuity guard: before any attempt the floor sits at the minimum. + ASSERT_EQ(ReferenceChainsTestAccessor::rootEnumMinIntervalNs(), + tracker->rootEnumIntervalFloorNsForTest()); + + int n = addNode(); + script = { + {JVMTI_HEAP_REFERENCE_JNI_GLOBAL, -1, n, -1}, + }; + + const u64 kRootEnumDelayMs = 150; + ior_delay_ns = kRootEnumDelayMs * 1000000ULL; + bool truncated = true; + // First pass of the search: root enumeration runs (the only path that tags n). + ASSERT_TRUE(tracker->runPass(&mock_jvmti, &mock_jni, &truncated)); + ior_delay_ns = 0; + EXPECT_NE(0, tags_ever_assigned[n]) << "root enumeration must have run"; + + u64 floor_ns = tracker->rootEnumIntervalFloorNsForTest(); + // >= 150 x ROOT_ENUM_INTERVAL_NS_PER_COST_MS (3s): the ns->ms conversion reached the floor. + EXPECT_GE(floor_ns, kRootEnumDelayMs * ReferenceChainsTestAccessor::rootEnumNsPerCostMs()) + << "measured root-enum cost must extend the interval floor (ns->ms conversion)"; + // > ROOT_ENUM_MIN_INTERVAL_NS: a /1e9 scale or a dropped assignment leaves it at MIN. + EXPECT_GT(floor_ns, ReferenceChainsTestAccessor::rootEnumMinIntervalNs()); + // < ROOT_ENUM_MAX_INTERVAL_NS: an unscaled ns cost would saturate the cap. + EXPECT_LT(floor_ns, ReferenceChainsTestAccessor::rootEnumMaxIntervalNs()) + << "a 150ms attempt must not saturate the cap (unscaled ns would)"; + + ReferenceChainsTestAccessor::setRootEnumRawStateForTest(saved.first, saved.second); + tracker->stop(); +} + +// shouldRunRootEnum()'s interval check is inclusive (now - _last_root_enum_ns >= floor), both at +// the base floor and at a cost-extended floor; the manual first pass bypasses it. +TEST_F(ReferenceChainsBfsTest, RootEnumIntervalBoundaryIsInclusive) { + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + const u64 kMin = ReferenceChainsTestAccessor::rootEnumMinIntervalNs(); + // Far from 0 (the never-ran value) and from wrapping. + const u64 kLast = 100 * kMin; + std::pair saved = ReferenceChainsTestAccessor::setRootEnumRawStateForTest(kLast, 0); + ASSERT_EQ(kMin, tracker->rootEnumIntervalFloorNsForTest()); + + EXPECT_TRUE(tracker->shouldRunRootEnumForTest(kLast + kMin, false)) + << "the base interval boundary must be inclusive (>=, not >)"; + EXPECT_FALSE(tracker->shouldRunRootEnumForTest(kLast + kMin - 1, false)) + << "one ns short of the base interval must still hold enumeration off"; + EXPECT_TRUE(tracker->shouldRunRootEnumForTest(kLast + kMin - 1, true)) + << "the search's first pass must bypass the interval"; + + const u64 kCostMs = 500; + ReferenceChainsTestAccessor::setRootEnumRawStateForTest(kLast, kCostMs); + const u64 kExtended = kCostMs * ReferenceChainsTestAccessor::rootEnumNsPerCostMs(); + ASSERT_EQ(kExtended, tracker->rootEnumIntervalFloorNsForTest()); + EXPECT_TRUE(tracker->shouldRunRootEnumForTest(kLast + kExtended, false)) + << "the cost-extended boundary must be inclusive"; + EXPECT_FALSE(tracker->shouldRunRootEnumForTest(kLast + kExtended - 1, false)) + << "one ns short of the cost-extended interval must still hold enumeration off"; + + // Mandatory: the fixture reset does not restore these fields. + ReferenceChainsTestAccessor::setRootEnumRawStateForTest(saved.first, saved.second); +} + +// shiftLastPassGcEpochForTest() shifts the last-pass epoch snapshot relative to the live counter, +// clamping a negative shift larger than the live counter to 0 (the never-ran value) instead of +// wrapping. The `epoch >= n` -> `epoch > n` mutant is equivalent (at epoch == n both branches +// yield 0), so no row can kill it. +TEST_F(ReferenceChainsBfsTest, ShiftLastPassGcEpochClampsAtZero) { + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + const u64 saved = tracker->gcFinishEpoch(); + struct Row { + u64 live; + long shift; + u64 expected; + }; + const Row rows[] = { + {0, -1, 0}, {5, -1, 4}, {5, -5, 0}, {5, -6, 0}, {5, 3, 8}, + }; + for (const Row &r : rows) { + ReferenceChainsTestAccessor::setGcFinishEpochForTest(r.live); + tracker->shiftLastPassGcEpochForTest(r.shift); + EXPECT_EQ(r.expected, ReferenceChainsTestAccessor::lastPassGcFinishEpoch()) + << "live=" << r.live << " shift=" << r.shift; + } + ReferenceChainsTestAccessor::setGcFinishEpochForTest(saved); +} + // With no leak-signal source active (neither generations nor liveheap // tracking), the collector must stay completely idle: no first pass and no // restart, ever. Searching without a signal runs whole-heap BFS passes with diff --git a/ddprof-lib/src/test/cpp/referenceChainsMockHeap.h b/ddprof-lib/src/test/cpp/referenceChainsMockHeap.h index bd0c4b2d95..49cffdaeef 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsMockHeap.h +++ b/ddprof-lib/src/test/cpp/referenceChainsMockHeap.h @@ -97,6 +97,11 @@ class ReferenceChainsMockHeap { // When non-zero, mock_GetObjectsWithTags() below busy-waits this many nanoseconds. u64 gotw_delay_ns = 0; + // When non-zero, mock_IterateOverReachableObjects() below busy-waits this many nanoseconds + // before reporting roots - a deterministic in-safepoint cost for root enumeration, measured + // by runPassManualWalk() into _last_root_enum_safepoint_ms. + u64 ior_delay_ns = 0; + // Synthetic frontier-holder arrays for expandFrontier()'s array-holder walk: // mock_NewObjectArray() hands back an opaque handle, mock_SetObjectArrayElement() records its // elements here, and mock_FollowReferences() treats every recorded element as an expansion seed @@ -428,6 +433,12 @@ class ReferenceChainsMockHeap { jvmtiEnv *, jvmtiHeapRootCallback heap_root_cb, jvmtiStackReferenceCallback, jvmtiObjectReferenceCallback, const void *user_data) { + if (active_fixture->ior_delay_ns != 0) { + u64 until = OS::nanotime() + active_fixture->ior_delay_ns; + while (OS::nanotime() < until) { + // busy-wait (see gotw_delay_ns): the measured duration must not undershoot. + } + } for (auto &e : active_fixture->script) { if (e.referrer_idx != -1) { continue; diff --git a/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc b/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc index 2080bc541c..3c7d4b0cbd 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc @@ -443,6 +443,10 @@ TEST_F(ReferenceChainsCanaryRefillTest, RefillRaiseTracksOpenChaseExactly) { // The refill logic lives past shouldRunPass()'s first-pass branch, so // mark the search started the way a first runPass() would have. ReferenceChainsTestAccessor::setSearchStartedForTest(true); + // Synthetic now values below start at 1000ns; shrink the cadence so the minimum-spacing gate + // (now_ns - _last_pass_ns >= _effective_cadence_ns, with _last_pass_ns = 0 after reset) stays + // open and the refill assertions exercise the canary lane, not the spacing gate. + ReferenceChainsTestAccessor::setEffectiveCadenceNs(100); double base = ReferenceChainsTestAccessor::basePainBudgetRefillRateForTest(); EXPECT_DOUBLE_EQ(0.01, base); // default painbudget=1 percent diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index 8d5d59ef4e..60fc0b0757 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -14,6 +14,7 @@ #include #include +#include #include #include "classTagAllocator.h" @@ -195,6 +196,16 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::instance()->setSearchStartedForTest(v); } + // Backdates the previous pass's end so shouldRunPass()'s minimum-spacing gate (which compares + // now_ns - _last_pass_ns against _effective_cadence_ns) stays open while a test exercises a + // specific authorization lane (canary backoff, GC epoch) in isolation. The store goes through + // the tracker seam, so it is made under _engine_lock with store() and is ordered with any + // pass; a real pass can still overwrite the seed between the seed and the assert (the same + // limitation as forceRunningSearchForTest()). + static void setLastPassNsForTest(u64 ns) { + ReferenceChainTracker::instance()->setLastPassNsForTest(ns); + } + static u64 searchPainMs() { return ReferenceChainTracker::instance()->_search_pain_ms; } @@ -545,6 +556,10 @@ class ReferenceChainsTestAccessor { static u64 baselineCadenceNs() { return ReferenceChainTracker::PASS_CADENCE_NS; } + static u64 minEffectiveCadenceNs() { + return ReferenceChainTracker::MIN_EFFECTIVE_CADENCE_NS; + } + // Start the controller from a chosen below-ceiling/above-baseline point with a fresh // _pause_pid, so a prior sequence's integral windup cannot leak into per-step assertions. static void setEffectiveBudget(int v) { @@ -559,6 +574,40 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::instance()->_pause_pid.reset(); } + // Root-enumeration raw state (shouldRunRootEnum()'s inputs) at exact values - unlike the + // tracker's setRootEnumStateForTest(), which derives the timestamp from OS::nanotime() and + // so cannot pin a boundary. Engine lock held (static accessor: take it explicitly). Returns + // the previous pair so the caller can restore it - the fixture reset does not zero these. + static std::pair setRootEnumRawStateForTest(u64 last_enum_ns, u64 cost_ms) { + ReferenceChainTracker *t = ReferenceChainTracker::instance(); + MutexLocker engine_guard(t->_engine_lock); + std::pair prev(t->_last_root_enum_ns, t->_last_root_enum_safepoint_ms); + t->_last_root_enum_ns = last_enum_ns; + t->_last_root_enum_safepoint_ms = cost_ms; + return prev; + } + + static std::pair rootEnumRawStateForTest() { + ReferenceChainTracker *t = ReferenceChainTracker::instance(); + MutexLocker engine_guard(t->_engine_lock); + return std::pair(t->_last_root_enum_ns, t->_last_root_enum_safepoint_ms); + } + + static u64 rootEnumMinIntervalNs() { return ReferenceChainTracker::ROOT_ENUM_MIN_INTERVAL_NS; } + static u64 rootEnumNsPerCostMs() { + return ReferenceChainTracker::ROOT_ENUM_INTERVAL_NS_PER_COST_MS; + } + static u64 rootEnumMaxIntervalNs() { return ReferenceChainTracker::ROOT_ENUM_MAX_INTERVAL_NS; } + + // Live GC-finish epoch (relaxed atomic), for the epoch-clamp test - save/restore it; the + // fixture reset zeroes _last_pass_gc_finish_epoch but not _gc_finish_epoch. + static void setGcFinishEpochForTest(u64 v) { + store(ReferenceChainTracker::instance()->_gc_finish_epoch, v); + } + static u64 lastPassGcFinishEpoch() { + return ReferenceChainTracker::instance()->_last_pass_gc_finish_epoch; + } + // Budget-borrowing (see _borrowed_budget in referenceChains.h). static int borrowCeilingMultiplier() { return ReferenceChainTracker::BORROW_CEILING_MULTIPLIER; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java index 3140c2c431..91ac875dd4 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java @@ -12,13 +12,13 @@ import java.util.Arrays; +import static com.datadoghq.profiler.referencechains.ReferenceChainAssertions.assumeDebugBuild; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import static org.junit.jupiter.api.Assumptions.assumeFalse; -import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * E2E coverage for the search-restart gate the reference-chain collector's pass loop polls @@ -83,12 +83,6 @@ protected boolean isPlatformSupported() { return !(Platform.isJavaVersion(8) || Platform.isJ9() || Platform.isZing()); } - private static void assumeDebugBuild() { - assumeTrue(ReferenceChainAssertions.isDebugInstrumentedBuild(), - "heapFloorRecordForTest0/setMaxHeapBytesForTest0/shouldRunPassForTest0 only exist in a " - + "-DDEBUG native build (debug/asan/tsan - javaApi.cpp's #ifdef DEBUG guard)"); - } - /** * Undoes this class's synthetic seeding so later tests in this shared, no-forkEvery JVM never * see it. This helper is the only place the cleanup ordering lives; every test's finally block diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java index aa6f0ba7b1..0eb121e087 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * Shared {@code datadog.ReferenceChain} JFR-parsing helpers. @@ -32,6 +33,17 @@ * via JMC's {@code JfrLoaderToolkit} ({@link #findMatchForClass(IItemCollection, Class)}). */ public final class ReferenceChainAssertions { + + /** + * Skips the calling test unless the native library is a debug-instrumented build - the + * reference-chain {@code *ForTest0} seams exist only under javaApi.cpp's {@code #ifdef DEBUG}. + */ + public static void assumeDebugBuild() { + assumeTrue(isDebugInstrumentedBuild(), + "reference-chain test seams only exist in a -DDEBUG native build (debug/asan/tsan - " + + "javaApi.cpp's #ifdef DEBUG guard)"); + } + private ReferenceChainAssertions() {} /** diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainSchedulingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainSchedulingTest.java new file mode 100644 index 0000000000..fb3ccd4d41 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainSchedulingTest.java @@ -0,0 +1,432 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.referencechains; + +import com.datadoghq.profiler.AbstractProfilerTest; +import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.ReferenceChainTestHooks; +import org.junit.jupiter.api.Test; + +import java.util.function.BooleanSupplier; + +import static com.datadoghq.profiler.referencechains.ReferenceChainAssertions.assumeDebugBuild; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Pins the two scheduling gates that bound the reference-chain tracker's JVMTI heap-walk + * frequency on services with a high young-GC rate: + *
    + *
  • Minimum pass spacing ({@code shouldRunPass()}): every authorization lane - GC + * epoch, canary back-to-back, fallback cadence - waits out {@code _effective_cadence_ns} since + * the previous pass; otherwise pass rate tracks the GC rate.
    • + *
  • Adaptive root-enumeration interval ({@code shouldRunRootEnum()}): re-enumeration + * waits out {@code ROOT_ENUM_MIN_INTERVAL_NS} extended by the last attempt's measured + * in-safepoint cost; otherwise a next-pass retry of a truncated attempt turns every pass into + * a deopt-all pass.
    • + *
+ * + *

Only runs under the debug native build - the backing native methods do not exist in a + * release build, same pattern as {@link ReferenceChainTestSeamsTest}. + */ +public class ReferenceChainSchedulingTest extends AbstractProfilerTest { + + // A cadence far above any timing jitter in these tests: with the spacing gate seeded to "the + // previous pass just ended", the gate stays closed for the whole assertion window, and the BFS + // thread - which reads the same cadence for its idle sleep - stays quiesced while the seeded + // state is in force. + private static final long CADENCE_MS = 10_000; + // Long enough for any BFS iteration already in flight when a test seeds its state to finish + // (post-reset the thread runs no pass: the search is not started and there is no leak signal, + // so an iteration is two atomic loads and a sleep decision). + private static final long QUIESCE_MS = 1_200; + // Java mirrors of referenceChains.h's root-enumeration constants - no seam exposes them: + // ROOT_ENUM_MIN_INTERVAL_NS, ROOT_ENUM_INTERVAL_NS_PER_COST_MS, ROOT_ENUM_MAX_INTERVAL_NS. + // A native retune must update these mirrors. rootEnumBaseIntervalMatchesNative and the + // floor-equality probes below pin all three against the live library, so a stale mirror fails + // loudly instead of silently re-seeding the wrong regime. + private static final long NS_PER_MS = 1_000_000L; + private static final long ROOT_ENUM_MIN_INTERVAL_NS = 2_000_000_000L; + private static final long ROOT_ENUM_NS_PER_COST_MS = 20_000_000L; + private static final long ROOT_ENUM_MAX_INTERVAL_NS = 30_000_000_000L; + // Seed ages either side of the base interval, and costs that extend / cap the floor. + private static final long ROOT_ENUM_BEFORE_BASE_MS = + ROOT_ENUM_MIN_INTERVAL_NS / NS_PER_MS - 1_000; + private static final long ROOT_ENUM_PAST_BASE_MS = + ROOT_ENUM_MIN_INTERVAL_NS / NS_PER_MS + 1_000; + private static final long ROOT_ENUM_EXTENDING_COST_MS = 500; + private static final long ROOT_ENUM_CAPPING_COST_MS = 5_000; + // Seed age just past the cap: with the capping cost the floor is ROOT_ENUM_MAX_INTERVAL_NS, so + // this age must open the gate (kills an uncapped floor). + private static final long ROOT_ENUM_PAST_CAP_MS = + ROOT_ENUM_MAX_INTERVAL_NS / NS_PER_MS + 1_000; + + @Override + protected String getProfilerCommand() { + return "generations=true,referencechains=true:hops=32:budget=500:ttl=60000:framecap=2000000"; + } + + @Override + protected boolean isPlatformSupported() { + return !(Platform.isJavaVersion(8) || Platform.isJ9() || Platform.isZing()); + } + + /** + * Evaluates {@code probe} against re-seeded scheduling state, retrying while a concurrently + * passing BFS thread invalidates the seed: a real pass overwrites {@code _last_pass_ns}, + * {@code _last_pass_gc_finish_epoch}, and the root-enumeration state with live values, and can + * drive the real search terminal, which can only ever flip a seeded-open gate to the closed + * reading - so a closed result is final and only an open/matching result needs the retry. + * The same real pass also produces the closed reading on its own, so a closed-direction probe + * (a negated gate reading) must be bracketed by a check that the seeded state is still in force + * (see {@link #rootEnumClosedWithSeedIntact(long)}) - otherwise a closed result caused by a + * real pass would be accepted instead of retried. + */ + private static boolean eventuallyTrue(String message, Runnable seed, + BooleanSupplier probe) + throws InterruptedException { + for (int i = 0; i < 50; i++) { + seed.run(); + if (probe.getAsBoolean()) { + return true; + } + Thread.sleep(100); + } + assertFalse(true, message + " (state re-seeded 50 times over ~5s, never observed)"); + return false; + } + + /** + * Closed-direction root-enum probe (see eventuallyTrue): a closed gate counts only while the + * seeded floor is still in force. A concurrent real enumeration also closes the gate (it + * refreshes _last_root_enum_ns to "now"), so a bare closed reading cannot tell the seeded floor + * apart from that unrelated close. The two JNI calls take the engine lock separately; a real + * pass landing between them either changes the floor (the equality fails and the probe + * retries) or leaves it unchanged (the closed reading is still what the seeded floor predicts), + * so that window can cause a retry, never a false accept. + */ + private static boolean rootEnumClosedWithSeedIntact(long expectedFloorNs) { + return !ReferenceChainTestHooks.shouldRunRootEnumForTest0(false) + && ReferenceChainTestHooks.rootEnumIntervalFloorNsForTest0() == expectedFloorNs; + } + + /** Seed relations the root-enum tests rely on, checked on the Java mirrors themselves. */ + private static void assertRootEnumSeedAssumptions() { + long pastBaseNs = ROOT_ENUM_PAST_BASE_MS * NS_PER_MS; + long extendedNs = ROOT_ENUM_EXTENDING_COST_MS * ROOT_ENUM_NS_PER_COST_MS; + assertTrue(ROOT_ENUM_BEFORE_BASE_MS > 0, "before-base age must be positive"); + assertTrue(pastBaseNs < extendedNs && extendedNs < ROOT_ENUM_MAX_INTERVAL_NS, + "extending cost must land strictly between the past-base age and the cap"); + assertTrue(ROOT_ENUM_CAPPING_COST_MS * ROOT_ENUM_NS_PER_COST_MS > ROOT_ENUM_MAX_INTERVAL_NS, + "capping cost must exceed the cap"); + assertTrue(ROOT_ENUM_MAX_INTERVAL_NS > pastBaseNs, "cap must exceed the past-base age"); + assertTrue(ROOT_ENUM_PAST_CAP_MS * NS_PER_MS > ROOT_ENUM_MAX_INTERVAL_NS, + "past-cap age must exceed the cap"); + } + + /** + * Runs real GCs until the live GC-finish epoch is observed to advance (up to 5 attempts), so a + * {@code -1} epoch shift makes the epoch look advanced instead of clamping to the never-ran + * value. An epoch already > 0 that did not advance during the loop is still sufficient (the + * shift lands below live). Skips the test - never passes it vacuously - if the epoch is still 0, + * e.g. a GC configuration that posts no GarbageCollectionFinish for System.gc(). + */ + private static void ensureGcEpochAdvanced() throws InterruptedException { + long before = ReferenceChainTestHooks.referenceChainGcFinishEpochForTest0(); + for (int i = 0; i < 5; i++) { + System.gc(); + if (ReferenceChainTestHooks.referenceChainGcFinishEpochForTest0() > before) { + return; + } + Thread.sleep(100); + } + if (ReferenceChainTestHooks.referenceChainGcFinishEpochForTest0() == 0) { + assumeTrue(false, "no GarbageCollectionFinish observed after 5 System.gc() calls - the " + + "-1 epoch shift would clamp to the live value and test nothing"); + } + } + + private static void seedRunningSearch() { + // forceRunningSearchForTest (not just "search started"): the live BFS thread may be running + // its own real search (leftover leak signal from earlier tests in this shared JVM) and can + // drive it terminal between this seed and the gate call - the forced RUNNING state with fresh + // pain budgets keeps the gate evaluating the running-search lanes deterministically. + ReferenceChainTestHooks.forceRunningReferenceChainSearchForTest0(); + // Zero candidates: the canary lane is off and the epoch/cadence lanes are what run. + ReferenceChainTestHooks.setReferenceChainCanaryStateForTest0(0, 0); + } + + /** + * Kills the GC-epoch bypass: with the previous pass a moment ago, a freshly completed + * GC must NOT authorize a pass ahead of the minimum spacing - without the gate the epoch + * branch of {@code shouldRunPass()} returns true unconditionally, so pass rate tracks the + * young-GC rate. + */ + @Test + public void gcEpochPassWaitsForMinimumPassSpacing() throws InterruptedException { + assumeDebugBuild(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + ensureGcEpochAdvanced(); + seedRunningSearch(); + // Quiesce first: the spacing seed blocks every BFS lane, so no pass can overwrite the + // epoch shift seeded below. + ReferenceChainTestHooks.setReferenceChainPassSpacingForTest0(0, CADENCE_MS); + Thread.sleep(QUIESCE_MS); + assertTrue(ReferenceChainTestHooks.referenceChainGcFinishEpochForTest0() > 0, + "the -1 epoch shift needs a live GC-finish epoch > 0 to land below live"); + ReferenceChainTestHooks.shiftReferenceChainLastGcEpochForTest0(-1); + assertFalse(ReferenceChainTestHooks.shouldRunPassForTest0(), + "A new GC epoch must not authorize a pass sooner than the effective cadence after " + + "the previous pass"); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** The epoch lane must only be deferred, not disabled: once the spacing has elapsed, the new + * epoch authorizes the pass. */ + @Test + public void gcEpochPassRunsOnceSpacingElapses() throws InterruptedException { + assumeDebugBuild(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + ensureGcEpochAdvanced(); + seedRunningSearch(); + eventuallyTrue("A new GC epoch must authorize a pass once the minimum spacing has elapsed", + () -> { + seedRunningSearch(); + ReferenceChainTestHooks.setReferenceChainPassSpacingForTest0( + CADENCE_MS * 2, CADENCE_MS); + ReferenceChainTestHooks.shiftReferenceChainLastGcEpochForTest0(-1); + }, + ReferenceChainTestHooks::shouldRunPassForTest0); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** + * Kills the canary lane's unbounded back-to-back rate: a productive chase (backoff multiplier + * 1) would otherwise skip its spacing gate entirely, chaining passes with no gap and making + * every pass pay its JVM VM-operation cost. The epoch is shifted stale so only the canary lane + * could open the gate - the assertion must fail on that lane specifically. + */ + @Test + public void canaryBackToBackPassWaitsForMinimumPassSpacing() throws InterruptedException { + assumeDebugBuild(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + seedRunningSearch(); + ReferenceChainTestHooks.setReferenceChainPassSpacingForTest0(0, CADENCE_MS); + Thread.sleep(QUIESCE_MS); + // One unresolved candidate: the gate takes the canary lane with backoff multiplier 1. + // Seeded after the quiesce so the BFS thread's per-iteration pollWatchedTargets() (which + // overwrites the candidate set from real leak signals) cannot clobber it. + ReferenceChainTestHooks.setReferenceChainCanaryStateForTest0(1, 0); + ReferenceChainTestHooks.shiftReferenceChainLastGcEpochForTest0(1_000); + assertFalse(ReferenceChainTestHooks.shouldRunPassForTest0(), + "A canary chase with fresh backoff (multiplier 1) must still wait out the minimum " + + "pass spacing before starting the next pass"); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** The canary floor must only defer the chase, not starve it: once the spacing has elapsed, + * the chase resumes at multiplier-1 rate. */ + @Test + public void canaryPassRunsOnceSpacingElapses() throws InterruptedException { + assumeDebugBuild(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + seedRunningSearch(); + eventuallyTrue("A canary chase with fresh backoff must run a pass once the minimum " + + "spacing has elapsed", + () -> { + seedRunningSearch(); + ReferenceChainTestHooks.setReferenceChainCanaryStateForTest0(1, 0); + ReferenceChainTestHooks.setReferenceChainPassSpacingForTest0( + CADENCE_MS * 2, CADENCE_MS); + }, + ReferenceChainTestHooks::shouldRunPassForTest0); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** + * The reset contract (resetSearchStateForTest()): it zeroes {@code _last_pass_ns} along with + * the pass counter, so the minimum-spacing gate is open again right after a reset - a stale + * previous-pass timestamp must not hold the next test's first pass off. A closed reading with + * zero passes run since the reset is definitive (no real pass refreshed the timestamp); a + * closed reading after a real pass is retried. + */ + @Test + public void resetReopensMinimumPassSpacingGate() throws InterruptedException { + assumeDebugBuild(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + for (int attempt = 0; attempt < 10; attempt++) { + // A closed gate: the previous pass "just ended" under a long cadence. + ReferenceChainTestHooks.setReferenceChainPassSpacingForTest0(0, CADENCE_MS); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + seedRunningSearch(); + if (ReferenceChainTestHooks.shouldRunPassForTest0()) { + return; + } + assertTrue(ReferenceChainTestHooks.referenceChainPassesRunForTest0() != 0, + "the spacing gate must be open right after a reset - _last_pass_ns/" + + "_effective_cadence_ns were not reset"); + Thread.sleep(100); + } + assertFalse(true, "an open spacing gate right after a reset was never observed"); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** Root/stack-ref enumeration waits out its base interval before re-running. */ + @Test + public void rootEnumWaitsForMinimumInterval() throws InterruptedException { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + eventuallyTrue("Re-enumeration must wait out the base interval since the last attempt", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_BEFORE_BASE_MS, 0), + () -> rootEnumClosedWithSeedIntact(ROOT_ENUM_MIN_INTERVAL_NS)); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** The base interval must only defer enumeration, not disable it. */ + @Test + public void rootEnumRunsAfterBaseInterval() throws InterruptedException { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + eventuallyTrue("Re-enumeration must run once the base interval has elapsed", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, 0), + () -> ReferenceChainTestHooks.shouldRunRootEnumForTest0(false)); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** + * Pins the ROOT_ENUM_MIN_INTERVAL_NS mirror against the live library: with no measured cost + * the native floor is exactly the base interval. + */ + @Test + public void rootEnumBaseIntervalMatchesNative() throws InterruptedException { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + eventuallyTrue("The base root-enumeration interval must match ROOT_ENUM_MIN_INTERVAL_NS", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, 0), + () -> ReferenceChainTestHooks.rootEnumIntervalFloorNsForTest0() + == ROOT_ENUM_MIN_INTERVAL_NS); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** + * Kills the fixed 2s interval: an attempt that held the VM at a safepoint for a known cost + * must push the next attempt out proportionally - on a busy service the cost recurs (the JVM + * re-creates frames with scalar-replaced locals continuously), so only a cost-scaled floor + * bounds the deopt-all rate. + */ + @Test + public void rootEnumIntervalExtendsWithMeasuredSafepointCost() throws InterruptedException { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + // Positive control: the past-base seed alone opens the gate, so the closed reading below + // is attributable to the cost-extended floor. + eventuallyTrue("A past-base seed with no measured cost must open the gate", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, 0), + () -> ReferenceChainTestHooks.shouldRunRootEnumForTest0(false)); + // ROOT_ENUM_EXTENDING_COST_MS of measured in-safepoint cost: floor = max(MIN, cost x + // PER_COST) - strictly between the past-base age and the cap. + eventuallyTrue("The interval floor must extend by ROOT_ENUM_INTERVAL_NS_PER_COST_MS per " + + "measured in-safepoint cost millisecond", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, ROOT_ENUM_EXTENDING_COST_MS), + () -> ReferenceChainTestHooks.rootEnumIntervalFloorNsForTest0() + == ROOT_ENUM_EXTENDING_COST_MS * ROOT_ENUM_NS_PER_COST_MS); + eventuallyTrue("Re-enumeration must wait out the cost-extended interval even though the " + + "base interval has elapsed", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, ROOT_ENUM_EXTENDING_COST_MS), + () -> rootEnumClosedWithSeedIntact( + ROOT_ENUM_EXTENDING_COST_MS * ROOT_ENUM_NS_PER_COST_MS)); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** The cost-scaled interval must stay capped so root refresh never stops entirely. */ + @Test + public void rootEnumIntervalCapsAtMaximum() throws InterruptedException { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + // Positive control, as in rootEnumIntervalExtendsWithMeasuredSafepointCost. + eventuallyTrue("A past-base seed with no measured cost must open the gate", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, 0), + () -> ReferenceChainTestHooks.shouldRunRootEnumForTest0(false)); + // ROOT_ENUM_CAPPING_COST_MS scales past ROOT_ENUM_MAX_INTERVAL_NS uncapped; the cap holds + // it there. + eventuallyTrue("The cost-scaled interval must be capped at ROOT_ENUM_MAX_INTERVAL_NS", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, ROOT_ENUM_CAPPING_COST_MS), + () -> ReferenceChainTestHooks.rootEnumIntervalFloorNsForTest0() + == ROOT_ENUM_MAX_INTERVAL_NS); + eventuallyTrue("Re-enumeration must wait out the capped interval", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_BASE_MS, ROOT_ENUM_CAPPING_COST_MS), + () -> rootEnumClosedWithSeedIntact(ROOT_ENUM_MAX_INTERVAL_NS)); + // An uncapped floor would still hold this age off. + eventuallyTrue("Re-enumeration must run once the capped interval has elapsed", + () -> ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0( + ROOT_ENUM_PAST_CAP_MS, ROOT_ENUM_CAPPING_COST_MS), + () -> ReferenceChainTestHooks.shouldRunRootEnumForTest0(false)); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } + + /** The search's first enumeration is mandatory and must ignore the interval entirely. */ + @Test + public void firstPassAlwaysRunsRootEnum() { + assumeDebugBuild(); + assertRootEnumSeedAssumptions(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + try { + // Worst case: an attempt "just ran" at maximum measured cost - the first pass of a fresh + // search must still enumerate. + ReferenceChainTestHooks.setReferenceChainRootEnumStateForTest0(0, ROOT_ENUM_CAPPING_COST_MS); + assertTrue(ReferenceChainTestHooks.shouldRunRootEnumForTest0(true), + "The search's first pass must always run root/stack-ref enumeration"); + } finally { + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + } + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java index c86a1aaf58..b7ee30eacd 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java @@ -10,9 +10,8 @@ import com.datadoghq.profiler.ReferenceChainTestHooks; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static com.datadoghq.profiler.referencechains.ReferenceChainAssertions.assumeDebugBuild; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * PROF-15341 follow-up: {@code ReferenceChainTrackingTest} drives slope detection and chain @@ -64,10 +63,6 @@ protected boolean isPlatformSupported() { return !(Platform.isJavaVersion(8) || Platform.isJ9() || Platform.isZing()); } - private static void assumeDebugBuild() { - assumeTrue(ReferenceChainAssertions.isDebugInstrumentedBuild()); - } - /** Seeds twenty epochs of strictly increasing population counts, then asserts * {@code selectLeakCandidates()} ranks the klass as a leak candidate - proving the ranking * logic works without the real JVMTI heap sampler ever surfacing this klass. */ @@ -160,38 +155,4 @@ public void shouldReconstructChainForDirectlyTaggedRoot() { chainTargetHolder = null; // shared-JVM hygiene } } - - /** The shared discovery-test failure fingerprint is empty on release builds (the gate lives in - * the helper) and, on debug builds, carries the canonical segments in the canonical order. Read - * only on the seams - no reset/seed calls. */ - @Test - public void failureDiagnosticsFingerprintHasCanonicalSegments() { - String d = ReferenceChainAssertions.referenceChainFailureDiagnostics(new Object(), null); - if (!ReferenceChainAssertions.isDebugInstrumentedBuild()) { - assertEquals("", d, "release builds must keep the plain failure message"); - return; - } - assertTrue(d.startsWith(" [leak-signal candidates=["), d); - assertTrue(d.endsWith(", observedChain0s=[]]"), d); - String[] segments = { - "leak-signal candidates=", - ", bfsPassesRun=", - ", representativeFrontierState=", - " (-1=never admitted, 0=admitted pending expansion, 1=expanded, 3=abandoned)", - ", observedChain0s=" - }; - int previous = -1; - for (String segment : segments) { - int index = d.indexOf(segment); - assertTrue(index > previous, "segment '" + segment + "' missing or out of order in: " + d); - previous = index; - } - assertTrue(d.matches("(?s).*representativeFrontierState=-?\\d+ \\(-1=never admitted.*"), d); - } - - /** observedChainHeads tolerates absent events: never throws, never returns null. */ - @Test - public void observedChainHeadsIsEmptyForNoEvents() { - assertTrue(ReferenceChainAssertions.observedChainHeads(null).isEmpty()); - } } From 4997498a2151b073b94a2097a3264d620228b13e Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 30 Oct 2026 16:33:51 +0100 Subject: [PATCH 3/4] docs: reference-chain pass spacing and adaptive root-enum interval --- .../LiveHeapReferenceChainsImplementation.md | 4 +++- .../ReferenceChainsSignalsExplained.md | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/doc/architecture/LiveHeapReferenceChainsImplementation.md b/doc/architecture/LiveHeapReferenceChainsImplementation.md index 5e8a14e31c..8c46ecf8f6 100644 --- a/doc/architecture/LiveHeapReferenceChainsImplementation.md +++ b/doc/architecture/LiveHeapReferenceChainsImplementation.md @@ -69,7 +69,7 @@ The tracker measures total pass time and in-safepoint time separately. It charge `IterateOverReachableObjects()` calls `heapRootCallback()` and `stackRefCallback()`. These callbacks admit root-attached objects. They do not traverse all children of each root. -The first pass always performs root enumeration. A later pass performs it after a truncated enumeration or an interval of at least two seconds. +The first pass always performs root enumeration. A later pass performs it after an interval of at least two seconds since the last attempt, extended by the last attempt's measured in-safepoint cost (20 milliseconds of interval per millisecond of cost, capped at 30 seconds). Each enumeration deoptimizes, on every thread, every compiled frame whose scope holds a scalar-replaced (escape-analysis-eliminated) object (one `VM_DeoptimizeFrame` operation per frame), and the JIT re-creates those frames continuously on a busy service, so the interval grows with the cost that recurs per attempt. A truncated attempt retries after the same interval as a completed one: with a first-pass budget too small for the live heap, every attempt truncates, and a next-pass retry would turn every pass into a deopt-all enumeration. A truncated pass ends after the enumeration - the next pass's expansion phases proceed on the roots already admitted. Root enumeration uses `firstpassbudget`. The name is historical: the implementation can use this budget again during later root-enumeration attempts. @@ -171,6 +171,8 @@ The base candidate-stall limit is 30 passes. Consecutive `CANARY_STUCK` restarts The implementation controls different costs separately: +- The minimum pass spacing (the effective cadence) applies to every pass authorization - GC epoch, canary back-to-back, and fallback cadence alike. +- The root-enumeration interval floor extends with the last attempt's measured in-safepoint cost. - `_effective_budget` limits edge admission for ordinary work. - `_pause_pid` adjusts that budget from measured in-safepoint time. - `_effective_cadence_ns` changes when the budget controller reaches a limit. diff --git a/doc/architecture/ReferenceChainsSignalsExplained.md b/doc/architecture/ReferenceChainsSignalsExplained.md index 8ef3295b56..c0f98ff925 100644 --- a/doc/architecture/ReferenceChainsSignalsExplained.md +++ b/doc/architecture/ReferenceChainsSignalsExplained.md @@ -30,13 +30,20 @@ The population-trend signal is constructed by the liveness tracker, which fills A running search uses the following order: -1. Check the non-safepoint CPU cost budget. -2. If unresolved candidates exist, apply candidate-search backoff. -3. Otherwise, run after a new GC epoch or after the adaptive fallback cadence expires. +1. Enforce the minimum pass spacing: no pass starts sooner than the effective cadence after the previous pass ended, on every authorization lane below. +2. Check the non-safepoint CPU cost budget. +3. If unresolved candidates exist, apply candidate-search backoff. +4. Otherwise, run after a new GC epoch or after the adaptive fallback cadence expires. + +### Minimum pass spacing + +A completed GC authorizes a pass, and a productive canary chase (backoff multiplier one) may run its next pass as soon as scheduling allows - but neither may start sooner than one effective cadence interval after the previous pass ended. Without this floor, a high young-GC rate re-triggers passes at the GC rate even after the pacing controller has widened the cadence to shed in-safepoint load, and the canary lane runs passes with no gap at all. Each pass costs several JVM VM operations (root enumeration, static-field sweep chunks, expansion batches), so unbounded pass rate multiplies into a safepoint storm. + +The OOM ramp needs no exemption: while urgent it owns the effective cadence outright, driving this floor down with urgency. ### Candidate-search backoff -The source calls the focused candidate search the *canary* path. A new candidate or a resolved candidate resets its spacing multiplier to one. With this value, the next pass can start after the previous pass finishes. +The source calls the focused candidate search the *canary* path. A new candidate or a resolved candidate resets its spacing multiplier to one. With this value, the next pass can start after the previous pass finishes, but never sooner than the minimum pass spacing above. If a pass makes no candidate progress, the multiplier doubles up to its cap. The required spacing is: @@ -48,9 +55,9 @@ The OOM ramp disables this backoff. The non-safepoint CPU cost budget still appl ### GC epoch and fallback cadence -When no unresolved candidate controls scheduling, a completed GC authorizes a pass. The tracker compares the current GC-finish epoch with the epoch recorded by the previous pass. +When no unresolved candidate controls scheduling, a completed GC authorizes a pass. The tracker compares the current GC-finish epoch with the epoch recorded by the previous pass. The authorization remains subject to the minimum pass spacing: when young GCs are more frequent than the effective cadence, the pass rate follows the cadence, not the GC rate. -The fallback cadence also authorizes a pass. It starts at one second and the pause controller can adjust it between 10 milliseconds and four seconds. +The fallback cadence also authorizes a pass. It starts at one second and the pause controller can adjust it between 10 milliseconds and four seconds. The minimum pass spacing uses the same value, so the two authorizations share one rate bound. ## Leak signal From bc30ba404b61cd157677e50f20a4960dfb078579 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Fri, 30 Oct 2026 16:33:51 +0100 Subject: [PATCH 4/4] test: opt-in EA spin workers in the reference-chain repro REFCHAINS_EASPIN= starts workers holding scalar-replaced locals live across a loop back-edge, so root enumerations deoptimize their compiled frames and the deopt axis of the safepoint storm is reproducible locally. Document the variable in run-refchains-repro.sh. --- .../repro/ReferenceChainLeakDemo.java | 99 +++++++++++++++++++ utils/run-refchains-repro.sh | 8 ++ 2 files changed, 107 insertions(+) diff --git a/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java b/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java index 9415bb57a1..455d7745b4 100644 --- a/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java +++ b/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java @@ -56,6 +56,16 @@ * anywhere near this app's {@code CachedEntry} population. Pass {@code firstpassbudget=N} * explicitly only to override that auto-scaled default. * + *

Optional escape-analysis spin workers: setting {@code REFCHAINS_EASPIN=} + * (e.g. {@code REFCHAINS_EASPIN=4}) starts worker threads that hold compiled frames with + * scalar-replaced (EA-eliminated) locals on stack while the tracker runs, so the root + * enumeration's forced deoptimizations (see {@link #spinLoop}) become observable in the + * recording. Each worker busy-spins one full + * core while it runs; the workers stop at the run's duration deadline (within one + * {@code spinLoop} trip) and, in the default run-forever mode, only at JVM exit - size the + * count well below the core count when comparing safepoint/GC timings, since the spin load + * competes with GC and the tracker thread. + * *

Grows an ever-referenced, never-evicted {@code HashMap}-backed cache (same shape as * ddprof-test's {@code LeakingCacheScenario}). Two deliberate choices, both found by running * without them and never seeing a chain get discovered: @@ -181,6 +191,8 @@ private static void run(Map cache, Path recordingPath, Stri long startNanos = System.nanoTime(); long deadlineNanos = durationSeconds >= 0 ? startNanos + durationSeconds * 1_000_000_000L : Long.MAX_VALUE; + + startEaSpinWorkers(deadlineNanos); long startHeapUsedMb = heapUsedMb(); long nextKey = 300; @@ -241,6 +253,9 @@ private static void run(Map cache, Path recordingPath, Stri Thread.sleep(roundIntervalMs); } + // Wind down the EA spin workers (no-op if none started) before the final dump/metrics. + spinStop = true; + if (durationSeconds >= 0) { profiler.dump(snapshotPath); System.out.println("[repro] dumped " + snapshotPath); @@ -309,6 +324,90 @@ private static String pct(double fraction) { return String.format("%.1f%%", fraction * 100.0); } + /** + * Escape-analysis spin workers ({@code REFCHAINS_EASPIN=} threads, default 0): while a + * worker sits in {@link #spinLoop}, the root enumeration's + * {@code EscapeBarrier::deoptimize_objects_all_threads} deoptimizes its compiled frame + * ({@code has_ea_local_in_scope()}). A forced deopt makes the nmethod not-entrant; + * re-invoking the method re-compiles it and re-applies EA, so deopts recur for the whole run. + * + *

Each worker is a full-core busy loop. In a bounded run it exits within one + * {@code spinLoop} trip once {@code deadlineNanos} passes or the main loop ends + * ({@link #spinStop}), so the final dump/metrics are not perturbed; in the default + * run-forever mode ({@code deadlineNanos == Long.MAX_VALUE}) it runs until JVM exit (daemon), + * matching the main loop. Extra spin load competes with GC and the tracker thread, so size + * {@code n} well below the core count when comparing safepoint/GC timings. + */ + private static void startEaSpinWorkers(final long deadlineNanos) { + int threads; + String raw = System.getenv("REFCHAINS_EASPIN"); + if (raw == null || raw.trim().isEmpty()) { + return; + } + try { + threads = Integer.parseInt(raw.trim()); + } catch (NumberFormatException e) { + System.err.println("[repro] invalid REFCHAINS_EASPIN='" + raw + "' - starting no spin workers"); + return; + } + for (int t = 0; t < threads; t++) { + final long seed = 0x51ED270B + t * 0x9E3779B97F4A7C15L; + Thread th = new Thread(() -> { + // Re-checked once per spinLoop trip, never inside spinLoop - its body shape is + // load-bearing for the EA deopt target - so exit latency is at most one trip. + while (!spinStop && System.nanoTime() < deadlineNanos) { + spinLoop(seed, SPIN_TRIP_COUNT, SPIN_SALT); + } + }, "easpin-" + t); + th.setDaemon(true); + th.start(); + } + System.out.println("[repro] started " + threads + " EA spin workers (one busy core each, " + + (deadlineNanos == Long.MAX_VALUE ? "until JVM exit" : "until the run deadline") + + ") - root enumerations should now deoptimize their compiled frames"); + } + + private static final long SPIN_TRIP_COUNT = 300_000_000L; + private static final long SPIN_SALT = 0x27D4EB2F165667C5L; + + // Set once run()'s main loop exits; spin workers check it between spinLoop trips. In bounded + // mode the main loop exits at the same deadline the workers check, so the flag only shortens + // the window by at most one trip; in run-forever mode the main loop never exits, so the flag + // is never set and the workers stop at JVM exit. + private static volatile boolean spinStop; + + /** + * One field, one constructor argument, so C2 scalar-replaces the instance - the shape + * {@code has_ea_local_in_scope()} reports. Identity hashCode, monitors, or a non-inlined + * call taking the object force a heap allocation and remove the deopt target. + */ + private static final class EaLocal { + long f; + + EaLocal(long f) { + this.f = f; + } + } + + /** + * Loop body shaped for a reliable deopt target: one {@code EaLocal} live across every + * iteration (at any safepoint inside the loop the compiled scope holds the scalar-replaced + * local), a conditional {@code p.f} write so the read cannot be hoisted away, and a salt-xor + * so the accumulator cannot be evaluated symbolically and the loop must actually run. + */ + private static long spinLoop(long seed, long iterations, long salt) { + EaLocal p = new EaLocal(seed ^ salt); + long acc = seed; + for (long i = 0; i < iterations; i++) { + acc += p.f; + acc ^= salt; + if ((i & 0xFFFF) == 0) { + p.f = acc; + } + } + return acc + p.f; + } + /** * Same shape as {@code LeakingCacheScenario.CachedPayload}/{@code * ReferenceChainLeakAntagonist.CachedEntry}: plain long fields, not a nested array, so a diff --git a/utils/run-refchains-repro.sh b/utils/run-refchains-repro.sh index 642f1b431a..ab539a7e07 100755 --- a/utils/run-refchains-repro.sh +++ b/utils/run-refchains-repro.sh @@ -42,6 +42,14 @@ # bare name ("Serial", "G1", "Parallel", "Shenandoah", # "Z", "Epsilon") or the full flag name ("SerialGC"). # e.g. REFCHAINS_GC=Serial or REFCHAINS_GC=ZGC +# REFCHAINS_EASPIN number of escape-analysis spin worker threads the demo +# app starts (default: 0 - passed through to the app via +# the env var of the same name; see ReferenceChainLeakDemo. +# startEaSpinWorkers). With workers running, root enumerations +# deoptimize their compiled frames, making VM_DeoptimizeFrame +# safepoints observable in the recording. Each worker +# busy-spins one core; workers stop at the run's deadline +# (run-forever: at JVM exit). set -euo pipefail