diff --git a/ddprof-lib/src/main/cpp/counters.h b/ddprof-lib/src/main/cpp/counters.h index fc148f4664..4f95d0aca2 100644 --- a/ddprof-lib/src/main/cpp/counters.h +++ b/ddprof-lib/src/main/cpp/counters.h @@ -197,6 +197,26 @@ * and re-emits it on a later dump while the leak candidate is still \ * live. */ \ X(REFERENCE_CHAIN_WRITE_DROPPED, "reference_chain_write_dropped") \ + /* LivenessTracker::releaseLeakTag() was called for a leak-tag slot that \ + * is already free (zero/zero encoding) - a double release. The release is \ + * dropped: pushing the index twice would let acquireLeakTag() hand the \ + * same tag to two live objects. A nonzero rate here means the leak-tag \ + * ownership accounting (tagLeakInstances()' tag-adoption branch) is \ + * sharing one pool tag between entries. */ \ + X(REFERENCE_CHAIN_LEAK_TAG_DOUBLE_RELEASE, "reference_chain_leak_tag_double_release") \ + /* LivenessTracker urgency-boost observability (admitForTracking()): \ + * ADMITS counts every 100% admission made while _urgent_tracking is set \ + * and the table is below its cap; BACKED_OFF counts admissions at the \ + * high-water mark, where the boost degrades to watched-tid-only and the \ + * thread falls back to the configured subsample ratio. A persistently \ + * rising BACKED_OFF means the urgency window is outpacing the cleanup \ + * reaper. */ \ + X(LIVENESS_URGENT_BOOST_ADMITS, "liveness_urgent_boost_admits") \ + X(LIVENESS_URGENT_BOOST_BACKED_OFF, "liveness_urgent_boost_backed_off") \ + /* Defensive cap: releaseLeakTag() found the free list full. Unreachable \ + * by construction while every release is paired with an acquire; a \ + * nonzero value means the acquire/release pairing is broken somewhere. */ \ + X(REFERENCE_CHAIN_LEAK_TAG_RELEASE_OVERFLOW, "reference_chain_leak_tag_release_overflow") \ /* FrontierTable's own calloc/realloc-backed storage (referenceChains.cpp) - \ * outside NMT's visibility since it bypasses os::malloc, so this is the only \ * way to attribute its native RSS contribution. */ \ diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index 90f6696755..77c04e6bfd 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -1104,6 +1104,223 @@ Java_com_datadoghq_profiler_JavaProfiler_dumpContext(JNIEnv* env, jclass unused) TEST_LOG("===> Context: tid:%lu, spanId=%lu, rootSpanId=%lu", OS::threadId(), spanId, rootSpanId); } +// LivenessTracker/ReferenceChainTracker test seams. Unlike +// testlog()/dumpContext() above (harmless no-ops in release, via TEST_LOG's +// own release-mode expansion to nothing), these mutate real tracker state +// (tagging objects, seeding population history) - shipping them into a +// release build would let a caller corrupt the actual leak-detection state, +// not just add a silent no-op. Guarded out entirely instead, so they only +// exist in the debug build ddprof-test's `testdebug` Gradle task loads +// (`-DDEBUG`, see ConfigurationPresets.kt's configureDebug()) - never in the +// `-DNDEBUG` release build. +#ifdef DEBUG +#include "livenessTracker.h" +#include "referenceChains.h" +#include + +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setGcGenerationsEnabled0( + JNIEnv *env, jclass unused, jboolean enabled) { + LivenessTracker::instance()->setGcGenerationsForTest(enabled); + return JNI_TRUE; +} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_seedKlassPopulationSample0( + JNIEnv *env, jclass unused, jint klassId, jint count, jlong epoch) { + int slot; + bool created; + LivenessTracker::instance()->klassPopulationRecordForTest( + (u32)klassId, (u16)count, (u64)epoch, &slot, &created); +} + +// Seeds one per-(klass, tid) trend sample - see tidTrendRecordForTest()'s +// own comment (livenessTracker.h) for the synthetic-flag exemption and the +// real-tid requirement scenarios must honor. +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_seedTidTrendSample0( + JNIEnv *env, jclass unused, jint klassId, jint tid, jint count, + jlong epoch) { + LivenessTracker::instance()->tidTrendRecordForTest( + (u32)klassId, (jint)tid, (u32)count, (u64)epoch); +} + +// Wires a real, caller-chosen live object in as klassId's leak-candidate +// representative, so a test-seeded slope signal (seedKlassPopulationSample0 +// above) and a directly-tagged frontier root (tagAsReferenceChainRoot0 +// below) can be joined into one deterministic end-to-end run of +// pollWatchedTargets()'s bridging step - without either LivenessTracker's +// real allocation sampler or ReferenceChainTracker's root-seeded walk ever +// running. Takes its own weak global ref (klassPopulationSetRepresentativeForTest()'s +// own contract, livenessTracker.h) rather than aliasing any handle the +// caller manages. +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setKlassPopulationRepresentativeForTest0( + JNIEnv *env, jclass unused, jint klassId, jobject representative) { + jweak rep = env->NewWeakGlobalRef(representative); + LivenessTracker::instance()->klassPopulationSetRepresentativeForTest( + env, (u32)klassId, rep); +} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_resetKlassPopulationForTest0( + JNIEnv *env, jclass unused) { + LivenessTracker::instance()->klassPopulationResetForTest(); +} + +extern "C" DLLEXPORT jintArray JNICALL +Java_com_datadoghq_profiler_JavaProfiler_selectLeakCandidateKlassIds0( + JNIEnv *env, jclass unused) { + KlassCandidate candidates[5]; + int n = LivenessTracker::instance()->selectLeakCandidates(candidates, 5); + jintArray result = env->NewIntArray(n); + if (result == nullptr || n == 0) { + return result; + } + jint ids[5]; + for (int i = 0; i < n; i++) { + ids[i] = (jint)candidates[i].klass_id; + } + env->SetIntArrayRegion(result, 0, n, ids); + return result; +} + +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_tagAsReferenceChainRoot0( + JNIEnv *env, jclass unused, jobject target) { + jvmtiEnv *jvmti = VM::jvmti(); + if (jvmti == nullptr) { + return 0; + } + return ReferenceChainTracker::instance()->tagAsRootForTest(jvmti, env, + target); +} + +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_runReferenceChainPass0( + JNIEnv *env, jclass unused) { + jvmtiEnv *jvmti = VM::jvmti(); + if (jvmti == nullptr) { + return JNI_FALSE; + } + return ReferenceChainTracker::instance()->runPassSerialized(jvmti, env); +} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_pollReferenceChainTargets0( + JNIEnv *env, jclass unused) { + jvmtiEnv *jvmti = VM::jvmti(); + if (jvmti == nullptr) { + return; + } + ReferenceChainTracker::instance()->pollWatchedTargetsSerialized(jvmti, env); +} + +extern "C" DLLEXPORT jint JNICALL +Java_com_datadoghq_profiler_JavaProfiler_drainReferenceChainEventCount0( + JNIEnv *env, jclass unused) { + std::vector events; + ReferenceChainTracker::instance()->drainPendingChainEvents(&events); + return (jint)events.size(); +} + +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_resetReferenceChainSearchForTest0( + JNIEnv *env, jclass unused) { + jvmtiEnv *jvmti = VM::jvmti(); + ReferenceChainTracker::instance()->resetSearchStateForTest(jvmti, env); +} + +// Diagnostic-only: reads target's existing JVMTI tag (does NOT tag it - +// unlike tagAsReferenceChainRoot0 above, a target the real search has not +// reached yet must be left untagged) and reports its FIFO distance from the +// front of ReferenceChainTracker's pending-expansion queue. See +// ReferenceChainTracker::pendingExpandPositionForTest()'s own comment for +// the return-value contract. +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_getReferenceChainPendingPositionForTest0( + JNIEnv *env, jclass unused, jobject target) { + jvmtiEnv *jvmti = VM::jvmti(); + if (jvmti == nullptr || target == nullptr) { + return -2; + } + jlong tag = 0; + jvmtiError err = jvmti->GetTag(target, &tag); + if (err != JVMTI_ERROR_NONE) { + return -2; + } + return (jlong)ReferenceChainTracker::instance()->pendingExpandPositionForTest( + tag); +} + +extern "C" DLLEXPORT jlong JNICALL +Java_com_datadoghq_profiler_JavaProfiler_getReferenceChainPendingSizeForTest0( + JNIEnv *env, jclass unused) { + return (jlong)ReferenceChainTracker::instance()->pendingExpandSizeForTest(); +} + +// Seeds one heap-floor-ring sample directly (LivenessTracker::secondsToOOM()'s +// input), bypassing the real GarbageCollectionFinish callback - lets a test +// build an arbitrary rising/flat heap-usage-over-time history without +// waiting on real GCs. timestampNs values are only ever compared against +// each other (secondsToOOM()'s own ringWindowStats() deltas), never against +// a real wall clock, so a test may use any self-consistent, strictly +// increasing sequence. +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_heapFloorRecordForTest0( + JNIEnv *env, jclass unused, jlong usedBytes, jlong timestampNs) { + LivenessTracker::instance()->heapFloorRecordForTest((u64)usedBytes, + (u64)timestampNs); +} + +// Bypasses initialize_table()'s JNI-dependent HeapUsage::getMaxHeap() call so +// secondsToOOM() can be exercised against a test-chosen fake max heap size, +// independent of whatever -Xmx this JVM's own shared, no-forkEvery fork +// happens to run with. +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setMaxHeapBytesForTest0( + JNIEnv *env, jclass unused, jlong maxHeapBytes) { + LivenessTracker::instance()->setMaxHeapBytesForTest((jlong)maxHeapBytes); +} + +// Temporarily disables onGC()'s own recordHeapFloorSample() call so a test +// can seed the heap-floor ring exclusively via heapFloorRecordForTest0() +// without a real GC interleaving a sample with a real OS::nanotime() +// timestamp and real heap usage, corrupting secondsToOOM()'s projection. +extern "C" DLLEXPORT void JNICALL +Java_com_datadoghq_profiler_JavaProfiler_setHeapFloorRecordingForTest0( + JNIEnv *env, jclass unused, jboolean enabled) { + LivenessTracker::instance()->setHeapFloorRecordingForTest(enabled == JNI_TRUE); +} + +// Exposes ReferenceChainTracker::shouldRunPass() directly (see that seam's +// own comment, referenceChains.h) - unlike runReferenceChainPass0() above, +// which calls runPass() unconditionally, this reports whether the +// search-restart gate itself (canAffordNewSearch() -> hasLeakSignal()) would +// currently allow a fresh/terminal search to start. +extern "C" DLLEXPORT jboolean JNICALL +Java_com_datadoghq_profiler_JavaProfiler_shouldRunPassForTest0(JNIEnv *env, + jclass unused) { + return ReferenceChainTracker::instance()->shouldRunPassForTest( + OS::nanotime()) + ? JNI_TRUE + : JNI_FALSE; +} + +// Exposes ReferenceChainTracker::passesRun() directly - not itself DEBUG-gated on the native side +// (used by production JFR event fields too), but exposed here only for test use: lets a test note +// the current pass count before creating an object, then wait for that count to advance before +// trusting any match against it - the only way to be certain a match came from a pass whose own +// expandFrontier() (and therefore collectStaleExpandedEntriesForRotation()) ran strictly after the +// object existed, rather than from the same pass racing the object's creation. +extern "C" DLLEXPORT jint JNICALL +Java_com_datadoghq_profiler_JavaProfiler_referenceChainPassesRunForTest0( + JNIEnv *env, jclass unused) { + return (jint)ReferenceChainTracker::instance()->passesRun(); +} + +#endif // DEBUG + // ---- Test-only reads of the current thread's OTEP record ----------------------------------- // Each reads the current carrier's record directly via ProfiledThread::current(), with no // detach/attach (diagnostic-only, not on any signal-handler or hot write path). diff --git a/ddprof-lib/src/main/cpp/livenessTracker.cpp b/ddprof-lib/src/main/cpp/livenessTracker.cpp index 5bfdb9aef8..ceb8e28b57 100644 --- a/ddprof-lib/src/main/cpp/livenessTracker.cpp +++ b/ddprof-lib/src/main/cpp/livenessTracker.cpp @@ -13,6 +13,7 @@ #include "common.h" #include "context.h" #include "context_api.h" +#include "counters.h" #include "hotspot/vmStructs.h" #include "hotspot/vmStructs.inline.h" #include "incbin.h" @@ -51,42 +52,35 @@ namespace { // blocked for the whole sweep otherwise. constexpr u32 RESOLVE_BUDGET_PER_SWEEP = 256; -// Trend statistics of a chronological ring window - the one computation +// Window aggregation for a chronological ring - the one computation // hasQualifyingGrowth() (per-klass count_ring) and heapFloorRising() (the // aggregate _heap_floor_ring) both need, factored out so the window/index -// derivation and the two aggregation loops exist in exactly one place rather -// than three near-identical copies. Templated on the reader rather than the -// ring's element type or storage: the per-klass ring is a plain array read -// under the caller's already-held _table_lock, while the heap-floor ring is -// lock-free and read via loadAcquire() (see _heap_floor_ring's own comment, -// livenessTracker.h) - `read(i)` lets each caller supply its own access -// discipline for physical slot `i` without this shared loop needing to know -// which one applies. -// -// DESPITE THE NAME, this is not thirds statistics: the design doc's original -// "mean of earliest third vs mean of recent third" comparison was replaced by -// a full-window least-squares linear regression (see ringThirdsStats below), -// which uses all samples and is far more robust for oscillating-but-growing -// trends. The field names survive as the regression values consumers treat -// as the window's "earliest"/"recent" levels: -// earliest_mean - regression value at the window's OLDEST sample (x = 0); -// recent_mean - regression value at the window's NEWEST sample (x = fill-1); -// earliest_min - true minimum over the FULL window; -// recent_min - true minimum over the most recent HALF of the window. -// Consumers read earliest_mean/recent_mean as a smoothed start-vs-end delta -// (a regression slope over the window's span) and earliest_min/recent_min as -// floor checks. Renaming the fields would touch every consumer for no -// behavioral change, so the mapping is documented here instead. -struct RingThirdsStats { - double earliest_mean; // regression value at the window's oldest sample - double recent_mean; // regression value at the window's newest sample - double earliest_min; // true min over the full window - double recent_min; // true min over the window's most recent half +// derivation and the aggregation loops exist in exactly one place rather +// than three near-identical copies. The name is historical: despite the +// struct's field names, this computes a FULL-WINDOW least-squares linear +// regression, not third means - see ringWindowStats()'s comment below. Templated on +// the reader rather than the ring's element type or storage: the per-klass +// ring is a plain array read under the caller's already-held _table_lock, +// while the heap-floor ring is lock-free and read via loadAcquire() (see +// _heap_floor_ring's own comment, livenessTracker.h) - `read(i)` lets each +// caller supply its own access discipline for physical slot `i` without +// this shared loop needing to know which one applies. +struct RingWindowStats { + // Fitted-line values, NOT window means: earliest_mean is the regression + // intercept (the fitted value at x=0, the oldest sample), recent_mean the + // fitted endpoint (x=n-1, the newest). earliest_min/recent_min are true + // minima over the first/second half of the window (recent_min's half + // boundary is i >= n/2, so for odd n the median sample joins the recent + // half). + double earliest_mean; + double recent_mean; + double earliest_min; + double recent_min; }; template -bool ringThirdsStats(int head, int fill, int ring_size, int min_fill, - Reader read, RingThirdsStats *out) { +bool ringWindowStats(int head, int fill, int ring_size, int min_fill, + Reader read, RingWindowStats *out) { if (fill < min_fill) { return false; } @@ -133,21 +127,21 @@ bool ringThirdsStats(int head, int fill, int ring_size, int min_fill, // Recent-half corroboration for a usage ring (see secondsToOOM()'s own // comment): a rising full-window trend whose most recent half is flat is a -// plateaued step change, not ongoing growth. The recent half's own -// regression (see ringThirdsStats) must show a strictly positive delta for -// the full-window trend to stand. A too-sparse recent half (below min_fill) -// REJECTS the projection rather than letting it through: false means -// "reject". Deliberately stricter than the single-ring version's -// have_recent_half semantics (which only rejected on a confirmed flat -// recent half) - with the ring-fill floors the caller already enforces, a -// sparse recent half means the recent data does not yet support the trend, -// so the boundary projection waits for more samples. +// plateaued step change, not ongoing growth. Returns true only when the +// recent half itself shows a rising trend; an absent or too-sparse recent +// half REJECTS the boundary (returns false). That is the conservative +// direction: a boundary projection that only the full window supports can +// mask a dip-then-recover shape whose full-window endpoints happen to +// agree, and the OOM urgency ramp is expensive enough to demand +// corroboration before firing. (An earlier draft of this comment claimed +// the sparse case lets the full-window trend stand alone - the code never +// did that; the comment was wrong.) template bool corroborateRecentHalf(u8 head, u8 fill, int ring_size, int min_fill, Reader read) { int half_fill = fill / 2; - RingThirdsStats recent_half_stats; - bool have_half = ringThirdsStats(head, half_fill, ring_size, min_fill, read, + RingWindowStats recent_half_stats; + bool have_half = ringWindowStats(head, half_fill, ring_size, min_fill, read, &recent_half_stats); double half_delta = have_half ? recent_half_stats.recent_mean - recent_half_stats.earliest_mean @@ -157,7 +151,8 @@ bool corroborateRecentHalf(u8 head, u8 fill, int ring_size, int min_fill, } // namespace -void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { +void LivenessTracker::cleanup_table(bool forced, bool allow_resolve, + bool account_epoch) { u64 current = load(_last_gc_epoch); u64 target_gc_epoch = load(_gc_epoch); TEST_LOG_SUMMARY("LivenessTracker::cleanup_table forced=%d gc_generations=%d current_epoch=%llu " @@ -187,23 +182,27 @@ void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { _table_lock.lock(); u64 claimed = load(_last_gc_epoch); - // Forward-only claim: a cleanup caller captured target_gc_epoch BEFORE it - // took the lock; by the time it holds the lock another caller may already - // have published a NEWER epoch (target 6 published while this call's - // snapshot still says 5). The old `!=`-gated unconditional CAS would move - // _last_gc_epoch BACKWARD 6->5, making epoch_diff negative and wrapping - // every survivor's unsigned age below - folding epochs out of order and - // manufacturing leak candidates / duplicate population samples. Only the - // caller whose target is strictly newer claims; a stale snapshot skips - // epoch accounting entirely (its survivors are still swept, their ages - // simply do not move for this no-op epoch). - bool is_epoch_owner = target_gc_epoch > claimed && + // account_epoch=false (track()'s table-overflow branch) makes this a + // pure reaper: it must NOT claim the epoch (otherwise the background + // sweep's fold for this epoch would be suppressed by the claimed + // _last_gc_epoch while this call never folds it - the epoch's population + // sample would be lost), must not age survivors (that accounting belongs + // to the epoch-advance pass), and must not touch the klass-population + // scratch (no fold will consume it here). It only reaps collected + // entries, which is what the overflow path needs to free table slots. + bool is_epoch_owner = account_epoch && target_gc_epoch != claimed && __atomic_compare_exchange_n(&_last_gc_epoch, &claimed, target_gc_epoch, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); - // On a lost CAS race `claimed` holds the actual current epoch (>= our - // stale target), so the raw diff is <= 0 for every non-owner; clamp so a - // survivor's unsigned age can never wrap. int epoch_diff = (int)(target_gc_epoch - claimed); + // A forced sweep can lose the epoch race: between this call's + // target_gc_epoch snapshot and the lock acquisition, a GC callback on + // another thread claimed a NEWER epoch (claimed > target_gc_epoch), + // making this raw difference negative. Aging survivors by a negative + // diff would rewind their ages and corrupt the Lindy oldest[] ordering + // and the generation-count signal. The newer claim already advanced + // every age by the full inter-epoch step, so this sweep contributes no + // aging of its own - clamp to zero. (The epoch-ownership CAS above is + // unaffected: is_epoch_owner already correctly reports false here.) if (epoch_diff < 0) { epoch_diff = 0; } @@ -231,7 +230,7 @@ void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { } } _klass_population_size = 0; - _klass_count_scratch_size = 0; + klassCountScratchReset(); _last_class_map_generation = current_class_map_generation; } @@ -263,8 +262,11 @@ void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { // session. Gated on is_epoch_owner (not !forced) so a forced // (table-overflow) sweep still contributes one population sample // per genuinely new GC epoch instead of silently dropping it. + // account_epoch=false sweeps never reach this (is_epoch_owner is + // forced false there). u32 klass_id = 0; - if (allow_resolve && resolve_budget > 0) { + if (allow_resolve && resolve_budget > 0 && + _table[target].cached_klass_id == 0) { // GetObjectClass + Class.getName() + StringDictionary lookup per // surviving entry, previously paid only at JFR-flush time (see // flush_table() below). Only affordable off the allocation-hot @@ -273,16 +275,29 @@ void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { // both pass allow_resolve=true; track()'s hot-path forced sweep // does not (see cleanup_table()'s own header comment). // - // Bounded per sweep (RESOLVE_BUDGET_PER_SWEEP): every resolution - // here runs under the EXCLUSIVE _table_lock, so an unbounded - // survivor count would stretch this sweep's critical section - // proportionally to the population (blocking every shared-lock - // scanner for the whole per-survivor JNI sequence). Entries past - // the budget fall through to the cached-id path below - the - // exact semantics the allow_resolve=false path already accepts - - // and the next epoch's sweep resolves the next tranche; the - // cached ids accumulated across sweeps keep most entries - // resolved anyway. + // The cached_klass_id == 0 gate is what keeps this affordable + // while holding the EXCLUSIVE table lock: an entry's class is + // immutable, so once resolved its id never changes (until the + // class-map generation reset above zeroes the whole cache). + // Steady state costs one u32 read per survivor; the full + // NewLocalRef + resolveKlassId() JNI round-trip is paid once + // per entry per class-map generation. Before this gate the + // per-survivor round-trip ran on EVERY sweep for entries whose + // resolution failed, and for every entry whenever + // _gc_generations was re-enabled. + // + // Bounded per sweep (RESOLVE_BUDGET_PER_SWEEP): the gate alone + // still lets a single sweep run the full JNI round-trip for the + // ENTIRE table when the class-map generation reset above has + // just zeroed every cached id (or when resolutions persistently + // fail) - stretching this sweep's exclusive-lock critical + // section proportionally to the population and blocking every + // shared-lock scanner for the whole per-survivor JNI sequence. + // Entries past the budget fall through to the cached-id path + // below - the exact semantics the allow_resolve=false path + // already accepts - and the next epoch's sweep resolves the + // next tranche; the cached ids accumulated across sweeps keep + // most entries resolved anyway. resolve_budget--; jobject ref = env->NewLocalRef(_table[target].ref); if (ref != nullptr) { @@ -301,17 +316,15 @@ void LivenessTracker::cleanup_table(bool forced, bool allow_resolve) { env->DeleteLocalRef(ref); } } else { - // track()'s table-overflow branch calls cleanup_table(true, - // false) synchronously from the allocation-sampling call stack - // (JVMTI SampledObjectAlloc callback). resolveKlassId() calls - // Class.getName(), a genuine Java-bytecode upcall (unlike the - // plain native jvmti->GetClassSignature() call - // ObjectSampler::recordAllocation already makes on this same - // callback stack) - too costly, and too re-entrancy-prone via - // the String allocation it can trigger, to run from there. Reuse - // whatever class id an earlier resolving sweep already resolved - // for this entry instead; if it was never resolved, this entry's - // sample for this epoch is dropped rather than resolving now. + // Either a non-resolving sweep (track()'s table-overflow branch + // calls cleanup_table(true, false) synchronously from the + // allocation-sampling call stack - JVMTI SampledObjectAlloc + // callback; resolveKlassId() calls Class.getName(), a genuine + // Java-bytecode upcall, too costly and re-entrancy-prone from + // there), an already-resolved entry on a resolving sweep, or an + // entry past this sweep's RESOLVE_BUDGET_PER_SWEEP. Reuse the + // cached id; if it was never resolved, this entry's sample for + // this epoch is dropped rather than resolving now. klass_id = _table[target].cached_klass_id; } if (klass_id != 0) { @@ -426,14 +439,10 @@ void LivenessTracker::insertOldestSample(KlassCountScratch &scratch, } jlong LivenessTracker::acquireLeakTag(u64 call_trace_id, jint tid) { - // Pool mutation is serialized by its own lock, not by _table_lock: this is - // called under the SHARED table lock (tagLeakInstances, BFS poll thread) - // while releaseLeakTag() runs under the EXCLUSIVE table lock (cleanup_table, - // GC-callback thread) - shared vs exclusive excludes those two from each - // other, but any future second shared-lock mutator would corrupt the LIFO - // free list. The dedicated lock keeps the pool correct independent of which - // table lock mode the caller holds. Lock order: _table_lock (any mode) is - // always acquired BEFORE _leak_tag_pool_lock, never the reverse. + // Pool lock: see _leak_tag_pool_lock's comment. Callers hold various + // combinations of the table lock (exclusive in cleanup_table's reaper, + // none in tagLeakInstances' unlocked tagging phase), so the pool cannot + // rely on either. _leak_tag_pool_lock.lock(); if (_leak_tag_free_count <= 0) { _leak_tag_pool_lock.unlock(); @@ -451,9 +460,28 @@ void LivenessTracker::releaseLeakTag(jlong tag) { return; } int idx = (int)(tag - LEAK_TAG_BASE); - // See acquireLeakTag()'s comment for the dedicated pool lock (and the - // _table_lock -> _leak_tag_pool_lock ordering). _leak_tag_pool_lock.lock(); + // Double-release guard: a zero/zero slot is free (see getLeakTagInfo()'s + // encoding note). Releasing an already-free tag would push its index onto + // the free list twice; a later acquireLeakTag() would then hand the same + // tag to two live objects and corrupt leak attribution. Found reachable + // in review: tagLeakInstances()' tag-adoption branch could let two table + // entries share one pool tag, so the second release hit this path. + if (_leak_tag_info[idx].call_trace_id == 0 && _leak_tag_info[idx].tid == 0) { + _leak_tag_pool_lock.unlock(); + Counters::increment(REFERENCE_CHAIN_LEAK_TAG_DOUBLE_RELEASE); + return; + } + // Bounds guard: never push past the pool. With the double-release guard + // above this is unreachable by construction (each index is pushed at most + // once between rebuilds), but the rebuild path (rebuildLeakTagFreeList) + // rewrites the whole list anyway, so a defensive cap here is cheap and + // keeps a future accounting bug from overflowing the array. + if (_leak_tag_free_count >= LEAK_TAG_POOL_SIZE) { + _leak_tag_pool_lock.unlock(); + Counters::increment(REFERENCE_CHAIN_LEAK_TAG_RELEASE_OVERFLOW); + return; + } _leak_tag_info[idx].call_trace_id = 0; _leak_tag_info[idx].tid = 0; _leak_tag_free_list[_leak_tag_free_count++] = idx; @@ -461,22 +489,25 @@ void LivenessTracker::releaseLeakTag(jlong tag) { } bool LivenessTracker::getLeakTagInfo(jlong tag, u64 *out_call_trace_id, - jint *out_tid) const { + jint *out_tid) { if (tag < LEAK_TAG_BASE || tag >= LEAK_TAG_BASE + LEAK_TAG_POOL_SIZE) { return false; } int idx = (int)(tag - LEAK_TAG_BASE); + // The two slot fields are written by acquireLeakTag() and releaseLeakTag() + // under the pool lock, from several distinct thread contexts + // (cleanup_table()'s reaper pass via flush_table()'s JFR cadence, + // track()'s overflow branch, maybeForceCleanup()'s background tick, and + // tagLeakInstances()' unlocked tagging phase). Read them under the same + // lock so a torn acquire/release cannot be observed as a spurious + // zero/zero (which would read as "not in use"). + _leak_tag_pool_lock.lock(); // releaseLeakTag() zeroes both fields, so a zero/zero slot means the tag // was released (or never acquired) - any other state is in use. (A slot // index comparison against _leak_tag_free_count proves nothing here: the // free list is a LIFO stack of indices, not an index-bounded region.) - // Read under the pool lock (see acquireLeakTag()'s comment): the intended - // caller (ReferenceChainTracker's BFS poll thread) holds no table lock in - // its polling path, and without this lock the releaseLeakTag() zeroing on - // the GC-callback thread would race these reads. - _leak_tag_pool_lock.lock(); - bool in_use = _leak_tag_info[idx].call_trace_id != 0 || - _leak_tag_info[idx].tid != 0; + bool in_use = + !(_leak_tag_info[idx].call_trace_id == 0 && _leak_tag_info[idx].tid == 0); if (in_use) { *out_call_trace_id = _leak_tag_info[idx].call_trace_id; *out_tid = _leak_tag_info[idx].tid; @@ -515,6 +546,19 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, u32 age; int distinct_ages; // age diversity of this entry's tid (computed below) bool leak_tag_recorded; // record already holds a pool tag (reused below) + // Snapshot payload (phase 1, under the shared lock): the tagging state + // machine below runs OUTSIDE _table_lock, so everything it reads from + // the record must be copied here - table slots move under cleanup_'s + // compaction and their payloads change, but these snapshots (plus the + // local ref) are stable for the duration of the poll. + jobject ref; // NewLocalRef of the entry's object (phase 1) + u64 call_trace_id; + u32 cached_klass_id; + u64 alloc_size; + u64 entry_time; // identity for the phase-3 write-back check + jlong recorded_leak_tag; + jlong write_leak_tag; // phase-3 result + bool write_back; }; // Stack scratch: matching entries are bounded by the tracking table's // small live population (~hundreds); tag in scan order beyond capacity. @@ -556,25 +600,38 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, // Check if this entry's (class, allocating thread) matches any // candidate: class alone is not enough - only the instances a // candidate's QUALIFYING tids allocated are in tagging scope. + // + // Klass-id pre-filter first: the scan runs over the ENTIRE table (up to + // MAX_TRACKING_TABLE_SIZE = 262144 entries) on the ~1s poll cadence, + // and the overwhelming majority of entries match no candidate klass. + // Comparing the entry's klass_id against the <= 5 candidate ids inline + // (sorted small set, no allocation) short-circuits before the nested + // candidate x qualifying-tid loops - the old shape cost up to + // ~10M compares per poll on a full table. u32 kid = _table[i].cached_klass_id; if (kid == 0) { continue; } - bool match = false; + int matched_candidate = -1; for (int k = 0; k < candidate_count; k++) { - if (candidates[k].klass_id != kid || - candidates[k].qualifying_tid_count <= 0) { - continue; + if (candidates[k].klass_id == kid && + candidates[k].qualifying_tid_count > 0) { + matched_candidate = k; + break; } - for (int q = 0; q < candidates[k].qualifying_tid_count; q++) { - if (candidates[k].qualifying_tids[q] == _table[i].tid) { + } + if (matched_candidate < 0) { + continue; + } + bool match = false; + { + const KlassCandidate &kc = candidates[matched_candidate]; + for (int q = 0; q < kc.qualifying_tid_count; q++) { + if (kc.qualifying_tids[q] == _table[i].tid) { match = true; break; } } - if (match) { - break; - } } if (!match) { continue; @@ -584,14 +641,34 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, // or the search restarts, and the state machine below must re-act on // the CURRENT JVMTI tag (re-establish, correlate, or leave alone). if (n_candidates < (int)(sizeof(scratch) / sizeof(scratch[0]))) { - scratch[n_candidates].table_idx = i; - scratch[n_candidates].tid = _table[i].tid; - scratch[n_candidates].age = _table[i].age; - scratch[n_candidates].distinct_ages = 0; - scratch[n_candidates].leak_tag_recorded = _table[i].leak_tag != 0; + TagCandidate &sc = scratch[n_candidates]; + sc.table_idx = i; + sc.tid = _table[i].tid; + sc.age = _table[i].age; + sc.distinct_ages = 0; + sc.recorded_leak_tag = _table[i].leak_tag; + sc.leak_tag_recorded = sc.recorded_leak_tag != 0; + sc.call_trace_id = _table[i].call_trace_id; + sc.cached_klass_id = _table[i].cached_klass_id; + sc.alloc_size = _table[i].alloc._size; + sc.entry_time = _table[i].time; + sc.write_leak_tag = 0; + sc.write_back = false; + // Local ref taken NOW, under the shared lock: past this point the + // record's ref can be reaped by a concurrent exclusive sweep, and the + // JVMTI tagging below needs a live jobject. + sc.ref = env->NewLocalRef(_table[i].ref); n_candidates++; } } + // Phase 2 runs unlocked: everything below reads only the scratch + // snapshots and object-level JVMTI/JNI state - no _table access - so the + // shared lock (which blocks the exclusive cleanup/flush sweeps) is + // released before the GetTag/SetTag/correlate work. This bounds the + // sweep-stall the old hold-across-JVMTI shape caused: the tagging + // machine ran NewLocalRef + jvmti->GetTag + jvmti->SetTag + a + // cross-singleton correlate call per candidate, all inside lockShared. + _table_lock.unlockShared(); // Compute per-tid distinct surviving ages (matching entries only - the // same diversity signal the epoch fold uses for clustering, computed here // directly from the tracked entries so the ranking reflects exactly the @@ -671,11 +748,12 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, // - no tag: plain SetTag (first tagging, or re-establishing after a // search restart wiped all tags via releaseSearchTags()). for (int c = 0; c < n_candidates; c++) { - u32 i = scratch[c].table_idx; - jobject ref = env->NewLocalRef(_table[i].ref); + TagCandidate &sc = scratch[c]; + jobject ref = sc.ref; if (ref == nullptr) { - // Object was collected between the null check and now - its record's - // tag (if any) is released by the GC cleanup path, nothing to do. + // Object was collected between the phase-1 NewLocalRef and now - its + // record's tag (if any) is released by the GC cleanup path, nothing + // to do. continue; } jlong existing = 0; @@ -685,38 +763,21 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, if (tag_err == JVMTI_ERROR_NONE && existing >= LEAK_TAG_BASE) { // Already carries a leak tag (ours, or one adopted below) - waiting // for the BFS interception. Make sure the record remembers it. - leak_tag = _table[i].leak_tag != 0 ? _table[i].leak_tag : existing; + leak_tag = sc.recorded_leak_tag != 0 ? sc.recorded_leak_tag : existing; } else if (tag_err == JVMTI_ERROR_NONE && existing > 0) { // Frontier tag: the BFS already admitted this object. Correlate the // existing entry rather than retagging - see the block comment above. - // - // Lock order (enforced, asymmetric): this call runs under THIS tracker's - // SHARED _table_lock and takes ReferenceChainTracker-internal locks - // (FrontierTable's SpinLock - held only inside individual - // insert/lookup/setLeakTag method bodies - and _resolved_chains_lock - // inside invalidateResolvedChain()). Every direction that could invert - // this is excluded today: RCT entry points into LT - // (resolveCandidateRepresentative, selectLeakCandidates, - // tagLeakInstances from pollWatchedTargets) take the table lock before - // touching any shared tracker state and are called with no RCT-internal - // lock held, so no path acquires _table_lock while already holding an - // RCT-internal lock. Any new RCT -> LT call made while holding an - // RCT-internal lock would invert the order and deadlock against this - // site. (Moving the correlate call outside the shared-lock section was - // rejected: it needs _table[i].ref/cached_klass_id, and the table - // index/weak ref can be compacted or reaped by a concurrent - // cleanup_table() once the shared lock is released - re-touching them - // unlocked would read freed/moved slots.) - leak_tag = _table[i].leak_tag; + leak_tag = sc.recorded_leak_tag; if (leak_tag == 0) { - leak_tag = acquireLeakTag(_table[i].call_trace_id, _table[i].tid); + leak_tag = acquireLeakTag(sc.call_trace_id, sc.tid); if (leak_tag == 0) { env->DeleteLocalRef(ref); + sc.ref = nullptr; continue; // pool exhausted - other candidates may still correlate } } if (!ReferenceChainTracker::instance()->correlateAdmittedLeakTag( - existing, leak_tag, _table[i].cached_klass_id)) { + existing, leak_tag, sc.cached_klass_id)) { // Not a live frontier tag after all (search just restarted) - // fall back to plain tagging. need_set = true; @@ -728,18 +789,19 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, // and frontier matching for the represented class. Preserve the // installed class tag untouched; the entry's pool-tag bookkeeping // stays as it is. - leak_tag = _table[i].leak_tag; + leak_tag = sc.recorded_leak_tag; need_set = false; } else { // No tag: first tagging, or re-establishment after a restart wiped // all tags (releaseSearchTags() clears every JVMTI tag while the // record keeps its pool tag - reusing it keeps pool accounting // stable across restarts). - leak_tag = _table[i].leak_tag; + leak_tag = sc.recorded_leak_tag; if (leak_tag == 0) { - leak_tag = acquireLeakTag(_table[i].call_trace_id, _table[i].tid); + leak_tag = acquireLeakTag(sc.call_trace_id, sc.tid); if (leak_tag == 0) { env->DeleteLocalRef(ref); + sc.ref = nullptr; break; // pool exhausted } } @@ -748,26 +810,18 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, if (need_set) { jvmti->SetTag(ref, leak_tag); } - // Store under the SHARED table lock: the only other writers are exclusive- - // lock holders (cleanup_table() zeroing a dead entry, start() reclaiming - // owned tags), which a shared holder excludes - and tagLeakInstances() - // itself runs on the single BFS poll thread, so no second shared-lock - // writer exists. Readers under either lock mode therefore always see a - // value from one of those serialized writers (aligned jlong stores are - // atomic on all supported architectures); a shared-mode reader may observe - // a stale 0 and re-tag - the correlate/re-establish state machine above is - // idempotent for that case. If a second shared-lock scanner ever starts - // writing leak_tag, this store (and those reads) must move under the - // exclusive lock or become atomic. - _table[i].leak_tag = leak_tag; + // Record write-back is deferred to phase 3 (the table lock is not held + // here); the summary below consumes the snapshot fields. + sc.write_leak_tag = leak_tag; + sc.write_back = true; tagged++; // Accumulate into the per-poll summary above instead of logging per // instance - one stable-pool poll re-logged all 256 tags' identical // lines every 1.4s before this. - u32 tagged_kid = _table[i].cached_klass_id; - jint tagged_tid = _table[i].tid; - u64 tagged_age = _table[i].age; - u64 tagged_size = _table[i].alloc._size; + u32 tagged_kid = sc.cached_klass_id; + jint tagged_tid = sc.tid; + u64 tagged_age = sc.age; + u64 tagged_size = sc.alloc_size; int g = 0; while (g < summary_count && (summary[g].klass_id != tagged_kid || summary[g].tid != tagged_tid)) { @@ -800,9 +854,36 @@ int LivenessTracker::tagLeakInstances(jvmtiEnv *jvmti, summary[g].max_size = tagged_size; } } - env->DeleteLocalRef(ref); + } + // Phase 3: write the resulting tags back to the records under a short + // shared-lock pass. A slot is only written when it still holds the SAME + // entry (identity = publish flag + allocation time + call_trace_id): a + // concurrent exclusive sweep may have reaped it (compaction moves slots), + // and writing a moved entry's tag would corrupt whoever reuses the slot. + // A skipped write-back is self-healing: the object carries the tag, the + // next poll's adoption branch re-records it, and start()'s pool rebuild + // reclaims any tag whose record is gone for good. + _table_lock.lockShared(); + for (int c = 0; c < n_candidates; c++) { + TagCandidate &sc = scratch[c]; + if (!sc.write_back) { + continue; + } + if (__atomic_load_n(&_table[sc.table_idx].ready, __ATOMIC_ACQUIRE) == 1 && + _table[sc.table_idx].time == sc.entry_time && + _table[sc.table_idx].call_trace_id == sc.call_trace_id) { + _table[sc.table_idx].leak_tag = sc.write_leak_tag; + } } _table_lock.unlockShared(); + // Release the phase-1 local refs (the state machine nulled ref on its own + // early-exit paths; those are already deleted). + for (int c = 0; c < n_candidates; c++) { + if (scratch[c].ref != nullptr) { + env->DeleteLocalRef(scratch[c].ref); + scratch[c].ref = nullptr; + } + } for (int g = 0; g < summary_count; g++) { TEST_LOG("LivenessTracker::tagLeakInstances summary klass_id=%u " "tid=%d tagged=%d need_set=%d min_age=%llu max_age=%llu " @@ -865,6 +946,45 @@ void LivenessTracker::insertThreadGen(KlassCountScratch &scratch, // the oldest[] array still captures instances from all threads. } +void LivenessTracker::klassCountScratchReset() { + _klass_count_scratch_size = 0; + memset(_klass_count_index, 0, sizeof(_klass_count_index)); +} + +LivenessTracker::KlassCountScratch *LivenessTracker::klassCountScratchSlot( + u32 klass_id, bool allocate) { + // Same mix() as the frontier's slot hashing (referenceChains.h); the + // klass_id need not be a tag, only uniformly distributed. + u64 i = (u64)(klass_id * 0x9E3779B97F4A7C15ULL) >> (64 - 9); + i &= KLASS_COUNT_INDEX_SLOTS - 1; + while (true) { + u16 slot = _klass_count_index[i]; + if (slot == 0) { + if (!allocate || + _klass_count_scratch_size >= MAX_KLASS_POPULATION_ENTRIES) { + return nullptr; + } + int idx = _klass_count_scratch_size++; + _klass_count_index[i] = (u16)(idx + 1); + KlassCountScratch &fresh = _klass_count_scratch[idx]; + // Full re-initialization: slots are reused across epochs (only the + // size + index are reset), so every field the fold reads must be + // cleared here, not just at construction. + fresh.klass_id = klass_id; + fresh.ages_count = 0; + fresh.ages_saturated = false; + fresh.oldest_count = 0; + fresh.thread_count = 0; + return &fresh; + } + KlassCountScratch &entry = _klass_count_scratch[slot - 1]; + if (entry.klass_id == klass_id) { + return &entry; + } + i = (i + 1) & (KLASS_COUNT_INDEX_SLOTS - 1); + } +} + void LivenessTracker::accumulateKlassCount(u32 klass_id, jlong age, jweak sample_source, jint tid) { @@ -873,45 +993,44 @@ void LivenessTracker::accumulateKlassCount(u32 klass_id, jlong age, // number of unique age values. This is the "generation // count" — if new instances keep arriving while old ones // survive, the number of distinct ages grows. - for (int i = 0; i < _klass_count_scratch_size; i++) { - if (_klass_count_scratch[i].klass_id == klass_id) { - auto &entry = _klass_count_scratch[i]; - // Per-class age dedup: only count each age once for the klass' - // generation count. But per-site tracking and oldest[] must see - // EVERY surviving object, not just the first per age — so those - // run unconditionally below, outside this dedup check. - bool age_seen = false; - for (u32 a : entry.ages) { - if (a == (u32)age) { - age_seen = true; - break; - } + // Direct-indexed (klassCountScratchSlot): the caller holds the exclusive + // table lock and this runs once per surviving table entry, so the old + // linear scan over _klass_count_scratch scaled O(entries x 256). + KlassCountScratch *entry = klassCountScratchSlot(klass_id, true); + if (entry != nullptr) { + // Per-class age dedup: only count each age once for the klass' + // generation count. But per-site tracking and oldest[] must see + // EVERY surviving object, not just the first per age — so those + // run unconditionally below, outside this dedup check. + bool age_seen = false; + for (int a = 0; a < entry->ages_count; a++) { + if (entry->ages[a] == (u32)age) { + age_seen = true; + break; } - if (!age_seen) { - entry.ages.push_back((u32)age); + } + if (!age_seen) { + if (entry->ages_count < KlassCountScratch::MAX_DISTINCT_AGES) { + entry->ages[entry->ages_count++] = (u32)age; + } else { + // Saturated: a klass this rich in distinct surviving ages in one + // epoch is already the strongest possible generation-count signal; + // stop growing (and stop paying the dedup scan) rather than + // allocating. See MAX_DISTINCT_AGES's comment. + entry->ages_saturated = true; } - // Track top-N oldest instances (Lindy bias): insert this sample - // into the oldest[] array, sorted by age descending, capped at - // MAX_OLDEST_SAMPLES. Runs for every object, not just new ages. - insertOldestSample(entry, sample_source, (u32)age, tid); - // Track per-thread distinct surviving generations (Cork/Swat - // heuristic): add this object's age to its thread's age set. - // Runs for every object — the thread's generation cardinality is - // the leak signal, and it must see all surviving objects to be - // accurate. - insertThreadGen(entry, tid, (u32)age); - return; } - } - if (_klass_count_scratch_size < MAX_KLASS_POPULATION_ENTRIES) { - KlassCountScratch &slot = _klass_count_scratch[_klass_count_scratch_size++]; - slot.klass_id = klass_id; - slot.ages.clear(); - slot.ages.push_back((u32)age); - slot.oldest_count = 0; - slot.thread_count = 0; - insertOldestSample(slot, sample_source, (u32)age, tid); - insertThreadGen(slot, tid, (u32)age); + // Track top-N oldest instances (Lindy bias): insert this sample + // into the oldest[] array, sorted by age descending, capped at + // MAX_OLDEST_SAMPLES. Runs for every object, not just new ages. + insertOldestSample(*entry, sample_source, (u32)age, tid); + // Track per-thread distinct surviving generations (Cork/Swat + // heuristic): add this object's age to its thread's age set. + // Runs for every object — the thread's generation cardinality is + // the leak signal, and it must see all surviving objects to be + // accurate. + insertThreadGen(*entry, tid, (u32)age); + return; } // else: this epoch's scratch snapshot already holds // MAX_KLASS_POPULATION_ENTRIES distinct surviving klasses - klass_id's @@ -972,6 +1091,9 @@ jweak LivenessTracker::recordKlassPopulationSampleLocked( _klass_population[slot].ring_fill = 0; _klass_population[slot].consecutive_positive = 0; _klass_population[slot].cached_slope = 0.0; + // A fresh/evicted slot has never been probed - force the staleness + // check on this fold. + _klass_population[slot].last_rep_probe_epoch = 0; // A reused (evicted) slot's previous class's per-tid trends must not // leak onto the new one, same as the fields above. _klass_population[slot].tid_trend_count = 0; @@ -1062,7 +1184,7 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, KlassCountScratch &s = _klass_count_scratch[i]; TEST_LOG("LivenessTracker::foldKlassCountsLocked scratch[%d] klass_id=%u gen_count=%zu " "thread_count=%d oldest_count=%d", - i, s.klass_id, s.ages.size(), s.thread_count, s.oldest_count); + i, s.klass_id, s.ages_count, s.thread_count, s.oldest_count); for (int ti = 0; ti < s.thread_count; ti++) { TEST_LOG(" thread[%d] tid=%d age_count=%u", ti, (int)s.threads[ti].tid, s.threads[ti].age_count); @@ -1071,7 +1193,7 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, bool created; jweak evicted[KlassPopulationEntry::MAX_REPRESENTATIVES_PER_KLASS]; int evicted_count = 0; - recordKlassPopulationSampleLocked(s.klass_id, (u32)s.ages.size(), + recordKlassPopulationSampleLocked(s.klass_id, (u32)s.ages_count, epoch, &slot, &created, evicted, &evicted_count, KlassPopulationEntry::MAX_REPRESENTATIVES_PER_KLASS); @@ -1110,21 +1232,35 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, bool need_mint = created || _klass_population[slot].representative_count == 0; if (!need_mint) { - // Check if all representatives are stale - bool any_live = false; - for (int r = 0; r < _klass_population[slot].representative_count; r++) { - jweak rep = _klass_population[slot].representatives[r]; - if (rep != nullptr) { - jobject probe = env->NewLocalRef(rep); - if (probe != nullptr) { - any_live = true; + // Staleness probe amortization: a jweak's pointer never nulls on + // collection, so the only way to detect a dead representative is a + // NewLocalRef probe - JNI churn under the exclusive table lock when + // run for every klass every epoch. A live representative stays live + // until collected, and a missed collection costs one epoch with a + // stale rep (the minting retry picks it up on the next probe), so + // probing every REP_PROBE_EPOCH_INTERVAL epochs bounds the gap + // without paying the JNI round-trips per sweep. + constexpr u64 REP_PROBE_EPOCH_INTERVAL = 4; + bool probe_due = + epoch - _klass_population[slot].last_rep_probe_epoch >= + REP_PROBE_EPOCH_INTERVAL; + if (probe_due) { + _klass_population[slot].last_rep_probe_epoch = epoch; + bool any_live = false; + for (int r = 0; r < _klass_population[slot].representative_count; r++) { + jweak rep = _klass_population[slot].representatives[r]; + if (rep != nullptr) { + jobject probe = env->NewLocalRef(rep); + if (probe != nullptr) { + any_live = true; + env->DeleteLocalRef(probe); + break; + } env->DeleteLocalRef(probe); - break; } - env->DeleteLocalRef(probe); } + need_mint = !any_live; } - need_mint = !any_live; } // Compute the dominant allocating thread (highest generation // cardinality — most distinct surviving GC ages). This reuses @@ -1193,19 +1329,6 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, jobject strong = env->NewLocalRef(s.oldest[r].ref); if (strong != nullptr) { jweak rep = env->NewWeakGlobalRef(strong); - if (rep == nullptr) { - // NewWeakGlobalRef failed under memory pressure (an - // OutOfMemoryError may be pending). Store no representative - // and stop minting: continuing JNI calls with a pending - // exception is undefined behavior, and a null rep would - // corrupt representative_count. The next epoch's need_mint - // retry re-attempts minting. - if (env->ExceptionCheck()) { - env->ExceptionClear(); - } - env->DeleteLocalRef(strong); - break; - } int idx = _klass_population[slot].representative_count++; _klass_population[slot].representatives[idx] = rep; _klass_population[slot].rep_tids[idx] = dominant_tid; @@ -1225,15 +1348,6 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, jobject strong = env->NewLocalRef(s.oldest[r].ref); if (strong != nullptr) { jweak rep = env->NewWeakGlobalRef(strong); - if (rep == nullptr) { - // Same OOM handling as the dominant-thread loop above: no null - // representative, pending exception cleared, minting stops. - if (env->ExceptionCheck()) { - env->ExceptionClear(); - } - env->DeleteLocalRef(strong); - break; - } int idx = _klass_population[slot].representative_count++; _klass_population[slot].representatives[idx] = rep; _klass_population[slot].rep_tids[idx] = s.oldest[r].tid; @@ -1253,7 +1367,7 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, minted, s.klass_id, (int)dominant_tid, dominant_gens); } } - _klass_count_scratch_size = 0; + klassCountScratchReset(); // Zero-sample pass: a klass whose every tracked instance died this epoch // never appears in _klass_count_scratch, so the loop above never refreshes @@ -1286,8 +1400,8 @@ void LivenessTracker::foldKlassCountsLocked(JNIEnv *env, u64 epoch, } bool LivenessTracker::hasQualifyingGrowth(const KlassPopulationEntry &entry) const { - RingThirdsStats stats; - if (!ringThirdsStats( + RingWindowStats stats; + if (!ringWindowStats( entry.ring_head, entry.ring_fill, KLASS_POPULATION_RING_SIZE, KLASS_POPULATION_MIN_FILL_FOR_TREND, [&entry](int i) { return (double)entry.count_ring[i]; }, &stats)) { @@ -1322,8 +1436,8 @@ bool LivenessTracker::hasQualifyingGrowth(const KlassPopulationEntry &entry) con bool LivenessTracker::hasQualifyingTidGrowth( const KlassPopulationEntry::TidTrend &trend) const { - RingThirdsStats stats; - if (!ringThirdsStats( + RingWindowStats stats; + if (!ringWindowStats( trend.ring_head, trend.ring_fill, KlassPopulationEntry::TID_TREND_RING_SIZE, TID_TREND_MIN_FILL_FOR_TREND, @@ -1514,8 +1628,8 @@ bool LivenessTracker::heapFloorRising() const { // loadAcquire() here is what makes the payload writes below visible. u8 fill = loadAcquire(_heap_floor_ring_fill); u8 head = loadAcquire(_heap_floor_ring_head); - RingThirdsStats stats; - if (!ringThirdsStats( + RingWindowStats stats; + if (!ringWindowStats( head, fill, KLASS_POPULATION_RING_SIZE, KLASS_POPULATION_MIN_FILL_FOR_TREND, [this](int i) { return (double)load(_heap_floor_ring[i]); }, @@ -1587,8 +1701,8 @@ double LivenessTracker::secondsToOOM() const { (int)fill, KLASS_POPULATION_MIN_FILL_FOR_TREND); return -1; } - RingThirdsStats time_stats; - if (!ringThirdsStats( + RingWindowStats time_stats; + if (!ringWindowStats( head, fill, KLASS_POPULATION_RING_SIZE, KLASS_POPULATION_MIN_FILL_FOR_TREND, [this](int i) { return (double)load(_heap_floor_time_ring[i]); }, @@ -1606,45 +1720,58 @@ double LivenessTracker::secondsToOOM() const { const char *best_source = "none"; double best_recent_mean = 0; - RingThirdsStats heap_bytes; + RingWindowStats heap_bytes; if (max_heap > 0 && - ringThirdsStats(head, fill, KLASS_POPULATION_RING_SIZE, + ringWindowStats(head, fill, KLASS_POPULATION_RING_SIZE, KLASS_POPULATION_MIN_FILL_FOR_TREND, [this](int i) { return (double)load(_heap_floor_ring[i]); }, &heap_bytes) && corroborateRecentHalf(head, fill, KLASS_POPULATION_RING_SIZE, HEAP_FLOOR_RECENT_HALF_MIN_FILL, [this](int i) { return (double)load(_heap_floor_ring[i]); })) { - double remaining = (double)max_heap - heap_bytes.recent_mean; - double secs = remaining <= 0 - ? 0 - : (remaining * time_delta_ns) / - (heap_bytes.recent_mean - heap_bytes.earliest_mean) / 1e9; - if (best_seconds < 0 || secs < best_seconds) { - best_seconds = secs; - best_source = "heap"; - best_recent_mean = heap_bytes.recent_mean; + // Denominator is the full-window byte delta (endpoints of the fitted + // line). A dip-then-recover window can pass corroborateRecentHalf() + // (its recent half rises) while the full-window endpoints agree, making + // this delta ~0 - an unguarded division projects +inf seconds, which + // silently disables the urgency ramp from this boundary forever (best + // -seconds would be set to +inf and never beaten). Skip the boundary + // instead: no usable rate, no projection. + double heap_delta = heap_bytes.recent_mean - heap_bytes.earliest_mean; + if (heap_delta > 0) { + double remaining = (double)max_heap - heap_bytes.recent_mean; + double secs = remaining <= 0 + ? 0 + : (remaining * time_delta_ns) / heap_delta / 1e9; + if (best_seconds < 0 || secs < best_seconds) { + best_seconds = secs; + best_source = "heap"; + best_recent_mean = heap_bytes.recent_mean; + } } } - RingThirdsStats container_bytes; + RingWindowStats container_bytes; if (container_limit > 0 && - ringThirdsStats(head, fill, KLASS_POPULATION_RING_SIZE, + ringWindowStats(head, fill, KLASS_POPULATION_RING_SIZE, KLASS_POPULATION_MIN_FILL_FOR_TREND, [this](int i) { return (double)load(_container_mem_ring[i]); }, &container_bytes) && corroborateRecentHalf(head, fill, KLASS_POPULATION_RING_SIZE, HEAP_FLOOR_RECENT_HALF_MIN_FILL, [this](int i) { return (double)load(_container_mem_ring[i]); })) { - double remaining = (double)container_limit - container_bytes.recent_mean; - double secs = remaining <= 0 - ? 0 - : (remaining * time_delta_ns) / - (container_bytes.recent_mean - container_bytes.earliest_mean) / 1e9; - if (best_seconds < 0 || secs < best_seconds) { - best_seconds = secs; - best_source = "container"; - best_recent_mean = container_bytes.recent_mean; + // Same zero-denominator guard as the heap boundary above. + double container_delta = + container_bytes.recent_mean - container_bytes.earliest_mean; + if (container_delta > 0) { + double remaining = (double)container_limit - container_bytes.recent_mean; + double secs = remaining <= 0 + ? 0 + : (remaining * time_delta_ns) / container_delta / 1e9; + if (best_seconds < 0 || secs < best_seconds) { + best_seconds = secs; + best_source = "container"; + best_recent_mean = container_bytes.recent_mean; + } } } @@ -1677,11 +1804,7 @@ int LivenessTracker::selectLeakCandidates(KlassCandidate *out, int max) { // for why an aggregate, non-attributed signal can only raise or lower the // bar uniformly, never reorder candidates against each other. Lock-free // (heapFloorRising()'s own comment), so no relation to _table_lock below. - // Computed once: the trailing diagnostic TEST_LOG at the end of this - // function reuses this cached result instead of re-evaluating the full - // O(ring_fill) ring scan a second time per BFS-thread wake. - const bool heap_floor_rising = heapFloorRising(); - const int required_hysteresis = heap_floor_rising + const int required_hysteresis = heapFloorRising() ? LEAK_TREND_HYSTERESIS_CORROBORATED : LEAK_TREND_HYSTERESIS_BASE; @@ -1770,7 +1893,7 @@ int LivenessTracker::selectLeakCandidates(KlassCandidate *out, int max) { _table_lock.unlockShared(); TEST_LOG("LivenessTracker::selectLeakCandidates returning %d candidates (required_hysteresis=%d, heapFloorRising=%d)", count, required_hysteresis, - (int)heap_floor_rising); + (int)heapFloorRising()); return count; } @@ -1922,25 +2045,22 @@ void LivenessTracker::flush_table(std::set *tracked_thread_ids) { if (_table[i].cached_klass_id != 0) { // Already resolved by cleanup_table()'s survivor loop this epoch // (resolveKlassId(), only when _gc_generations is enabled) - reuse - // it instead of repeating the GetObjectClass+Class.getName()+ - // lookupClass() JNI round-trip for the same object. + // it instead of repeating the JNI round-trip for the same object. class_id = _table[i].cached_klass_id; } else { - jclass clz = env->GetObjectClass(ref); - jstring name_str = (jstring)env->CallObjectMethod(clz, _Class_getName); - env->DeleteLocalRef(clz); - jniExceptionCheck(env); - // name_str can be null if the call above threw and - // jniExceptionCheck() cleared the pending exception rather than - // propagating it - GetStringUTFChars()/ReleaseStringUTFChars() - // require a non-null jstring (mirrors resolveKlassId()'s own guard). - if (name_str != nullptr) { - const char *name = env->GetStringUTFChars(name_str, nullptr); - if (name != nullptr) { - class_id = Profiler::instance()->lookupClass(name, strlen(name)); - env->ReleaseStringUTFChars(name_str, name); - } - env->DeleteLocalRef(name_str); + // Same sequence as resolveKlassId(): GetClassSignature + + // normalizeClassSignature + lookupClass (slash-notation key), NOT + // the old Class.getName() path. The two produce DIFFERENT + // StringDictionary keys for the same class ("com/foo/Bar" vs + // "com.foo.Bar"), so a cache miss resolved through getName() could + // emit a different class id for the same class than the cache hit + // path right above - two id spaces in one event stream. resolve + // KlassId() also caches back into the entry, so a later sweep pays + // one u32 read instead of this round-trip. + int resolved = (int)resolveKlassId(env, ref); + class_id = resolved; + if (resolved > 0) { + _table[i].cached_klass_id = (u32)resolved; } } @@ -2033,6 +2153,9 @@ Error LivenessTracker::start(Arguments &args) { } } _table_lock.unlock(); + // Rebuild under the pool lock: tagLeakInstances()' unlocked tagging phase + // and getLeakTagInfo() readers may run concurrently with start(). + _leak_tag_pool_lock.lock(); int free_w = 0; for (int i = 0; i < LEAK_TAG_POOL_SIZE; i++) { if (tag_owned[i]) { @@ -2044,6 +2167,7 @@ Error LivenessTracker::start(Arguments &args) { free_w++; } _leak_tag_free_count = free_w; + _leak_tag_pool_lock.unlock(); if (!_enabled) { // disabled return Error::OK; @@ -2094,15 +2218,6 @@ Error LivenessTracker::initialize(Arguments &args) { // start gets the correct setting even when the table persists across recordings. _record_heap_usage = args._record_heap_usage; - // Fresh recording: no chase is open, so no watched tids and no urgency - // boost may leak in from a previous recording's lifecycle. This MUST run on - // EVERY start - not only the first initialization: the `_initialized` - // early return below would otherwise leave a previous recording's watched - // thread set and urgent-tracking state active, admitting the new - // recording's unrelated allocations at 100 percent. - __atomic_store_n(&_watched_tid_count, 0, __ATOMIC_RELEASE); - __atomic_store_n(&_urgent_tracking, false, __ATOMIC_RELEASE); - if (_initialized) { // if the tracker was previously initialized return the stored result for // consistency this hack also means that if the profiler is started with @@ -2168,6 +2283,11 @@ Error LivenessTracker::initialize(Arguments &args) { // decision in track() is an integer compare rather than a double multiply. _subsample = SubsampleRate(args._live_samples_ratio); + // Fresh recording: no chase is open, so no watched tids and no urgency + // boost may leak in from a previous recording's lifecycle. + __atomic_store_n(&_watched_tid_count, 0, __ATOMIC_RELEASE); + __atomic_store_n(&_urgent_tracking, false, __ATOMIC_RELEASE); + _table_size = 0; _table_cap = std::min(2048, _table_max_cap); // with default 512k sampling interval, it's @@ -2212,7 +2332,28 @@ static ThreadLocal skipped; // per thread and probabilistic by design). bool LivenessTracker::admitForTracking(jint tid) { if (__atomic_load_n(&_urgent_tracking, __ATOMIC_ACQUIRE)) { - return true; + // Volume backstop for the urgency boost (100% admission, bypassing even + // the subsample draw): once the table is at its high-water mark the + // boost's own cost - track()'s overflow branch firing a forced sweep on + // the SampledObjectAlloc callback stack, per admission - outweighs the + // extra diagnostic coverage. Fall back to the watched-tid-only boost, + // which keeps 100% admission exactly where the leak data value is (the + // candidate sites) while every other thread goes back to the configured + // subsample ratio. Admissions at the high-water mark are still counted + // so the degradation is observable in production. + if (_table_max_cap > 0 && _table_size >= _table_max_cap) { + Counters::increment(LIVENESS_URGENT_BOOST_BACKED_OFF); + int n = __atomic_load_n(&_watched_tid_count, __ATOMIC_ACQUIRE); + for (int i = 0; i < n; i++) { + if (_watched_tids[i] == tid) { + return true; + } + } + // Fall through to the configured-ratio draw below. + } else { + Counters::increment(LIVENESS_URGENT_BOOST_ADMITS); + return true; + } } // Count+array two-phase publish (noteSelectedCandidates() writes the // slots before release-storing the count): the acquire load pairs with @@ -2221,7 +2362,7 @@ bool LivenessTracker::admitForTracking(jint tid) { // for why RELAXED is not an option on arm64. int n = __atomic_load_n(&_watched_tid_count, __ATOMIC_ACQUIRE); for (int i = 0; i < n; i++) { - if (__atomic_load_n(&_watched_tids[i], __ATOMIC_RELAXED) == tid) { + if (_watched_tids[i] == tid) { return true; } } @@ -2275,15 +2416,12 @@ void LivenessTracker::noteSelectedCandidates(const KlassCandidate *candidates, } full: // Copy into the live array before publishing the count (two-phase - // publish mirrored by admitForTracking()'s acquire load). Slot accesses - // are ATOMIC (relaxed): the poll thread rewrites slots while allocation - // threads can still read them under an acquire count load that observed - // the OLD count - plain loads/stores there are a C++ data race (UB). - // A reader mid-scan may transiently mix old and new slot values below the - // OLD count - harmless: admission is advisory, and the worst case is one + // publish mirrored by admitForTracking()'s acquire load). A reader + // mid-scan may transiently mix old and new slot values below the OLD + // count - harmless: admission is advisory, and the worst case is one // allocation admitted per the previous poll's set. for (int i = 0; i < n; i++) { - __atomic_store_n(&_watched_tids[i], tids[i], __ATOMIC_RELAXED); + _watched_tids[i] = tids[i]; } __atomic_store_n(&_watched_tid_count, n, __ATOMIC_RELEASE); if (n > 0) { @@ -2293,8 +2431,7 @@ void LivenessTracker::noteSelectedCandidates(const KlassCandidate *candidates, TEST_LOG("LivenessTracker::noteSelectedCandidates watched tids[%d]:", n); for (int i = 0; i < n; i++) { - TEST_LOG(" watched tid=%d", - __atomic_load_n(&_watched_tids[i], __ATOMIC_RELAXED)); + TEST_LOG(" watched tid=%d", _watched_tids[i]); } } } @@ -2349,7 +2486,18 @@ void LivenessTracker::track(JNIEnv *env, AllocEvent &event, jint tid, } bool retried = false; retry: - if (!_table_lock.tryLockShared()) { + // EXCLUSIVE, not shared: the fill below RE-USES slots (idx < _table_cap + // after _table_size wraps past reaped entries), and a shared lock does + // not exclude other shared holders - a scanner (tagLeakInstances(), + // getLiveTraceIds()) that had already passed its ready==1 check for the + // slot's OLD entry could read the payload mid-fill and get a torn mix of + // old and new values. The unpublish/publish dance cannot close that + // (the scanner's check happened before the unpublish). Exclusive here + // serializes fills against scanners; contention is bounded because this + // lock is only taken once per SAMPLED allocation (admitForTracking()'s + // subsample draw above rejects the unsampled majority before any lock + // is taken). + if (!_table_lock.tryLock()) { // we failed to add the weak reference to the table so it won't get cleaned // up otherwise env->DeleteWeakGlobalRef(ref); @@ -2385,7 +2533,7 @@ void LivenessTracker::track(JNIEnv *env, AllocEvent &event, jint tid, __atomic_store_n(&_table[idx].ready, 1, __ATOMIC_RELEASE); } - _table_lock.unlockShared(); + _table_lock.unlock(); if (idx == _table_cap) { if (!retried) { @@ -2396,7 +2544,13 @@ void LivenessTracker::track(JNIEnv *env, AllocEvent &event, jint tid, // space. allow_resolve=false: this runs synchronously on the // allocation-sampling callback stack (see cleanup_table()'s own header // comment for why resolveKlassId() is unsafe here). - cleanup_table(true, false); + // account_epoch=false: pure reaper - no epoch claim, no survivor + // aging, no population fold. The fold's nested per-(klass,tid) loops + // ran on this hot callback stack under the exclusive table lock; + // the background/GC sweeps (which claim the epoch) do the full + // accounting, so the overflow path only pays for the reaping it + // actually needs. + cleanup_table(true, false, false); if (_table_cap < _table_max_cap) { diff --git a/ddprof-lib/src/main/cpp/livenessTracker.h b/ddprof-lib/src/main/cpp/livenessTracker.h index a942d66479..136e599609 100644 --- a/ddprof-lib/src/main/cpp/livenessTracker.h +++ b/ddprof-lib/src/main/cpp/livenessTracker.h @@ -101,9 +101,8 @@ typedef struct KlassPopulationEntry { // without this counter it gets reported as a leak candidate almost as // often as a real leak does. u8 consecutive_positive; - // Slope (regression value at the ring's newest sample minus the value at - // its oldest sample - see ringThirdsStats, livenessTracker.cpp) as of the - // last push, computed and cached by hasQualifyingGrowth() alongside + // Slope (recent third's mean minus earliest third's mean) as of the last + // push, computed and cached by hasQualifyingGrowth() alongside // consecutive_positive above - selectLeakCandidates() reads this directly // for ranking instead of re-scanning the ring: the ring only changes on // push, so a second scan at scan time would just recompute the same @@ -113,6 +112,13 @@ typedef struct KlassPopulationEntry { // existed. mutable double cached_slope; u64 last_updated_epoch; // _gc_epoch value as of the last write, for LRU + // Epoch of the last representative-staleness JNI probe (foldKlassCounts + // Locked()'s NewLocalRef check of each stored jweak). The probe is + // amortized: without this field it ran for every klass with survivors on + // EVERY epoch (up to 256 klasses x 3 jweaks of JNI churn per sweep) even + // when a representative is long-lived and nothing changed. See + // REP_PROBE_EPOCH_INTERVAL. + u64 last_rep_probe_epoch; // eviction when the table is full // Stable per-class identifier, from the process-wide, negative-tag // allocator shared with ReferenceChainTracker (classTagAllocator.h) - NOT @@ -220,8 +226,8 @@ class alignas(alignof(SpinLock)) LivenessTracker { // a klass starts being tracked." constexpr static int KLASS_POPULATION_MIN_FILL_FOR_TREND = 10; // Per-tid trend gate's own minimum fill (see KlassPopulationEntry:: - // TidTrend): 6 samples of the 16-slot ring leaves a 2-3 sample - // regression, small enough that a per-tid qualification (6 pushes + + // TidTrend): 6 samples of the 16-slot ring leaves a 2-3 sample thirds + // comparison, small enough that a per-tid qualification (6 pushes + // the 3-5 hysteresis epochs, ~9-11) completes no later than the klass // gate's own ~13-15-epoch latency, keeping the klass ring the sole // latency driver for candidate emergence. @@ -268,11 +274,11 @@ class alignas(alignof(SpinLock)) LivenessTracker { // surviving instance count: a klass whose survivors keep spanning more // distinct allocation cohorts over time is one where old instances are // not dying as new ones arrive, which is the leak shape this gate looks - // for. The gate itself is a single condition - the ring's regression end - // value must exceed its start value by a meaningful + // for. The gate itself is a single condition - the recent third's mean + // generation count must exceed the earliest third's by a meaningful // margin (LEAK_GROWTH_REL_MIN/LEAK_GROWTH_ABS_MIN, whichever is larger). - // An earlier revision of this gate also required the recent half's - // *minimum* to exceed the full window's minimum (a floor-rise check, + // An earlier revision of this gate also required the recent third's + // *minimum* to exceed the earliest third's minimum (a floor-rise check, // to reject oscillations whose peak alone passes the growth test) - that // check was tuned for raw population counts (which can run into the // thousands) and does not transfer to generation counts, which are small @@ -300,7 +306,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { constexpr static int LEAK_TREND_HYSTERESIS_CORROBORATED = 3; // --- Aggregate post-GC heap floor (heapFloorRising() below) --- - // Same "regression growth + floor rise" shape as the per-klass test + // Same "mean-of-thirds growth + floor rise" shape as the per-klass test // above, applied to a single global ring of post-GC live heap size // instead of one klass's sampled population - see this class's own ring // (_heap_floor_ring below). Its own thresholds are deliberately looser @@ -330,10 +336,6 @@ class alignas(alignof(SpinLock)) LivenessTracker { // comment below). _watched_tids is two-phase-published: slots first, then // _watched_tid_count with RELEASE (admitForTracking()'s ACQUIRE load pairs // with it) - a reader never trusts a slot beyond the count it observed. - // Every slot access is ATOMIC (__atomic_* builtins, relaxed): the poll - // thread rewrites slots while allocation threads may still read them under - // an acquire count load that observed the OLD count - plain accesses there - // are a C++ data race (UB), not just a staleness artifact. jint _watched_tids[KlassCandidate::MAX_QUALIFYING_TIDS]; volatile int _watched_tid_count; volatile bool _urgent_tracking; @@ -469,9 +471,23 @@ class alignas(alignof(SpinLock)) LivenessTracker { typedef struct KlassCountScratch { u32 klass_id; // Distinct GC ages (generations) of surviving tracked instances - // of this klass at this epoch. The size of this vector - // is the klass' generation count. - std::vector ages; + // of this klass at this epoch. The count (ages_count, saturated + // at MAX_DISTINCT_AGES) is the klass' generation count. + // + // Fixed-size (was a std::vector): push_back ran while the + // EXCLUSIVE table lock is held, including on the forced + // cleanup_table(true, false) path invoked synchronously from + // track()'s table-overflow branch on the JVMTI SampledObjectAlloc + // callback stack - a malloc there is exactly what the project's + // allocation-free GC-path preference forbids. The generation-count + // signal saturates: a klass with >= MAX_DISTINCT_AGES distinct + // surviving ages in a single epoch is already an extreme leak + // signal, and the population ring it feeds is only 30 samples + // (KLASS_POPULATION_RING_SIZE). + static constexpr int MAX_DISTINCT_AGES = 64; + u32 ages[MAX_DISTINCT_AGES]; + int ages_count; + bool ages_saturated; // set once ages_count hits MAX_DISTINCT_AGES // Top-N oldest surviving instances of this klass seen this epoch, // sorted by age descending. Used by foldKlassCountsLocked() to mint // representatives biased toward long-lived instances (Lindy effect: @@ -516,6 +532,26 @@ class alignas(alignof(SpinLock)) LivenessTracker { } KlassCountScratch; KlassCountScratch _klass_count_scratch[MAX_KLASS_POPULATION_ENTRIES]; int _klass_count_scratch_size; + // Open-addressed direct index over _klass_count_scratch, keyed by + // klass_id: slot value is scratch_index + 1 (0 = empty), hashed by the + // same mix() convention the frontier uses. accumulateKlassCount() runs + // once per SURVIVING table entry (up to MAX_TRACKING_TABLE_SIZE = 262144) + // while the exclusive table lock is held; the old linear scan over the + // 256 scratch entries made a fully-populated sweep O(entries x klasses). + // With the index each accumulate is one probe chain. Rebuilt by + // klassCountScratchReset(); MAX_KLASS_POPULATION_ENTRIES = 256 entries + // in a 512-slot table keeps the load factor at 0.5. + static constexpr int KLASS_COUNT_INDEX_SLOTS = MAX_KLASS_POPULATION_ENTRIES * 2; + u16 _klass_count_index[KLASS_COUNT_INDEX_SLOTS]; + + // Zeroes the scratch (size + every KlassCountScratch field the fold + // reads) and the direct index. Every _klass_count_scratch_size = 0 site + // must go through this so the index can never name a stale slot. + void klassCountScratchReset(); + + // Index lookup: returns the scratch slot for klass_id or nullptr (and + // takes an empty slot when `allocate` and scratch has capacity). + KlassCountScratch *klassCountScratchSlot(u32 klass_id, bool allocate); // Profiler::classMap()'s generation as of the last cleanup_table() call // that checked it, mirroring ReferenceChainTracker::_last_class_map_generation @@ -552,15 +588,15 @@ class alignas(alignof(SpinLock)) LivenessTracker { // instance of the same class) and correlate chains with HeapLiveObject. static constexpr int LEAK_TAG_POOL_SIZE = 256; static constexpr jlong LEAK_TAG_BASE = 0x40000000LL; - // Serializes the pool's free list and _leak_tag_info entries. NOT _table_lock: - // acquireLeakTag() runs under the SHARED table lock (tagLeakInstances) while - // releaseLeakTag() runs under the EXCLUSIVE one (cleanup_table) - a shared - // holder excludes the exclusive one, but only this dedicated lock makes the - // pool safe for any future second shared-lock mutator, and getLeakTagInfo() - // takes NO table lock at all (BFS poll thread). Lock order: _table_lock - // (any mode) is always acquired BEFORE _leak_tag_pool_lock, never the - // reverse; no path acquires _table_lock while holding the pool lock. - mutable SpinLock _leak_tag_pool_lock; + // Own lock for the pool state below (free list + info slots). The pool + // is mutated from cleanup_table()'s reaper pass (under the EXCLUSIVE + // table lock), tagLeakInstances()'s tagging state machine (deliberately + // run OUTSIDE the table lock - see its phase comment), and read by + // getLeakTagInfo() from ReferenceChainTracker's coverage path. A dedicated + // tiny spinlock keeps those contexts independent of _table_lock ordering + // (taking _table_lock inside/outside inconsistently would risk lock-order + // inversion); the critical sections are a few integer ops, no JNI. + SpinLock _leak_tag_pool_lock; int _leak_tag_free_list[LEAK_TAG_POOL_SIZE]; int _leak_tag_free_count; // Side table: for each tag in the pool, the (call_trace_id, tid) of @@ -600,7 +636,8 @@ class alignas(alignof(SpinLock)) LivenessTracker { // upcalls flush_table() already makes safely are just as safe there - see // that method's own comment for why a third caller needs both bypassing // the early-exit *and* resolution. - void cleanup_table(bool force = false, bool allow_resolve = true); + void cleanup_table(bool force = false, bool allow_resolve = true, + bool account_epoch = true); void flush_table(std::set *tracked_thread_ids); @@ -707,7 +744,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { // --- Slope computation and candidate ranking (selectLeakCandidates() below) --- - // Per-tid sustained-trend gate half #1: the same regression growth + // Per-tid sustained-trend gate half #1: the same mean-of-thirds growth // test hasQualifyingGrowth() below applies to a klass's ring, at // KlassPopulationEntry::TidTrend granularity (TID_TREND_MIN_FILL_FOR_TREND // samples of that smaller ring, same LEAK_GROWTH_REL_MIN/ABS_MIN growth @@ -752,13 +789,12 @@ class alignas(alignof(SpinLock)) LivenessTracker { // The sustained-trend gate (this class's own header comment above, // "Sustained-trend gate") - both-required growth-magnitude and floor-rise - // tests, design doc's original "mean of thirds" choice since replaced by - // full-window least-squares regression (see ringThirdsStats, - // livenessTracker.cpp - cheap, allocation-free, one pass over the + // tests, design doc's explicit "mean of thirds" choice over full + // least-squares regression (cheap, allocation-free, one pass over the // ring, no sorting or extra storage). A single scan - // (ringThirdsStats(), livenessTracker.cpp) both derives the pass/fail - // result below AND updates entry.cached_slope (regression end value minus - // start value) for selectLeakCandidates()'s ranking, rather than + // (ringWindowStats(), livenessTracker.cpp) both derives the pass/fail + // result below AND updates entry.cached_slope (recent third's mean minus + // earliest third's mean) for selectLeakCandidates()'s ranking, rather than // that method re-scanning the same unchanged ring a moment later. Returns // false (leaving entry.cached_slope untouched) if entry.ring_fill is below // KLASS_POPULATION_MIN_FILL_FOR_TREND - not enough history yet to trust a @@ -921,9 +957,11 @@ class alignas(alignof(SpinLock)) LivenessTracker { // Look up the (call_trace_id, tid) recorded for a leak tag. Returns // false if the tag is not a valid leak tag or has been returned to the - // pool. Used by ReferenceChainTracker for coverage tracking. + // pool. Used by ReferenceChainTracker for coverage tracking. Takes the + // table lock in shared mode: the slot fields are written under the + // exclusive lock from several distinct threads (GC reaper paths). bool getLeakTagInfo(jlong tag, u64 *out_call_trace_id, - jint *out_tid) const; + jint *out_tid); // Reads _klass_population and writes up to `max` STABLE CLASS TAGS // (KlassPopulationEntry::stable_class_tag - NOT the classMap dictionary @@ -986,7 +1024,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { // head/fill index, shared _heap_floor_time_ring timestamps - see // _container_mem_ring's own comment) so this compares _max_heap_bytes // against _container_memory_limit up front (the latter treated as - // unbounded when unavailable) and runs the regression-based rate + // unbounded when unavailable) and runs the "mean of thirds" rate // extrapolation (allocation-free, one ring scan) only once, against // whichever limit is smaller - not once per boundary. This matters // because container memory can grow from causes the heap-floor ring never @@ -1107,9 +1145,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { return __atomic_load_n(&_watched_tid_count, __ATOMIC_ACQUIRE); } - jint watchedTidForTest(int i) const { - return __atomic_load_n(&_watched_tids[i], __ATOMIC_RELAXED); - } + jint watchedTidForTest(int i) const { return _watched_tids[i]; } static jlong leakTagBaseForTest() { return LEAK_TAG_BASE; } @@ -1346,7 +1382,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { // reuses recordKlassPopulationSampleLocked()'s own creation branch // exactly; the seeded rising ramp that follows still clears // hasQualifyingGrowth() (a single 0 at the ring's start only lowers the - // regression start value, which RAISES the slope). + // earliest-third mean, which RAISES the slope). int slot = -1; for (int i = 0; i < _klass_population_size; i++) { if (_klass_population[i].klass_id == real_id) { @@ -1445,7 +1481,7 @@ class alignas(alignof(SpinLock)) LivenessTracker { void klassPopulationResetForTest() { _table_lock.lock(); _klass_population_size = 0; - _klass_count_scratch_size = 0; + klassCountScratchReset(); _test_klass_alias_count = 0; _table_lock.unlock(); // Also reset the heap-floor ring: it is a sibling piece of the same diff --git a/ddprof-lib/src/main/cpp/objectSampler.cpp b/ddprof-lib/src/main/cpp/objectSampler.cpp index 4232581200..3a11cc2fb5 100644 --- a/ddprof-lib/src/main/cpp/objectSampler.cpp +++ b/ddprof-lib/src/main/cpp/objectSampler.cpp @@ -182,13 +182,14 @@ Error ObjectSampler::start(Arguments &args) { return error; } if (_interval > 0) { - if (_record_liveness || _gc_generations) { - error = LivenessTracker::instance()->start(args); - if (error) { - return error; - } - } - + // Always call through, even when this start's own args request neither + // liveness recording nor gc generations: LivenessTracker::start() -> + // initialize() refreshes its own _gc_generations/_enabled from args + // unconditionally (see that method's own comment) and is a no-op beyond + // that when disabled. Gating this call on ObjectSampler's own + // (freshly-set, correct) flags left LivenessTracker's flags stuck at + // whatever the previous recording in this process last set them to, + // since it never got a chance to observe this recording's request at all. jvmtiEnv *jvmti = VM::jvmti(); // JVMTI Object Sampler is a 'solo' feature, meaning that it can only be // used by one JVMTI environment. Therefore, we can rely on the fact that if @@ -198,9 +199,18 @@ Error ObjectSampler::start(Arguments &args) { JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, NULL); __atomic_store_n(&_active, true, __ATOMIC_RELEASE); __atomic_store_n(&_last_config_update_ts, OS::nanotime(), __ATOMIC_RELEASE); + // Started LAST, after every step that can fail above: the old order + // (tracker first) left LivenessTracker started-but-never-driven if the + // JVMTI enabling failed - its GC-callback machinery and table would run + // with no sampler feeding it until the next stop(). stop() still stops + // it unconditionally (LivenessTracker::stop() self-guards on _enabled). // need to reset the running sum in order for 'updateConfiguration' to be // able to generate proper diffs _alloc_event_count = 0; + error = LivenessTracker::instance()->start(args); + if (error) { + return error; + } } return Error::OK; @@ -212,9 +222,9 @@ void ObjectSampler::stop() { jvmti->SetEventNotificationMode(JVMTI_DISABLE, JVMTI_EVENT_SAMPLED_OBJECT_ALLOC, NULL); - if (_record_liveness || _gc_generations) { - LivenessTracker::instance()->stop(); - } + // See start()'s own comment on why this call is unconditional - + // LivenessTracker::stop() already self-guards on its own _enabled. + LivenessTracker::instance()->stop(); } Error ObjectSampler::updateConfiguration(u64 events, double time_coefficient) { diff --git a/ddprof-lib/src/main/cpp/os_linux.cpp b/ddprof-lib/src/main/cpp/os_linux.cpp index daabc8f5b7..ba4cc28505 100644 --- a/ddprof-lib/src/main/cpp/os_linux.cpp +++ b/ddprof-lib/src/main/cpp/os_linux.cpp @@ -35,6 +35,7 @@ #include "guards.h" #include "log.h" #include "os.h" +#include "spinLock.h" #ifndef __musl__ #include @@ -916,8 +917,19 @@ int OS::getCgroupCpuMillicores() { // cgroups whose usage the leaf's memory.current excludes - pairing the // ancestor limit with leaf usage would overstate the available memory and // delay the OOM projection. +// +// Both globals below are written by getContainerMemoryLimit() (called from +// LivenessTracker::start() on the profiler-start thread and from +// SanityCheckTest's setup) and read by getContainerMemoryUsage() (from the +// JVMTI GarbageCollectionFinish callback, on whatever thread triggered +// GC) - genuinely concurrent threads, so every access is serialized by +// g_container_mem_lock. Without it the reader could observe a torn, +// half-overwritten path string. The usage-leaf cache additionally keeps +// the per-GC cost at one open/read/close: getContainerMemoryUsage() reads +// the remembered leaf directly instead of re-deriving it via +// getOwnCgroupPath() (/proc/self/cgroup parsing) on every GC. +static SpinLock g_container_mem_lock; static char g_memory_limit_cgroup_path[PATH_MAX] = {0}; - // Which cgroup hierarchy (v2 or v1) supplied the winning limit recorded in // g_memory_limit_cgroup_path: the usage file must be read from the SAME // hierarchy (v2 memory.current vs v1 memory.usage_in_bytes), not probed by @@ -925,6 +937,10 @@ static char g_memory_limit_cgroup_path[PATH_MAX] = {0}; // for the same cgroup dir, and reading the wrong one pairs the limit with an // unrelated usage number. static bool g_memory_limit_cgroup_v2 = true; +// The leaf whose memory.current/memory.usage_in_bytes pairs with the +// winning limit (the winner cgroup itself, or - when unconstrained - the +// process's own leaf). Empty until the first successful limit walk. +static char g_usage_leaf_path[PATH_MAX] = {0}; static long walkCgroupV2MemoryLimit(char* path, char* winner_path_out) { size_t base_len = strlen("/sys/fs/cgroup"); @@ -991,11 +1007,17 @@ static long walkCgroupV1MemoryLimit(char* path, char* winner_path_out) { long OS::getContainerMemoryLimit() { char subpath[PATH_MAX]; char path[PATH_MAX]; + // Local winner/usage-leaf bookkeeping, published under the lock at the + // end - never write the shared globals directly (the usage reader runs + // concurrently on GC-callback threads). + char winner[PATH_MAX] = {0}; + char usage_leaf[PATH_MAX] = {0}; + bool v2 = true; + long result; // Recomputed on every call; getContainerMemoryUsage() pairs its usage // read with whatever path won here (see the winner-path comment on // walkCgroupV2MemoryLimit()). - g_memory_limit_cgroup_path[0] = '\0'; // Try cgroup v2 first, resolved from this process's own cgroup path. if (getOwnCgroupPath("", subpath, sizeof(subpath))) { @@ -1010,8 +1032,13 @@ long OS::getContainerMemoryLimit() { int fd = open(leaf, O_RDONLY); if (fd != -1) { close(fd); - g_memory_limit_cgroup_v2 = true; - return walkCgroupV2MemoryLimit(path, g_memory_limit_cgroup_path); + result = walkCgroupV2MemoryLimit(path, winner); + // The usage read pairs with the winner cgroup when the + // walk found a limit; otherwise the process's own leaf. + snprintf(usage_leaf, sizeof(usage_leaf), "%s", + winner[0] != '\0' ? winner : path); + v2 = true; + goto publish; } } } @@ -1031,14 +1058,29 @@ long OS::getContainerMemoryLimit() { int fd = open(leaf, O_RDONLY); if (fd != -1) { close(fd); - g_memory_limit_cgroup_v2 = false; - return walkCgroupV1MemoryLimit(path, g_memory_limit_cgroup_path); + result = walkCgroupV1MemoryLimit(path, winner); + snprintf(usage_leaf, sizeof(usage_leaf), "%s", + winner[0] != '\0' ? winner : path); + v2 = false; + goto publish; } } } } - return -1; + // Unconstrained or unavailable - remember that (empty paths) so the + // usage reader skips straight to its own leaf re-derivation. + result = -1; + winner[0] = '\0'; + usage_leaf[0] = '\0'; + +publish: + g_container_mem_lock.lock(); + memcpy(g_memory_limit_cgroup_path, winner, sizeof(winner)); + memcpy(g_usage_leaf_path, usage_leaf, sizeof(usage_leaf)); + g_memory_limit_cgroup_v2 = v2; + g_container_mem_lock.unlock(); + return result; } // Reads the current usage from the same cgroup level that supplied @@ -1053,26 +1095,51 @@ long OS::getContainerMemoryUsage() { char subpath[PATH_MAX]; char path[PATH_MAX]; + // Fast path: the leaf remembered by the last limit walk (winner cgroup, + // or the process's own leaf when unconstrained). Snapshot under the + // lock - the walk may be republishing concurrently, and an unsynchronized + // read could observe a torn half-written path. This keeps the per-GC + // cost at one open/read/close instead of re-deriving the cgroup path + // via getOwnCgroupPath() (/proc/self/cgroup parsing) on every GC. + char leaf[PATH_MAX] = {0}; + g_container_mem_lock.lock(); + if (g_usage_leaf_path[0] != '\0') { + memcpy(leaf, g_usage_leaf_path, sizeof(leaf)); + } else if (g_memory_limit_cgroup_path[0] != '\0') { + memcpy(leaf, g_memory_limit_cgroup_path, sizeof(leaf)); + } + g_container_mem_lock.unlock(); + // Same cgroup the winning limit came from, if the limit walk recorded // one - read its usage first, falling back to the process's own leaf. // The usage filename follows the hierarchy that supplied the limit (the - // recorded winner's hierarchy is authoritative, not filename order). - if (g_memory_limit_cgroup_path[0] != '\0') { + // recorded winner's hierarchy is authoritative, not filename order - on + // a hybrid system both controller files can be visible for the same + // cgroup dir); the other hierarchy's file is only a fallback for a leaf + // remembered before the hierarchy flag existed. + if (leaf[0] != '\0') { char file[PATH_MAX]; - const char *fmt = g_memory_limit_cgroup_v2 ? "%s/memory.current" - : "%s/memory.usage_in_bytes"; - if ((size_t)snprintf(file, sizeof(file), fmt, - g_memory_limit_cgroup_path) < sizeof(file)) { + g_container_mem_lock.lock(); + bool limit_was_v2 = g_memory_limit_cgroup_v2; + g_container_mem_lock.unlock(); + const char *fmts[2] = { + limit_was_v2 ? "%s/memory.current" : "%s/memory.usage_in_bytes", + limit_was_v2 ? "%s/memory.usage_in_bytes" : "%s/memory.current"}; + for (const char *fmt : fmts) { + if ((size_t)snprintf(file, sizeof(file), fmt, leaf) >= sizeof(file)) { + continue; + } int fd = open(file, O_RDONLY); - if (fd != -1) { - char buf[32] = {0}; - ssize_t r = read(fd, buf, sizeof(buf) - 1); - close(fd); - if (r > 0) { - long usage = atol(buf); - if (usage >= 0) { - return usage; - } + if (fd == -1) { + continue; + } + char buf[32] = {0}; + ssize_t r = read(fd, buf, sizeof(buf) - 1); + close(fd); + if (r > 0) { + long usage = atol(buf); + if (usage >= 0) { + return usage; } } } diff --git a/ddprof-lib/src/main/cpp/os_macos.cpp b/ddprof-lib/src/main/cpp/os_macos.cpp index 3d3aa8ef6b..575acd805b 100644 --- a/ddprof-lib/src/main/cpp/os_macos.cpp +++ b/ddprof-lib/src/main/cpp/os_macos.cpp @@ -385,7 +385,12 @@ long OS::getContainerMemoryLimit() { } long OS::getContainerMemoryUsage() { - return -1; // macOS has no cgroup support. + // Contract mirror of the Linux implementation: "no boundary available" + // is -1 (treated by LivenessTracker::secondsToOOM() as no projection, + // never as unbounded), not 0 - a 0 here would read as a container at + // zero usage and skew the container-boundary ring's slope toward + // noise. macOS has no cgroup support. + return -1; } u64 OS::getProcessCpuTime(u64* utime, u64* stime) { diff --git a/ddprof-lib/src/main/cpp/profiler.cpp b/ddprof-lib/src/main/cpp/profiler.cpp index 12c8ea3b1c..7ac9504976 100644 --- a/ddprof-lib/src/main/cpp/profiler.cpp +++ b/ddprof-lib/src/main/cpp/profiler.cpp @@ -100,6 +100,14 @@ void Profiler::onThreadStart(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) { updateThreadName(jvmti, jni, thread, true); } + // Registers the tid -> Thread-object global ref that the reference-chain + // engine's walkCandidateThreadLocals() descends from for + // candidate-scoped ThreadLocalMap reach. No-op while reference chains are + // disabled (checked inside the tracker); jni/thread may be null on the + // internal pre-existing-threads call from start(), which the tracker + // also refuses. + ReferenceChainTracker::instance()->registerThreadObject(jni, tid, thread); + _cpu_engine->registerThread(tid); _wall_engine->registerThread(tid); } @@ -113,6 +121,11 @@ void Profiler::onThreadEnd(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) { // ProfiledThread is alive - do full cleanup and use efficient tid access int slot_id = current->filterSlotId(); tid = current->tid(); + // NOT gated on reference-chains enabled: a thread registered while a + // recording ran must release its global ref when it ends, even if the + // recording has since stopped (see unregisterThreadObject()'s comment, + // referenceChains.h). + ReferenceChainTracker::instance()->unregisterThreadObject(jni, tid); if (_thread_filter.enabled()) { _thread_filter.unregisterThread(slot_id); @@ -140,6 +153,11 @@ void Profiler::onThreadEnd(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread) { return; } + // Same rationale as the ProfiledThread-alive branch above: a thread + // registered during an active recording must release its global ref when + // it ends, whatever path its teardown takes. + ReferenceChainTracker::instance()->unregisterThreadObject(jni, tid); + updateThreadName(jvmti, jni, thread, false); _cpu_engine->unregisterThread(tid); _wall_engine->unregisterThread(tid); @@ -877,6 +895,117 @@ void Profiler::writeHeapUsage(long value, bool live) { _locks[lock_index].unlock(); } +void Profiler::writeReferenceChainAbandoned(ReferenceChainAbandonedEvent *event) { + int tid = ProfiledThread::currentTid(); + if (tid < 0) { + return; + } + // Same bounded-retry pattern as writeReferenceChain() below: the caller + // (Profiler::dump()'s drain loop, referenceChains.cpp + // drainPendingAbandonedEvents()) has already removed the event from the + // pending queue, so a bare non-blocking 3-slot sweep that misses all + // three locks would lose it permanently - unlike resolved-chain events + // (which snapshot-and-keep), an abandoned event has no second chance. + u32 lock_index; + bool locked = false; + int sweeps = 0; + u64 start_ns = OS::nanotime(); + // Per-event budget: abandoned events are rare (one per abandoned search) + // and carry no batch deadline from the caller. + const u64 kAbandonedWriteBudgetNs = 50 * 1000000ULL; + u64 deadline_ns = OS::nanotime() + kAbandonedWriteBudgetNs; + for (;;) { + sweeps++; + lock_index = getLockIndex(tid); + if (_locks[lock_index].tryLock() || + _locks[lock_index = (lock_index + 1) % CONCURRENCY_LEVEL].tryLock() || + _locks[lock_index = (lock_index + 2) % CONCURRENCY_LEVEL].tryLock()) { + locked = true; + break; + } + if (OS::nanotime() >= deadline_ns) { + break; + } + usleep(1000); + } + if (!locked) { + Counters::increment(REFERENCE_CHAIN_EVENTS_DROPPED); + TEST_LOG("Profiler::writeReferenceChainAbandoned drop: lock contention " + "exhausted budget after sweeps=%d waited_us=%llu", + sweeps, (unsigned long long)((OS::nanotime() - start_ns) / 1000)); + return; + } + _jfr.recordReferenceChainAbandoned(lock_index, event); + _locks[lock_index].unlock(); +} + +// Unlike writeReferenceChainAbandoned() above (mirroring CPU/wall's signal-handler-safe +// non-blocking pattern out of caution, even though its own call site - Profiler::dump(), +// profiler.cpp - isn't a signal handler either), this call site genuinely cannot be one: +// this is called from Profiler::dump()'s drain loop, on dump()'s own calling thread, once +// per event snapshotted from ReferenceChainTracker::_resolved_chains (up to +// MAX_RESOLVED_CHAINS per dump) - never from pollWatchedTargets() or any other call on +// ReferenceChainTracker's own BFS agent thread, and never from a signal handler. A single +// bare 3-slot tryLock() sweep with no wait - correct for a signal handler, which must never +// block - was found, by running the end-to-end integration test for real, +// to drop this event under perfectly ordinary contention: the same _locks[] pool is shared +// with every other sample type (recordJVMTISample() et al.), and any nontrivial allocation +// throughput keeps enough of CONCURRENCY_LEVEL's slots busy that 3 immediate, back-to-back +// attempts routinely all miss. A bounded retry with a short sleep between sweeps costs +// nothing the dump()-thread cannot afford, but the retry budget below is a single deadline +// shared across the *entire* drain batch (see the caller in dump()) rather than per event: +// with up to MAX_RESOLVED_CHAINS events snapshotted, a fresh per-event budget could stall +// the dump/JFR-flush thread for seconds under contention. Once the shared deadline has +// passed this degrades to the same single non-blocking 3-slot sweep as +// writeReferenceChainAbandoned() above for the remainder of the batch. +void Profiler::writeReferenceChain(ReferenceChainEvent *event, u64 deadline_ns) { + int tid = ProfiledThread::currentTid(); + if (tid < 0) { + TEST_LOG("Profiler::writeReferenceChain drop: currentTid() < 0"); + return; + } + u32 lock_index; + bool locked = false; + int sweeps = 0; + u64 start_ns = OS::nanotime(); + for (;;) { + sweeps++; + lock_index = getLockIndex(tid); + if (_locks[lock_index].tryLock() || + _locks[lock_index = (lock_index + 1) % CONCURRENCY_LEVEL].tryLock() || + _locks[lock_index = (lock_index + 2) % CONCURRENCY_LEVEL].tryLock()) { + locked = true; + break; + } + if (OS::nanotime() >= deadline_ns) { + // Shared batch budget exhausted - the sweep just above was already a + // single non-blocking attempt, so stop retrying rather than sleeping + // again. + break; + } + usleep(1000); + } + if (!locked) { + // Unlike the drain-once era, this drop is NOT permanent: the event was + // only copied out of ReferenceChainTracker::_resolved_chains + // (drainPendingChainEvents() snapshots without clearing), so as long as + // the sample stays live the next dump re-emits it and gets another chance + // at the lock. Still counted like every other counted-drop path + // (REFERENCE_CHAIN_WRITE_DROPPED's own comment) rather than dropping it + // silently. + Counters::increment(REFERENCE_CHAIN_WRITE_DROPPED); + TEST_LOG("Profiler::writeReferenceChain drop: lock contention exhausted shared " + "deadline after sweeps=%d waited_us=%llu", + sweeps, (unsigned long long)((OS::nanotime() - start_ns) / 1000)); + return; + } + TEST_LOG("Profiler::writeReferenceChain locked lock_index=%u after sweeps=%d " + "waited_us=%llu", + lock_index, sweeps, (unsigned long long)((OS::nanotime() - start_ns) / 1000)); + _jfr.recordReferenceChain(lock_index, event); + _locks[lock_index].unlock(); +} + bool Profiler::prewarmUnwinder() { #ifdef __linux__ // Force libgcc_s.so.1 to load now and report whether that succeeded. This @@ -1711,26 +1840,6 @@ Error Profiler::start(Arguments &args, bool reset) { } } } - if ((activated & EM_ALLOC) && args._reference_chains) { - // Reference-chain tracking chases LivenessTracker's leak-tagged - // candidates, so it only runs when allocation sampling actually - // activated. Must run AFTER ObjectSampler::start() -> - // LivenessTracker::start() (ordering noted in referenceChains.cpp) - - // the block above is that ordering point. - error = ReferenceChainTracker::instance()->start(args); - if (error) { - Log::warn("%s", error.message()); - error = Error::OK; // recoverable - recording continues without chains - } else { - ReferenceChainTracker::instance()->startThread(); - // Pre-existing threads must be registered from the profiler lifecycle - // (registerExistingThreads()'s own comment): Profiler::onThreadStart() - // only sees threads started after the recording began. - ReferenceChainTracker::instance()->registerExistingThreads(VM::jvmti(), - VM::jni()); - _reference_chains_active = true; - } - } if (_event_mask & EM_NATIVEMEM) { error = malloc_tracer.start(args); if (error) { @@ -1778,6 +1887,42 @@ Error Profiler::start(Arguments &args, bool reset) { // Paired with drainInflight() on the stop side. _cpu_engine->enableEvents(true); + // Independent of the CPU/wall/alloc engine mask above (GC-triggered, not + // sample-triggered) - same pattern as malloc_tracer/NativeSocketSampler + // being gated on their own flags rather than folded into `activated`. + // Placed after the engines are confirmed running (inside this + // `if (activated)` block) so there is nothing to unwind here if it + // fails - see this method's failure path below, which never reaches + // this point. + // Called unconditionally, not gated on args._reference_chains: start() + // is the only place that refreshes ReferenceChainTracker::_enabled + // (stop() deliberately leaves it unchanged - see that method's own + // comment), so a previous recording's `referencechains=true` session + // must still reach start() when this one opts out, or the tracker keeps + // reporting enabled()==true - and Profiler::dump()'s reference-chains + // gate keeps emitting that stale session's cached chains/abandonment + // state - for the entire duration of this new, opted-out recording. + // start() itself sets `_enabled = args._reference_chains` up front and + // returns early when that is false, so this call is a cheap no-op for + // an opted-out recording. + error = ReferenceChainTracker::instance()->start(args); + if (error) { + Log::warn("%s", error.message()); + error = Error::OK; // recoverable + } else if (args._reference_chains) { + // Only safe once the JVM/JVMTI environment is fully up, which is + // guaranteed at this point in Profiler::start() - see + // ReferenceChainTracker::start()'s own comment (referenceChains.cpp) + // for why this is not called from inside start() itself. + ReferenceChainTracker::instance()->startThread(); + // Pre-existing threads (alive since before this recording began) + // never fired onThreadStart() - same lifecycle rationale as + // startThread() above for why this runs here rather than inside + // ReferenceChainTracker::start(). + ReferenceChainTracker::instance()->registerExistingThreads( + VM::jvmti(), VM::jni()); + } + _state.store(RUNNING, std::memory_order_release); _start_time = time(NULL); __atomic_add_fetch(&_epoch, 1, __ATOMIC_RELAXED); @@ -1820,17 +1965,48 @@ Error Profiler::stop() { if (_event_mask & EM_ALLOC) _alloc_engine->stop(); - if (_reference_chains_active) { - // Join the BFS thread and clear the recording-boundary state before the - // rest of the teardown (stopThread() wakes and joins; stop() resets the - // per-recording caches). Matches the Profiler::stop() order documented - // in referenceChains.cpp's stopThread()/stop() comments. + if (_event_mask & EM_NATIVEMEM) + malloc_tracer.stop(); + // Not part of _event_mask (see the matching start() block above) - gated + // on enabled() instead, which start() set from args._reference_chains for + // this session. + if (ReferenceChainTracker::instance()->enabled()) { ReferenceChainTracker::instance()->stopThread(); ReferenceChainTracker::instance()->stop(); - _reference_chains_active = false; + // Drains the global refs of threads that ended during this recording + // (referenceChains.h, _thread_refs_pending_delete). Safe here: the BFS + // thread was joined by stopThread() above, so no walk phase can still + // hold a copied Thread-object ref. + ReferenceChainTracker::instance()->releaseEndedThreadRefs(VM::jni()); + // Final drain: only dump() writes the tracker's resolved-chain cache and + // abandoned-event queue, so a recording that ends without a preceding + // dump() would lose every reference-chain result discovered since the + // last dump. Both writes go through the same JFR write paths dump() + // uses, before the final chunk is finalized by _jfr.stop() below. The + // tracker is fully stopped here, so the caches are stable snapshots. + std::vector pending_abandoned_events; + ReferenceChainTracker::instance()->drainPendingAbandonedEvents( + &pending_abandoned_events); + for (auto &rc_event : pending_abandoned_events) { + writeReferenceChainAbandoned(&rc_event); + } + std::vector pending_chain_events; + ReferenceChainTracker::instance()->drainPendingChainEvents( + &pending_chain_events); + const u64 kChainDrainBudgetNs = 50 * 1000000ULL; + u64 chain_drain_deadline_ns = OS::nanotime() + kChainDrainBudgetNs; + for (auto &rc_event : pending_chain_events) { + writeReferenceChain(&rc_event, chain_drain_deadline_ns); + } + // Threads still alive at stop keep registered global refs that nothing + // else will release: the tracker's BFS thread is gone (no walks need + // them) and threads that end AFTER this point take the fallback + // onThreadEnd path, whose unregister can no longer be relied on for + // entries a future recording did not re-register. Delete every + // remaining registry entry so ended threads cannot stay reachable + // anchors for the rest of the JVM's life. + ReferenceChainTracker::instance()->releaseAllThreadObjects(VM::jni()); } - if (_event_mask & EM_NATIVEMEM) - malloc_tracer.stop(); // Stop the refresher BEFORE socket unpatch: the refresher calls // install_socket_hooks() which re-reads _socket_active before acquiring the // patch lock. If the refresher runs concurrently with unpatch_socket_functions() @@ -1991,29 +2167,60 @@ Error Profiler::dump(const char *path, const int length) { // by the live objects LivenessTracker::instance()->flush(thread_ids); - // Emit the reference-chain tracker's pending events into this dumping - // chunk: chain events are snapshot-and-kept (re-emitted into every chunk - // while the sample stays live), abandonment events are a true drain. - // Runs before rotateDictsAndRun() so the events land inside the chunk - // being written, and under a profiler lock like every other - // recording-buffer writer (dump runs on a normal thread holding only - // _state_lock; the _state_lock -> _locks order is the codebase's). - { - int dump_tid = ProfiledThread::currentTid(); - u32 lock_index = getLockIndex(dump_tid >= 0 ? dump_tid : 0); - _locks[lock_index].lock(); - std::vector chain_events; - ReferenceChainTracker::instance()->drainPendingChainEvents(&chain_events); - for (auto &event : chain_events) { - _jfr.recordReferenceChain(lock_index, &event); - } - std::vector abandoned_events; + // ReferenceChainTracker::_resolved_chains (and the search-state fields + // read below) are intentionally left populated across a stop()/start() + // cycle - see _resolved_chains' own comment (referenceChains.h) - but + // that means they can still hold state from a *previous* recording that + // had referencechains enabled, even once the current recording started + // with referencechains=false (in which case ReferenceChainTracker:: + // start() sets _enabled=false and no BFS thread is polling to ever + // refresh or prune them). Gate both emissions on the current session's + // flag so an opted-out recording does not keep re-reporting a dead + // session's abandoned search or stale resolved chains. + if (ReferenceChainTracker::instance()->enabled()) { + // ReferenceChainTracker's BFS thread restarts an ABANDONED search on + // its own ~1s cadence (referenceChains.cpp shouldRunPass() -> + // restartSearch()), which clears the very state + // buildAbandonedEvent() needs. A live re-read of searchState() here + // would almost always miss that ~1s window against dump()'s much + // slower JFR-chunk-rotation cadence. Instead each abandon is + // snapshotted into a queue at the moment it happens + // (enqueuePendingAbandonedEvent(), called from runPass()) and drained + // here - a true drain, unlike drainPendingChainEvents() below, since + // an abandon is a one-off past occurrence rather than an ongoing live + // sample. + std::vector pending_abandoned_events; ReferenceChainTracker::instance()->drainPendingAbandonedEvents( - &abandoned_events); - for (auto &event : abandoned_events) { - _jfr.recordReferenceChainAbandoned(lock_index, &event); + &pending_abandoned_events); + for (auto &rc_event : pending_abandoned_events) { + // No re-stamp here: the event's _start_time was set when the search + // actually stopped (enqueuePendingAbandonedEvent()) - re-stamping at + // dump time would misreport a seconds-old abandon as happening now. + writeReferenceChainAbandoned(&rc_event); } - _locks[lock_index].unlock(); + + // Re-emit every currently-cached datadog.ReferenceChain pollWatchedTargets() + // (referenceChains.cpp) has resolved - snapshotted here, on this call's + // own thread, rather than written eagerly from the BFS scheduling thread + // that discovered them (see ReferenceChainTracker::_resolved_chains' own + // comment for why the cache re-emits on every dump rather than draining). + std::vector pending_chain_events; + ReferenceChainTracker::instance()->drainPendingChainEvents( + &pending_chain_events); + // One ~50ms retry budget for the *whole* batch, not per event - + // writeReferenceChain()'s own comment for why: up to + // MAX_RESOLVED_CHAINS events can be snapshotted, and a fresh per-event + // budget would let this dump()-thread stall for seconds under ordinary + // _locks[] contention. + const u64 kChainDrainBudgetNs = 50 * 1000000ULL; + u64 chain_drain_deadline_ns = OS::nanotime() + kChainDrainBudgetNs; + long long write_dropped_before = Counters::getCounter(REFERENCE_CHAIN_WRITE_DROPPED); + for (auto &rc_event : pending_chain_events) { + writeReferenceChain(&rc_event, chain_drain_deadline_ns); + } + TEST_LOG("Profiler::dump reference-chain batch=%d write_dropped=%lld", + (int)pending_chain_events.size(), + Counters::getCounter(REFERENCE_CHAIN_WRITE_DROPPED) - write_dropped_before); } Libraries::instance()->refresh(); @@ -2031,6 +2238,15 @@ Error Profiler::dump(const char *path, const int length) { err = _jfr.dump(path, length); __atomic_add_fetch(&_epoch, 1, __ATOMIC_SEQ_CST); }); + if (err) { + // Log::debug, not just TEST_LOG: TEST_LOG is compiled out of release + // builds entirely, so a JFR dump failure - the event stream this + // whole subsystem exists to produce - would fail silently in + // production. debug-level logging keeps it out of the steady-state + // log while still being reachable when someone turns debug on. + Log::debug("Profiler::dump _jfr.dump failed: %s", err.message()); + TEST_LOG("Profiler::dump _jfr.dump failed: %s", err.message()); + } _thread_info.clearAll(thread_ids); _thread_info.reportCounters(); diff --git a/ddprof-lib/src/main/cpp/profiler.h b/ddprof-lib/src/main/cpp/profiler.h index a2f3f56d97..ec9f1c79d3 100644 --- a/ddprof-lib/src/main/cpp/profiler.h +++ b/ddprof-lib/src/main/cpp/profiler.h @@ -195,7 +195,9 @@ class alignas(alignof(SpinLock)) Profiler { // // rotate() is self-contained: it uses _accepting + RefCountGuard to drain // concurrent JNI readers, and SignalBlocker prevents profiling signals on - // this thread from inserting into old_active between Phase 1 and Phase 2. + // this thread from inserting into old_active between the pre-populate copy + // step and the catch-up copy step of the dictionary's two-step rotation + // (see StringDictionary::rotate(), stringDictionary.h). // No external lock is required for rotation. // // lockAll() wraps jfr_op only — to gate call-trace writers (signal handlers @@ -470,6 +472,20 @@ class alignas(alignof(SpinLock)) Profiler { void writeDatadogProfilerSetting(int tid, int length, const char *name, const char *value, const char *unit); void writeHeapUsage(long value, bool live); + // Mirrors writeHeapUsage()'s shape exactly. Called from dump() whenever + // ReferenceChainTracker's search has ended in SearchState::ABANDONED, + // the same way LivenessTracker::flush() is called from dump(). + void writeReferenceChainAbandoned(ReferenceChainAbandonedEvent *event); + // Unlike writeReferenceChainAbandoned() above, this is NOT a bare 3-slot + // tryLock() sweep - it retries with a bounded, sleeping loop because its + // call site is dump()'s drain loop (profiler.cpp), on dump()'s own calling + // thread, which can tolerate blocking, unlike a signal handler; see this + // method's own comment in profiler.cpp for why that retry exists. + // `deadline_ns` is a single retry budget shared across dump()'s *entire* + // drain batch (not reset per event) - see the caller in dump() and this + // method's own comment in profiler.cpp for why a per-event budget would be + // unbounded across a large batch. + void writeReferenceChain(ReferenceChainEvent *event, u64 deadline_ns); int eventMask() const { return _event_mask; } bool isRemoteSymbolication() const { return _remote_symbolication; } bool sanityCheckFailed() const { return _sanity_check_failed; } diff --git a/ddprof-lib/src/main/cpp/referenceChainEvents.cpp b/ddprof-lib/src/main/cpp/referenceChainEvents.cpp index 648cc360bd..23b42e4e76 100644 --- a/ddprof-lib/src/main/cpp/referenceChainEvents.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainEvents.cpp @@ -257,6 +257,11 @@ void ReferenceChainTracker::pollWatchedTargets(jvmtiEnv *jvmti, JNIEnv *jni) { return; } + // Best-effort drain of any deferred resolved-chain invalidations that a pass ending between + // polls did not already apply (see runPassManualWalk()'s end-of-pass drain): chains rebuilt + // this poll must not be gated by a stale cache entry the callback asked to drop. + drainPendingChainInvalidations(); + // Stamp every entry this poll refreshes with the current search generation. const u64 current_search_ns = load(_search_start_ns); @@ -586,6 +591,24 @@ void ReferenceChainTracker::invalidateResolvedChain(jlong source_tag) { _resolved_chains_lock.unlock(); } +void ReferenceChainTracker::deferResolvedChainInvalidation(jlong source_tag) { + _pending_chain_invalidations_lock.lock(); + _pending_chain_invalidations.push_back(source_tag); + _pending_chain_invalidations_lock.unlock(); +} + +void ReferenceChainTracker::drainPendingChainInvalidations() { + std::vector pending; + { + _pending_chain_invalidations_lock.lock(); + pending.swap(_pending_chain_invalidations); + _pending_chain_invalidations_lock.unlock(); + } + for (size_t i = 0; i < pending.size(); i++) { + invalidateResolvedChain(pending[i]); + } +} + // Builds and caches chain events for every auto-marked discovered instance recorded against a slot // holding klass_id (see the auto-mark block in heapReferenceCallback() for how instances get // recorded). diff --git a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp index 6c142029ab..796962dbf7 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -158,6 +158,11 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, // walk holds them. releaseEndedThreadRefs(jni); + // Same safe point for the heap callback's deferred resolved-chain invalidations: applying them + // here (outside any walk) keeps _resolved_chains_lock traffic off the FollowReferences pause + // entirely. + drainPendingChainInvalidations(); + *safepoint_ticks = 0; // Shared wall-clock ceiling for this whole call's static-field sweep, expandFrontier(), and @@ -430,6 +435,12 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, // last one that ran. *truncated = *truncated || rotation_truncated; *frontier_cap_hit = *frontier_cap_hit || rotation_frontier_cap_hit; + + // End-of-pass drain for the heap callback's deferred resolved-chain invalidations + // (improveChain/re-parent/root-upgrade evictions recorded during the walks): applying them here + // keeps _resolved_chains_lock traffic outside the FollowReferences pauses entirely, and ensures + // the next pollWatchedTargets() never rebuilds from a stale cached chain. + drainPendingChainInvalidations(); } // Incremental resumption across passes. diff --git a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp index a74ecfbc38..6cb2a4597e 100644 --- a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp @@ -420,7 +420,10 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( (u8)reference_kind)) { // Chain was improved — invalidate any cached chain for this tag so pollWatchedTargets // rebuilds it with the deeper path. - ctx->tracker->invalidateResolvedChain(*tag_ptr); + // Deferred (see deferResolvedChainInvalidation()): this runs inside the FollowReferences + // stop-the-world pause; taking _resolved_chains_lock here can block the walk behind + // drainPendingChainEvents()'s full-cache copy on the JFR dump thread. + ctx->tracker->deferResolvedChainInvalidation(*tag_ptr); if (was_root_attached_durable) { // Demotion push (B'): the replaced entry's static/JNI-global attribution was its only // anchor-tier eligibility, and it is gone now. @@ -433,7 +436,8 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( // Equal-depth re-parent from a transient root to a durable one (improveChain() cannot // express it - see its declaration) - same cache invalidation so the rebuilt chain uses the // durable root. - ctx->tracker->invalidateResolvedChain(*tag_ptr); + // Deferred: same STW-lock rationale as the improveChain branch. + ctx->tracker->deferResolvedChainInvalidation(*tag_ptr); } } else { // Already-admitted entry reached via a NEW root-like edge (parent_tag == 0): the static-field @@ -443,7 +447,8 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( if (ctx->tracker->maybeUpgradeRootAttachedRootKind(ctx->frontier, *tag_ptr, (u8)reference_kind)) { - ctx->tracker->invalidateResolvedChain(*tag_ptr); + // Deferred: same STW-lock rationale as the improveChain branch. + ctx->tracker->deferResolvedChainInvalidation(*tag_ptr); } else if (reference_kind == JVMTI_HEAP_REFERENCE_STATIC_FIELD) { // The upgrade refused (maybeUpgradeRootAttachedRootKind returns false for parent_tag != 0 // by design), so this STATIC_FIELD edge just proved an at-risk static attachment the anchor diff --git a/ddprof-lib/src/main/cpp/referenceChains.h b/ddprof-lib/src/main/cpp/referenceChains.h index 11640aa529..4641d1778d 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -383,6 +383,15 @@ class ReferenceChainTracker { } // Idempotent: returns false if `tag` is already indexed. + // Occupancy-bounded: a completely full table (2048/2048) would make + // the linear-probe loop wrap forever - a livelock on the engine + // thread inside rotation collection or rebuildFrom(). The external + // invariant (deque <= PRIORITY_EXPAND_CAP by construction) normally + // prevents this, but the invariant is enforced only at push sites; + // the occupancy check makes insert() self-terminating even if a + // future call site breaks it (insert returns false - the tag is + // treated as not-queued, the same degradation a full deque push + // already accepts). bool insert(jlong tag) { u64 i = mix(tag) >> (64 - SLOT_SHIFT); while (_used[i]) { @@ -390,6 +399,10 @@ class ReferenceChainTracker { return false; } i = (i + 1) & SLOT_MASK; + // Wrapped all 2048 slots without an empty one: table full. + if (i == (mix(tag) >> (64 - SLOT_SHIFT))) { + return false; + } } _used[i] = 1; _keys[i] = tag; @@ -567,6 +580,14 @@ class ReferenceChainTracker { std::unordered_map _resolved_chains; SpinLock _resolved_chains_lock; + // Resolved-chain evictions deferred by the FollowReferences heap callback + // (see deferResolvedChainInvalidation()). Small: each callback-side + // improveChain/re-parent/root-upgrade evicts at most one entry, and the + // pending set is drained outside any walk (runPassManualWalk()'s start and + // end, pollWatchedTargets()'s entry). + std::vector _pending_chain_invalidations; + SpinLock _pending_chain_invalidations_lock; + // Abandoned-search events awaiting Profiler::dump() (profiler.cpp). static constexpr int MAX_PENDING_ABANDONED_EVENTS = 16; std::vector _pending_abandoned_events; @@ -972,6 +993,20 @@ class ReferenceChainTracker { // Remove a cached chain so pollWatchedTargets rebuilds it on the next poll. void invalidateResolvedChain(jlong source_tag); + // Record a resolved-chain eviction from inside the FollowReferences heap + // callback WITHOUT taking _resolved_chains_lock: the callback runs inside + // the JVMTI FollowReferences stop-the-world pause, and the lock is also + // held across drainPendingChainEvents()'s full-cache copy on the JFR dump + // thread, so locking here can stall the walk behind the dump thread. + // drainPendingChainInvalidations() applies the recorded evictions on the + // BFS thread, outside any walk. + void deferResolvedChainInvalidation(jlong source_tag); + + // Apply the evictions deferResolvedChainInvalidation() recorded - called + // only from runPassManualWalk()'s start/end and pollWatchedTargets()'s + // entry, always outside any FollowReferences walk. + void drainPendingChainInvalidations(); + // Snapshots the just-abandoned search into _pending_abandoned_events - called from runPass() // (referenceChains.cpp) immediately after it writes SearchState::ABANDONED, while // buildAbandonedEvent()'s source fields are still valid (see _pending_abandoned_events' own diff --git a/ddprof-lib/src/main/cpp/vmEntry.cpp b/ddprof-lib/src/main/cpp/vmEntry.cpp index acf8382322..be3f5fa57e 100644 --- a/ddprof-lib/src/main/cpp/vmEntry.cpp +++ b/ddprof-lib/src/main/cpp/vmEntry.cpp @@ -395,6 +395,15 @@ bool VM::initLibrary(JavaVM *vm) { return true; } +// jvmtiEventCallbacks has a single function-pointer slot per event; both +// LivenessTracker and ReferenceChainTracker need GarbageCollectionFinish, +// so this trampoline dispatches to both instead of one +// subsystem's registration clobbering the other's. +static void JNICALL onGarbageCollectionFinish(jvmtiEnv *jvmti_env) { + LivenessTracker::GarbageCollectionFinish(jvmti_env); + ReferenceChainTracker::GarbageCollectionFinish(jvmti_env); +} + void VM::probeJFRRequestStackTrace() { jint ext_count = 0; jvmtiExtensionFunctionInfo *ext_functions = nullptr; @@ -445,17 +454,6 @@ bool VM::initializeRequestStackTrace() { return false; } -// JVMTI delivers ONE callback per event slot; both trackers need the -// GarbageCollectionFinish signal (liveness GC epochs + reference-chain pass -// scheduling), so this vmEntry-level forwarder fans the single slot out to -// both static callbacks. Order is irrelevant (each only does lock-free -// bookkeeping); LivenessTracker's callback keeps its own -// initCurrentThreadSignalSafe() behavior. -void JNICALL ForwardedGarbageCollectionFinish(jvmtiEnv *jvmti_env) { - LivenessTracker::GarbageCollectionFinish(jvmti_env); - ReferenceChainTracker::GarbageCollectionFinish(jvmti_env); -} - bool VM::initProfilerBridge(JavaVM *vm, bool attach) { TEST_LOG("VM::initProfilerBridge"); if (!initShared(vm)) { @@ -531,7 +529,7 @@ bool VM::initProfilerBridge(JavaVM *vm, bool attach) { callbacks.ThreadEnd = Profiler::ThreadEnd; callbacks.SampledObjectAlloc = ObjectSampler::SampledObjectAlloc; callbacks.GarbageCollectionStart = ReferenceChainTracker::GarbageCollectionStart; - callbacks.GarbageCollectionFinish = ForwardedGarbageCollectionFinish; + callbacks.GarbageCollectionFinish = onGarbageCollectionFinish; callbacks.NativeMethodBind = VMStructs::NativeMethodBind; _jvmti->SetEventCallbacks(&callbacks, sizeof(callbacks)); 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 c65d80b00d..dea1dee574 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -462,6 +462,19 @@ public Map getDebugCounters() { private static native int getTid0(); + /** + * Test seam (debug native builds only): returns the calling thread's profiler tid + * (ProfiledThread::currentTid()'s value on the native side - the same tid space + * {@code seedTidTrendSample0} matches tracked instances' allocating threads + * against). Scenarios seeding per-(klass, tid) trend ramps must call this ON + * the leaking thread and pass the result to {@code seedTidTrendSample0}, or + * the qualifying tid will match no tracked instance and no leak tag will + * ever be assigned to the scenario's real instances. + */ + public static int getTid() { + return getTid0(); + } + private static native boolean recordTrace0(long rootSpanId, String endpoint, String operation, int sizeLimit); private static native void dump0(String recordingFilePath); @@ -530,6 +543,181 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l */ public static native boolean testTlsPrimingAvailable(); + /** + * Test seam (debug native builds only - a no-op returning {@code false}/{@code 0}/an + * empty array in release builds): decouples LivenessTracker's leak-candidate + * detection from ReferenceChainTracker's chain reconstruction, each independently + * verifiable end-to-end without depending on both the probabilistic JVMTI heap + * sampler and the reference-chain BFS search organically producing the right + * conditions in the same test run. + *

+ * Enables/disables LivenessTracker's per-klass population tracking directly, + * bypassing {@code initialize()}'s live-JVM requirement. Returns {@code true} on + * debug builds. + */ + static native boolean setGcGenerationsEnabled0(boolean enabled); + + /** + * Test seam (debug native builds only): seeds one epoch's worth of population + * history for {@code klassId} directly into LivenessTracker's ring buffer, + * bypassing real allocation sampling. Repeated calls (with distinct + * {@code epoch} values) build up a trend {@link #selectLeakCandidateKlassIds0()} + * can then rank, letting a test assert a slope signal would be generated for a + * chosen klass id without waiting on real GC epochs. + */ + static native void seedKlassPopulationSample0(int klassId, int count, long epoch); + + /** + * Test seam (debug native builds only): seeds one epoch's worth of per-tid trend + * history for {@code klassId}, the (klass, tid) qualification + * selectLeakCandidates() now requires on top of the klass-level ramp seeded by + * {@link #seedKlassPopulationSample0}. Repeated calls with the same rising shape + * (and the same {@code tid}) build the sustained rise that qualifies {@code tid} + * as a leak-site thread. {@code tid} must be the leaking thread's real profiler + * tid from {@link #getTid()} whenever the scenario also relies on leak tagging + * of real tracked instances; see that method's own comment. + */ + static native void seedTidTrendSample0(int klassId, int tid, int count, long epoch); + + /** + * Test seam (debug native builds only): wires {@code representative} in as {@code klassId}'s + * leak-candidate representative directly (a fresh weak global ref owned by LivenessTracker), + * bypassing the real allocation-sampling path that would otherwise populate this. Combined + * with {@link #seedKlassPopulationSample0} and {@link #tagAsReferenceChainRoot0}, lets a test + * join a synthetic slope signal to a real, directly-tagged object so + * {@link #pollReferenceChainTargets0()}'s bridging step can be exercised end-to-end with + * neither the real sampler nor the real root-seeded walk involved. + */ + static native void setKlassPopulationRepresentativeForTest0(int klassId, Object representative); + + /** + * Test seam (debug native builds only): clears LivenessTracker's per-klass population table, + * so a later test in the same JVM does not observe leak candidates seeded by an earlier one. + */ + static native void resetKlassPopulationForTest0(); + + /** + * Test seam (debug native builds only): returns the klass ids LivenessTracker's + * real leak-candidate ranking (positive population slope, top 5) currently + * selects - the same call ReferenceChainTracker's restart gate and target-polling + * bridge use in production, exposed here so a test can assert a slope signal was + * generated (real or seeded via {@link #seedKlassPopulationSample0}) without + * needing a reference-chain search to also be running. + */ + static native int[] selectLeakCandidateKlassIds0(); + + /** + * Test seam (debug native builds only): tags {@code target} and inserts it + * directly as a reference-chain frontier root, bypassing ReferenceChainTracker's + * normal discovery path (a root-seeded FollowReferences walk) and + * LivenessTracker's leak-candidate selection entirely. Lets a test drive + * {@link #runReferenceChainPass0()}/{@link #pollReferenceChainTargets0()} against + * a known, caller-chosen live object. Returns the assigned frontier tag (matching + * the {@code target_tag} a resulting {@code datadog.ReferenceChain} event + * reports), or {@code 0} on failure (reference chains disabled, or the frontier + * table is at capacity). + */ + static native long tagAsReferenceChainRoot0(Object target); + + /** + * Test seam (debug native builds only): runs exactly one bounded BFS pass of the + * reference-chain search synchronously, rather than waiting on the tracker's own + * background thread/cadence. Returns {@code false} if reference chains are + * disabled or the tracker was never started. + */ + static native boolean runReferenceChainPass0(); + + /** + * Test seam (debug native builds only): runs one poll of + * ReferenceChainTracker's LivenessTracker-to-chain-reconstruction bridging step + * synchronously - for each current leak candidate already discovered by a prior + * {@link #runReferenceChainPass0()} walk, reconstructs and queues its chain + * event, rather than waiting on the background thread's own scheduling cycle. + */ + static native void pollReferenceChainTargets0(); + + /** + * Test seam (debug native builds only): drains and returns the number of + * reference-chain events queued by {@link #pollReferenceChainTargets0()} so far + * (the same queue {@code Profiler.dump()} drains in production to write + * {@code datadog.ReferenceChain} JFR events) - lets a test assert a chain was + * actually reconstructed without needing a real JFR dump. + */ + static native int drainReferenceChainEventCount0(); + + /** + * Test seam (debug native builds only): resets ReferenceChainTracker's search/frontier state + * back to a brand-new tracker's, releasing any tags a previous search still held. Since the + * tracker is a process-wide singleton, an in-process test that needs its own genuine first + * root-seeded walk (runPass() only re-walks from the roots once per search's whole lifetime) + * calls this at the start of its test body to force one, rather than depending on being the + * first reference-chain test to run in a shared test JVM. + */ + static native void resetReferenceChainSearchForTest0(); + + /** + * Test seam (debug native builds only): diagnostic-only, does not tag {@code target}. Reads + * target's existing JVMTI tag (0 if the real search has never admitted it) and reports its + * FIFO distance from the front of ReferenceChainTracker's pending-expansion queue: {@code >=0} + * (0 = expands next) if still queued, {@code -1} if tagged but no longer queued (already + * expanded), or {@code -2} if never admitted at all. + */ + static native long getReferenceChainPendingPositionForTest0(Object target); + + /** + * Test seam (debug native builds only): the current size of ReferenceChainTracker's + * pending-expansion queue, for computing {@link #getReferenceChainPendingPositionForTest0}'s + * position as a fraction of the current backlog. + */ + static native long getReferenceChainPendingSizeForTest0(); + + /** + * Test seam (debug native builds only): seeds one heap-floor-ring sample - the input to + * {@code LivenessTracker::secondsToOOM()}'s time-to-OOM projection - directly, bypassing the + * real {@code GarbageCollectionFinish} callback. {@code timestampNs} values are only ever + * compared against each other, never against a real wall clock, so a test may use any + * self-consistent, strictly increasing sequence to build an arbitrary rising or flat + * heap-usage-over-time history without waiting on real GCs. + */ + static native void heapFloorRecordForTest0(long usedBytes, long timestampNs); + + /** + * Test seam (debug native builds only): overrides the max-heap-size {@code secondsToOOM()} + * projects against, bypassing the real {@code Runtime.maxMemory()} resolution - so a test can + * exercise the projection deterministically, independent of whatever {@code -Xmx} this JVM's + * own shared, no-{@code forkEvery} fork happens to run with. + */ + static native void setMaxHeapBytesForTest0(long maxHeapBytes); + + /** + * Test seam (debug native builds only): temporarily disables {@code onGC()}'s own + * {@code recordHeapFloorSample()} call so a test can seed the heap-floor ring exclusively + * via {@link #heapFloorRecordForTest0(long, long)} without a real GC interleaving a sample + * with a real {@code OS::nanotime()} timestamp and real heap usage, corrupting + * {@code secondsToOOM()}'s projection. Pass {@code false} to disable, {@code true} to restore. + */ + static native void setHeapFloorRecordingForTest0(boolean enabled); + + /** + * Test seam (debug native builds only): reports whether ReferenceChainTracker's search-restart + * gate ({@code canAffordNewSearch()} -> {@code hasLeakSignal()}) would currently allow a + * fresh/terminal search to start - in particular, whether {@code secondsToOOM()}'s urgent-OOM + * bypass opens this gate even with zero per-klass leak candidate (confirmable in the same test + * via {@link #selectLeakCandidateKlassIds0()}). Unlike {@link #runReferenceChainPass0()}, which + * calls {@code runPass()} unconditionally, this reads the gate itself without running a pass. + */ + static native boolean shouldRunPassForTest0(); + + /** + * Test seam (debug native builds only): the number of BFS passes run for the current/most + * recent reference-chain search. Lets a test note this count before creating an object, then + * wait for it to advance before trusting a match against that object - the only way to be + * certain the match came from a pass whose own {@code expandFrontier()} (and therefore {@code + * collectStaleExpandedEntriesForRotation()}) ran strictly after the object existed, rather + * than from the same pass racing the object's creation. + */ + static native int referenceChainPassesRunForTest0(); + // ---- 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-lib/src/test/cpp/livenessTracker_ut.cpp b/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp index 557280a930..ce6a0fb11d 100644 --- a/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp +++ b/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp @@ -1125,6 +1125,60 @@ TEST_F(SecondsToOOMTest, RisingFloorProjectsExpectedSeconds) { EXPECT_NEAR(tracker->secondsToOOM(), 9.0, 1e-6); } +// A dip-then-recover window (usage falls for half the window, then rises +// back to where it started): the full-window fitted-line endpoints nearly +// agree (byte delta ~0), so the unguarded division would project +inf +// seconds - silently disabling the urgency ramp from this boundary forever. +// The boundary must be SKIPPED (no projection from this ring), and the +// result must be finite. Here the container ring is unavailable (no +// container limit set), so the heap ring's skip means no projection at all. +TEST_F(SecondsToOOMTest, DipThenRecoverFloorReturnsNegativeNotInf) { + LivenessTracker *tracker = LivenessTracker::instance(); + tracker->setMaxHeapBytesForTest((jlong)(2800 * MiB)); + // Perfectly symmetric V: pairs (i, 9-i) carry equal usage, so the + // least-squares fit's slope is exactly 0 in double arithmetic and the + // fitted endpoints agree EXACTLY (delta == 0), while the recent half + // (indices 5..9: 1500..1900) rises steeply - corroboration passes, the + // full-window denominator does not. This is precisely the shape the + // zero-denominator guard exists for; an approximately-equal pair would + // only produce a huge-but-finite projection instead of the +inf the + // guard prevents. + static const long shape[] = {1900, 1800, 1700, 1600, 1500, + 1500, 1600, 1700, 1800, 1900}; + for (int i = 0; i < 10; i++) { + tracker->heapFloorRecordForTest((u64)shape[i] * MiB, (u64)i * SEC_NS); + } + double secs = tracker->secondsToOOM(); + EXPECT_LT(secs, 0.0) + << "a dip-then-recover window must not offer a projection"; + EXPECT_TRUE(secs > -1e18 && secs < 1e18) + << "the projection must be finite, never +inf"; +} + +// Odd-length window (11 samples): pins the recent-half boundary of the +// corroboration pass (ringWindowStats' recent_min boundary is i >= n/2, so +// for odd n the median sample joins the RECENT half) and the fitted-line +// endpoint arithmetic (recent_mean at x = n-1). A rising ramp must still +// project, and the projection must match the even-window scaling of the +// same 100MiB/s rate - this is the mutation-coverage the review asked for +// on ringWindowStats' two boundary mutations (i >= n/2 and endpoint n-1). +TEST_F(SecondsToOOMTest, OddLengthRisingWindowStillProjects) { + LivenessTracker *tracker = LivenessTracker::instance(); + tracker->setMaxHeapBytesForTest((jlong)(3000 * MiB)); + // 11 samples, 100MiB apart, one second apart: 1000..2000MiB. + for (int i = 0; i < 11; i++) { + tracker->heapFloorRecordForTest(1000 * MiB + (u64)i * 100 * MiB, + (u64)i * SEC_NS); + } + double secs = tracker->secondsToOOM(); + // Rising floor, recent fitted endpoint at 2000MiB, headroom 1000MiB at + // 100MiB/s -> ~10s. Assert finite, positive, and in the right decade - + // exact value depends on the fitted endpoints the regression produces, + // which the even-window test above already pins precisely. + EXPECT_GT(secs, 0.0); + EXPECT_NEAR(secs, 9.0, 1.5); +} + // The floor's own recent-third mean has already reached the max heap size - // exhaustion is "now", not some positive number of seconds out. TEST_F(SecondsToOOMTest, FloorAtMaxHeapReturnsZero) { @@ -1197,6 +1251,40 @@ TEST_F(LeakTagPoolTest, ReleaseReturnsTagToPoolAndInfoIsInvalidated) { EXPECT_EQ(pool_size - 1, tracker->leakTagFreeCountForTest()); } +// A double release (releasing a tag that is already free) must be rejected: +// pushing the index twice would let acquireLeakTag() hand the same tag to +// two live objects. Found reachable in review via tagLeakInstances()' tag- +// adoption branch; the release path now guards on the zero/zero encoding. +TEST_F(LeakTagPoolTest, DoubleReleaseIsRejectedWithoutCorruptingTheFreeList) { + LivenessTracker *tracker = LivenessTracker::instance(); + int pool_size = tracker->leakTagPoolSizeForTest(); + + jlong tag = tracker->acquireLeakTagForTest(7, 7); + ASSERT_GE(tag, tracker->leakTagBaseForTest()); + tracker->releaseLeakTagForTest(tag); + EXPECT_EQ(pool_size, tracker->leakTagFreeCountForTest()); + + // Second release of the same tag: must be a no-op, not a second push. + tracker->releaseLeakTagForTest(tag); + EXPECT_EQ(pool_size, tracker->leakTagFreeCountForTest()) + << "double release must not grow the free list past the pool"; + + // The pool is still fully usable afterwards: draining and refilling it + // hands out exactly pool_size distinct tags, never a duplicate while + // all are live. + jlong seen[1]; + (void)seen; + int acquired = 0; + for (int i = 0; i < pool_size; i++) { + if (tracker->acquireLeakTagForTest(500 + i, 1) != 0) { + acquired++; + } + } + EXPECT_EQ(pool_size, acquired); + EXPECT_EQ(0, tracker->acquireLeakTagForTest(1, 1)) + << "pool must still exhaust at exactly LEAK_TAG_POOL_SIZE"; +} + TEST_F(LeakTagPoolTest, ReleaseOutsidePoolRangeIsIgnored) { LivenessTracker *tracker = LivenessTracker::instance(); jlong base = tracker->leakTagBaseForTest(); diff --git a/ddprof-lib/src/test/cpp/referenceChainsCoreTestsPorted.inc b/ddprof-lib/src/test/cpp/referenceChainsCoreTestsPorted.inc new file mode 100644 index 0000000000..b0a4ecb569 --- /dev/null +++ b/ddprof-lib/src/test/cpp/referenceChainsCoreTestsPorted.inc @@ -0,0 +1,98 @@ +// Tests from the rc-4 monolith-era ut body: PriorityExpandSet and ClassTagAllocator +// coverage the split-TU .inc families never had. (The five fold-gap regression +// tests are already present in the Bfs/Traversal/Core .inc families on this stack.) + +// PriorityExpandSet (referenceChains.h) - the fixed-capacity open-addressed +// set behind _priority_expand. Reviewed gaps: no test referenced the class +// at all, so the probe/insert/clear/rebuildFrom paths and the full-table +// self-termination were only covered indirectly. Driven through the test +// accessor (the class is a private member of the tracker singleton); all +// operations here are pure in-process logic, no JVMTI. +// --------------------------------------------------------------------------- +TEST(PriorityExpandSetTest, InsertContainsClearRoundTrip) { + ReferenceChainsTestAccessor::pesClear(); + + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(42)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesInsert(42)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(42)); + // Idempotent insert: already present returns false. + EXPECT_FALSE(ReferenceChainsTestAccessor::pesInsert(42)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesInsert(43)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(43)); + + ReferenceChainsTestAccessor::pesClear(); + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(42)) + << "clear must empty the set"; + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(43)); + + // Re-insert after clear (slot reuse must work). + EXPECT_TRUE(ReferenceChainsTestAccessor::pesInsert(42)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(42)); + ReferenceChainsTestAccessor::pesClear(); +} + +TEST(PriorityExpandSetTest, RebuildFromMatchesQueueContents) { + ReferenceChainsTestAccessor::pesClear(); + + std::deque queue = {10, 20, 30}; + ReferenceChainsTestAccessor::pesRebuildFrom(queue); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(10)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(20)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(30)); + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(40)); + + // Pop the front (rotation collection's ordinary shape) and rebuild: + // membership must track the queue exactly - 10 is gone. + queue.pop_front(); + ReferenceChainsTestAccessor::pesRebuildFrom(queue); + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(10)) + << "stale member after rebuildFrom"; + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(20)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pesContains(30)); + ReferenceChainsTestAccessor::pesClear(); +} + +// A completely full table must not livelock the linear-probe loop: insert() +// is required to be self-terminating (returning false - the same "treated +// as not-queued" degradation a full deque push already accepts) even if a +// call site breaks the external "<= PRIORITY_EXPAND_CAP by construction" +// invariant. Fills to capacity with DISTINCT tags, then asserts one more +// insert on a fresh tag terminates (and returns false), and that a +// duplicate of an existing member still returns false via the key match. +// ClassTagAllocator (classTagAllocator.h) - the process-wide negative +// class-object tag allocator shared by ReferenceChainTracker and +// LivenessTracker. Reviewed gap: no test exercised its conventions +// directly. resetForTest() makes each test's minting sequence +// deterministic despite the process-wide counter. +TEST(ClassTagAllocatorTest, MintsStrictlyNegativeStrictlyDecreasingTags) { + ClassTagAllocator::resetForTest(); + jlong first = ClassTagAllocator::next(); + jlong second = ClassTagAllocator::next(); + EXPECT_LT(first, 0) << "class tags must be negative (heapReferenceCallback" + "() dispatches on the sign)"; + EXPECT_LT(second, first) << "tags must decrease monotonically (no reuse)"; + ClassTagAllocator::resetForTest(); + EXPECT_EQ(first, ClassTagAllocator::next()) + << "resetForTest must restart the sequence at the same value"; +} + +TEST(PriorityExpandSetTest, InsertOnFullTableTerminatesWithoutLivelock) { + ReferenceChainsTestAccessor::pesClear(); + + const int capacity = 1 << 11; // 2048 slots (SLOT_SHIFT == 11) + for (int i = 0; i < capacity; i++) { + // Distinct tags spread by Fibonacci hashing - sequential ints are + // the worst case for proving full-table behavior is probe-safe, so + // use them deliberately: they land on distinct slots only via the + // mix(), exercising the wrap path. + ASSERT_TRUE(ReferenceChainsTestAccessor::pesInsert(1000 + (jlong)i * 7919)) + << "insert must succeed until the table is full (i=" << i << ")"; + } + // Table full: a fresh tag must return false promptly (no hang). + EXPECT_FALSE(ReferenceChainsTestAccessor::pesInsert(987654321)); + // A duplicate of an existing member still returns false via the key + // match (found while probing). + EXPECT_FALSE(ReferenceChainsTestAccessor::pesInsert(1000)); + ReferenceChainsTestAccessor::pesClear(); + EXPECT_FALSE(ReferenceChainsTestAccessor::pesContains(1000)); +} diff --git a/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc b/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc index e8be62ca72..29dbc36d1f 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsOomTests.inc @@ -259,12 +259,13 @@ TEST_F(ReferenceChainsCanaryRefillTest, BackoffGateHoldsUnlessOomRampActive) { ReferenceChainsTestAccessor::setSearchStartedForTest(true); // Open chase with the backoff engaged: mult=4 x ema=100ms = 400ms - // spacing, and only 50ms since the last pass. + // spacing, and only 50ms since the last pass. (HEAD's canary EMA is + // stored in ns - the sweep's ms->ns rename - so the seam takes ns.) ReferenceChainsTestAccessor::setCandidateCountForTest(2); ReferenceChainsTestAccessor::setCandidateFoundBitsForTest(0x1); u64 now = 6000000000ULL; // 6s into the test's synthetic clock ReferenceChainsTestAccessor::setCanaryBackoffForTest( - 4, 100, now - 50 * 1000000ULL); + 4, 100ULL * 1000000ULL, now - 50 * 1000000ULL); ReferenceChainsTestAccessor::setOomRampActive(false); EXPECT_FALSE(ReferenceChainsTestAccessor::shouldRunPass(now)); diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index d86ca72dde..d26c1afdc0 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -14,6 +14,8 @@ #ifndef REFERENCE_CHAINS_TEST_ACCESSORS_H #define REFERENCE_CHAINS_TEST_ACCESSORS_H +#include +#include #include #include @@ -223,12 +225,24 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::instance()->drainPendingChainEvents(out); } + // Faithful pass-through: cacheResolvedChain() keys _resolved_chains by + // source_tag and records source_tag_val as the entry's source tag (the + // frontier tag the chain was resolved from), so both are forwarded + // unchanged. static void cacheChain(jlong source_tag, ReferenceChainEvent event, jlong source_tag_val, u64 source_search_ns) { ReferenceChainTracker::instance()->cacheResolvedChain( source_tag, std::move(event), source_tag_val, source_search_ns); } + // HEAD's collapsed form - what the inline (pre-merge) ut accessor + // exposed; kept so both call shapes compile against this one class. + static void cacheChain(jlong source_tag, ReferenceChainEvent event, + u64 source_search_ns) { + cacheChain(source_tag, std::move(event), source_tag, source_search_ns); + } + + static int maxResolvedChains() { return ReferenceChainTracker::MAX_RESOLVED_CHAINS; } @@ -888,6 +902,24 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::instance() ->seedLeakAccumulationForNewlyWatchedKlass(klass_id); } + // PriorityExpandSet drives (the set type is private; the friend class + // reaches it for the PriorityExpandSet unit tests). + static void pesClear() { + ReferenceChainTracker::instance()->_priority_expand_set.clear(); + } + + static bool pesContains(jlong tag) { + return ReferenceChainTracker::instance()->_priority_expand_set.contains(tag); + } + + static bool pesInsert(jlong tag) { + return ReferenceChainTracker::instance()->_priority_expand_set.insert(tag); + } + + static void pesRebuildFrom(const std::deque &queue) { + ReferenceChainTracker::instance()->_priority_expand_set.rebuildFrom(queue); + } + }; #endif // REFERENCE_CHAINS_TEST_ACCESSORS_H diff --git a/ddprof-lib/src/test/cpp/referenceChains_ut.cpp b/ddprof-lib/src/test/cpp/referenceChains_ut.cpp index 2c1815728c..87e7fc0cc0 100644 --- a/ddprof-lib/src/test/cpp/referenceChains_ut.cpp +++ b/ddprof-lib/src/test/cpp/referenceChains_ut.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -54,3 +55,4 @@ static ReferenceChainsGlobalSetup global_setup; #include "referenceChainsTraversalTests.inc" #include "referenceChainsAnchorTests.inc" #include "referenceChainsEventTests.inc" +#include "referenceChainsCoreTestsPorted.inc"