From 2277cb5398dc1615fa60a5e3a8f7c81a99832473 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Sat, 24 Oct 2026 20:15:25 +0200 Subject: [PATCH 1/6] Keep resolved leak chains across improve-path invalidations improveChain/reparent/root-kind upgrades queued a deferred invalidation that drainPendingChainEvents applied as an erase at dump time. If the search then terminated via the canary, nothing could rebuild the erased chains (candidate and discovered state are zeroed, search pain-budget blocked ~11-15s), losing the correlation for the rest of the window - against the restartSearch() contract that resolved chains stay cached across a restart. Split deferred invalidations into EVICT (noise eviction, filter: erased at dump time as before) and REBUILD (path changed, source still live: applied by the BFS-thread poll, which rebuilds the chain from the intact frontier and keeps the old chain if the rebuild fails). Kind is encoded in-band in the existing bounded pending buffers; no new allocation or locks. Reproducer: ReferenceChainsBfsTest.ImprovedPathInvalidationKeepsLeakChainAcrossTerminalReset (fails before, passes after). PROF-16316. --- .../src/main/cpp/referenceChainEvents.cpp | 123 +++++++++++++++-- .../src/main/cpp/referenceChainTraversal.cpp | 3 +- .../src/main/cpp/referenceChainWalk.cpp | 13 +- ddprof-lib/src/main/cpp/referenceChains.h | 49 +++++-- .../src/test/cpp/referenceChainsBfsTests.inc | 126 ++++++++++++++++++ .../test/cpp/referenceChainsTestAccessors.h | 8 +- 6 files changed, 295 insertions(+), 27 deletions(-) diff --git a/ddprof-lib/src/main/cpp/referenceChainEvents.cpp b/ddprof-lib/src/main/cpp/referenceChainEvents.cpp index 7290ce94a0..e22a6c2b05 100644 --- a/ddprof-lib/src/main/cpp/referenceChainEvents.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainEvents.cpp @@ -150,12 +150,13 @@ void ReferenceChainTracker::pollWatchedTargets(jvmtiEnv *jvmti, JNIEnv *jni) { return; } - // Don't rebuild chains against cache entries the heap 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); + // Don't rebuild chains against cache entries the heap callback asked to drop, and refresh the + // ones whose path changed (the only site that applies REBUILD invalidations). + applyPendingChainInvalidations(jvmti, jni, current_search_ns); + // klass_ids resolved (and therefore already pruned-if-dead) by the candidate loop below, so the // prune pass afterwards skips re-resolving them - it only needs to cover cached klasses that are // no longer flagged. @@ -572,7 +573,7 @@ int ReferenceChainTracker::snapshotResolvedChainTags(jlong *out) { void ReferenceChainTracker::deferResolvedChainInvalidation( jlong source_tag, const jlong *resolved_chain_tags, - int resolved_chain_tag_count) { + int resolved_chain_tag_count, ChainInvalidationKind kind) { bool was_cached = false; for (int i = 0; i < resolved_chain_tag_count; i++) { if (resolved_chain_tags[i] == source_tag) { @@ -583,32 +584,61 @@ void ReferenceChainTracker::deferResolvedChainInvalidation( if (!was_cached) { return; } + // Cache keys are frontier tags (>= 0); a negative key would alias another tag's REBUILD + // encoding, so it is never queued. + assert(source_tag >= 0); + if (source_tag < 0) { + return; + } + const jlong evict = encodePendingChainInvalidation(source_tag, ChainInvalidationKind::EVICT); + const jlong rebuild = encodePendingChainInvalidation(source_tag, ChainInvalidationKind::REBUILD); _pending_chain_invalidations_lock.lock(); - for (jlong tag : _pending_chain_invalidations) { - if (tag == source_tag) { + for (jlong &pending : _pending_chain_invalidations) { + if (pending == evict) { + _pending_chain_invalidations_lock.unlock(); + return; // EVICT already queued - it wins over any REBUILD + } + if (pending == rebuild) { + if (kind == ChainInvalidationKind::EVICT) { + pending = evict; // the source was rejected after its path changed: erase, don't rebuild + } _pending_chain_invalidations_lock.unlock(); return; } } // Cache insertion is serialized with traversals on the engine thread and dumps can only remove - // keys, so one pre-traversal snapshot bounds distinct eligible tags to the cache cap. + // keys, so one pre-traversal snapshot bounds distinct eligible tags to the cache cap. REBUILD + // entries the dump path leaves pending do not break the bound: only pollWatchedTargets() inserts + // into the cache, and it drains every pending entry first. assert((int)_pending_chain_invalidations.size() < MAX_PENDING_CHAIN_INVALIDATIONS); if ((int)_pending_chain_invalidations.size() < MAX_PENDING_CHAIN_INVALIDATIONS) { - _pending_chain_invalidations.push_back(source_tag); + _pending_chain_invalidations.push_back( + kind == ChainInvalidationKind::REBUILD ? rebuild : evict); } _pending_chain_invalidations_lock.unlock(); } void ReferenceChainTracker::drainPendingChainInvalidations() { // Serialize drainers and scratch-buffer use with the cache. The producer lock is held only for - // the O(1) handoff, never for unordered_map erase or node destruction. + // a bounded (<= MAX_PENDING_CHAIN_INVALIDATIONS) in-place partition - EVICT entries move to the + // reserved scratch buffer, REBUILD entries stay pending for the BFS thread - never for + // unordered_map erase or node destruction. Neither step reallocates. ExclusiveLockGuard guard(&_resolved_chains_lock); _pending_chain_invalidations_lock.lock(); assert(_draining_chain_invalidations.empty()); - _pending_chain_invalidations.swap(_draining_chain_invalidations); + size_t kept = 0; + for (size_t i = 0; i < _pending_chain_invalidations.size(); i++) { + jlong pending = _pending_chain_invalidations[i]; + if (isPendingChainRebuild(pending)) { + _pending_chain_invalidations[kept++] = pending; + } else { + _draining_chain_invalidations.push_back(pending); + } + } + _pending_chain_invalidations.resize(kept); _pending_chain_invalidations_lock.unlock(); for (jlong tag : _draining_chain_invalidations) { @@ -620,6 +650,76 @@ void ReferenceChainTracker::drainPendingChainInvalidations() { _draining_chain_invalidations.clear(); } +void ReferenceChainTracker::applyPendingChainInvalidations( + jvmtiEnv *jvmti, JNIEnv *jni, u64 current_search_ns) { + // REBUILD tags whose cached chain is current-generation. A chain cached by an earlier search + // under the same numeric tag describes a different object (restartSearch() resets the tag + // namespace), so a path change on the new object says nothing about it: leave it alone. + jlong rebuild_tags[MAX_PENDING_CHAIN_INVALIDATIONS]; + int rebuild_count = 0; + { + ExclusiveLockGuard guard(&_resolved_chains_lock); + _pending_chain_invalidations_lock.lock(); + assert(_draining_chain_invalidations.empty()); + _pending_chain_invalidations.swap(_draining_chain_invalidations); + _pending_chain_invalidations_lock.unlock(); + + for (jlong pending : _draining_chain_invalidations) { + jlong tag = pendingChainInvalidationTag(pending); + if (!isPendingChainRebuild(pending)) { + if (_resolved_chains.erase(tag) > 0) { + TEST_LOG("ReferenceChainTracker::drainPendingChainInvalidations source_tag=%lld", + (long long)tag); + } + continue; + } + auto it = _resolved_chains.find(tag); + if (it != _resolved_chains.end() && + it->second.source_search_ns == current_search_ns && + rebuild_count < MAX_PENDING_CHAIN_INVALIDATIONS) { + rebuild_tags[rebuild_count++] = tag; + } + } + _draining_chain_invalidations.clear(); + } + + for (int i = 0; i < rebuild_count; i++) { + const jlong tag = rebuild_tags[i]; + ReferenceChainEvent event; + // The frontier entry stays intact until restartSearch(), so this does not depend on the + // candidate/discovered slots runPass()'s terminal branch may already have zeroed. + if (!buildChainEvent(jvmti, jni, tag, &event)) { + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations rebuild failed, " + "keeping cached chain source_tag=%lld", + (long long)tag); + continue; + } + if (suppressChainEvent(event)) { + // The new path turned the chain into a filtered shape: same outcome as every other build + // site (see buildDiscoveredInstanceChains()). + invalidateResolvedChain(tag); + continue; + } + if (isLeakTag((jlong)event._target_tag) && !chainTargetsCurrentLeakOwner(event)) { + // Dead-owner rebuild: keep the old chain; the next dump evicts it if it is dead too. + continue; + } + event._start_time = TSC::ticks(); + ExclusiveLockGuard guard(&_resolved_chains_lock); + auto it = _resolved_chains.find(tag); + // Replace only: a dump may have evicted the entry (dead owner) since the scan above, and a + // rebuild must never resurrect it. + if (it != _resolved_chains.end() && + it->second.source_search_ns == current_search_ns) { + it->second.event = std::move(event); + it->second.source_tag = tag; + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations rebuilt " + "source_tag=%lld depth=%u", + (long long)tag, it->second.event._depth); + } + } +} + void ReferenceChainTracker::clearPendingChainInvalidations() { ExclusiveLockGuard cache_guard(&_resolved_chains_lock); _pending_chain_invalidations_lock.lock(); @@ -759,7 +859,8 @@ void ReferenceChainTracker::recordDiscoveredInstance( _candidate_discovered_tags[s][d] = frontier_tag; if (invalidation_mode == CacheInvalidationMode::DEFERRED) { deferResolvedChainInvalidation(victim, resolved_chain_tags, - resolved_chain_tag_count); + resolved_chain_tag_count, + ChainInvalidationKind::EVICT); } else { invalidateResolvedChain(victim); } diff --git a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp index ceb9fb1292..c9e70ed77a 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -129,7 +129,8 @@ jvmtiIterationControl JNICALL ReferenceChainTracker::heapRootCallback( ctx->frontier, *tag_ptr, translated_root_kind)) { ctx->tracker->deferResolvedChainInvalidation( *tag_ptr, ctx->resolved_chain_tags, - ctx->resolved_chain_tag_count); + ctx->resolved_chain_tag_count, + ReferenceChainTracker::ChainInvalidationKind::REBUILD); } break; default: diff --git a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp index 5acd946065..4634f6fb40 100644 --- a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp @@ -457,12 +457,13 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( if (ctx->frontier->improveChain(*tag_ptr, parent_tag, referrer_klass, depth, 0, edge_field_index, (u8)reference_kind)) { - // Chain was improved — invalidate any cached chain for this tag so pollWatchedTargets - // rebuilds it with the deeper path. + // Chain was improved — queue a REBUILD so pollWatchedTargets replaces any cached chain for + // this tag with the deeper path (never erased: the source is still live). // Deferred: we're inside the FollowReferences pause (see deferResolvedChainInvalidation()). ctx->tracker->deferResolvedChainInvalidation( *tag_ptr, ctx->resolved_chain_tags, - ctx->resolved_chain_tag_count); + ctx->resolved_chain_tag_count, + ReferenceChainTracker::ChainInvalidationKind::REBUILD); if (was_root_attached_durable) { // Demotion push: the replaced entry's static/JNI-global attribution was its only // anchor-tier eligibility, and it is gone now. @@ -477,7 +478,8 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( // durable root. ctx->tracker->deferResolvedChainInvalidation( *tag_ptr, ctx->resolved_chain_tags, - ctx->resolved_chain_tag_count); + ctx->resolved_chain_tag_count, + ReferenceChainTracker::ChainInvalidationKind::REBUILD); } } else { // Already-admitted entry reached via a NEW root-like edge (parent_tag == 0): the static-field @@ -489,7 +491,8 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( (u8)reference_kind)) { ctx->tracker->deferResolvedChainInvalidation( *tag_ptr, ctx->resolved_chain_tags, - ctx->resolved_chain_tag_count); + ctx->resolved_chain_tag_count, + ReferenceChainTracker::ChainInvalidationKind::REBUILD); } 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 bfde3616ab..01a0341786 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -586,10 +586,13 @@ class ReferenceChainTracker { std::unordered_map _resolved_chains; SpinLock _resolved_chains_lock; - // Chain evictions deferred by heap callbacks. Only tags present in the immutable pre-traversal - // resolved-cache snapshot are eligible, so distinct pending tags are bounded exactly by the - // cache cap. Both buffers are reserved up front: drains swap the active producer buffer into the - // scratch buffer under the pending lock, then erase cache entries after releasing that lock. + // Chain invalidations deferred by heap callbacks. Only tags present in the immutable + // pre-traversal resolved-cache snapshot are eligible, so distinct pending tags are bounded + // exactly by the cache cap. Both buffers are reserved up front: drains move entries into the + // scratch buffer under the pending lock, then touch cache entries after releasing that lock. + // The ChainInvalidationKind is stored in-band (see encodePendingChainInvalidation()): EVICT + // entries hold the source tag, REBUILD entries its bitwise complement. Every read and write of + // both buffers happens under _pending_chain_invalidations_lock (full-barrier acquire/release). static constexpr int MAX_PENDING_CHAIN_INVALIDATIONS = MAX_RESOLVED_CHAINS; std::vector _pending_chain_invalidations; std::vector _draining_chain_invalidations; @@ -1010,16 +1013,46 @@ class ReferenceChainTracker { // Copies the current cache keys into fixed caller-owned storage before a synchronous traversal. int snapshotResolvedChainTags(jlong *out); - // Queues an eviction from a heap callback only when the tag was cached in the immutable + // What a deferred invalidation does to the cached chain. + // EVICT: the source was rejected (noise eviction) - the cached chain is erased, at dump time or + // on the next poll, whichever drains first. + // REBUILD: the source is still live and its frontier entry intact, only its path changed + // (improveChain()/reparentToDurableRoot()/root-kind upgrade). Never erased at dump time: only + // the BFS-thread poll applies it, rebuilding the chain from the frontier and keeping the old one + // if the rebuild fails (restartSearch()'s keep-across-restart contract). EVICT wins over + // REBUILD for the same pending tag. + enum class ChainInvalidationKind : u8 { EVICT, REBUILD }; + + // In-band kind encoding for the pending buffers. Cache keys are frontier tags (>= 0), so the + // complement of a REBUILD tag is always negative and never collides with an EVICT entry. + static jlong encodePendingChainInvalidation(jlong source_tag, + ChainInvalidationKind kind) { + return kind == ChainInvalidationKind::REBUILD ? ~source_tag : source_tag; + } + static bool isPendingChainRebuild(jlong encoded) { return encoded < 0; } + static jlong pendingChainInvalidationTag(jlong encoded) { + return encoded < 0 ? ~encoded : encoded; + } + + // Queues an invalidation from a heap callback only when the tag was cached in the immutable // pre-traversal snapshot. This never takes _resolved_chains_lock or allocates. void deferResolvedChainInvalidation(jlong source_tag, const jlong *resolved_chain_tags, - int resolved_chain_tag_count); + int resolved_chain_tag_count, + ChainInvalidationKind kind); - // Applies queued exact-tag evictions. Called by the engine thread and by dump threads, which may - // overlap a traversal; the resolved-cache/pending locks provide synchronization. + // Dump-path drain: applies only the queued EVICT invalidations and leaves REBUILD entries + // pending for the BFS thread (see ChainInvalidationKind). May overlap a traversal; the + // resolved-cache/pending locks provide synchronization. void drainPendingChainInvalidations(); + // BFS-thread drain (pollWatchedTargets()): applies EVICT entries like the dump path and rebuilds + // each REBUILD entry's current-generation cached chain from the frontier via buildChainEvent(), + // overwriting it on success and keeping it on failure. Independent of candidate/discovered + // slots, so it still works after runPass()'s terminal branch zeroed them. + void applyPendingChainInvalidations(jvmtiEnv *jvmti, JNIEnv *jni, + u64 current_search_ns); + // Clears both pending buffers at cache/tag-generation boundaries without releasing capacity. void clearPendingChainInvalidations(); diff --git a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc index 6295e26850..3b428dceff 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc @@ -1135,3 +1135,129 @@ TEST_F(ReferenceChainsBfsTest, ResolveOrDropPrunesDeadFrontierEntries) { // harness (design node: design-pod-in-a-jar-harness; meta-whackamole-analysis): the REAL tracker // loop (shouldRunPass -> runPass -> pollWatchedTargets, the exact threadLoop body) driven over the // scripted mock heap, asserting SYSTEM INVARIANTS instead of unit symptoms. + +// PROF-16316: an improveChain() deeper-path invalidation must REPLACE a resolved leak chain, never +// erase it. Sequence from the late-TL RCA: a leak-correlated chain is cached, a later pass re-reaches +// its source through a strictly deeper path (improveChain() queues a deferred invalidation), the +// search then terminates (candidate/discovered slots zeroed, so no poll path can rebuild from +// them), and the next dump drains the pending invalidations. The source is still live, so the dump +// must keep re-emitting its chain, and the BFS-thread poll rebuilds it from the intact frontier. +TEST_F(ReferenceChainsBfsTest, ImprovedPathInvalidationKeepsLeakChainAcrossTerminalReset) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + FrontierTable *frontier = tracker->frontierTable(); + + const u64 kSearchNs = 4242; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kSearchNs); + const jlong kLeakTag = ReferenceChainsTestAccessor::leakTagBase() + 7; + const u32 kStamp = LivenessTracker::instance()->leakTagIncarnation(kLeakTag); + + int parentNode = addNode(); + int leakNode = addNode(); + script = {{JVMTI_HEAP_REFERENCE_FIELD, parentNode, leakNode, -1}}; + node_tags[leakNode] = 105; + node_tags[parentNode] = 104; + // Leak-correlated source 105, first resolved root-attached (depth 0, static field). Parent 104 + // is a durable root whose expansion delivers the deeper (depth 1) path to 105. + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, 105, 0, 0, FrontierEntryState::EXPANDED, + JVMTI_HEAP_REFERENCE_STATIC_FIELD)); + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, 104, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_JNI_GLOBAL)); + frontier->setLeakTag(105, kLeakTag, kStamp); + + ReferenceChainEvent resolved; + ASSERT_TRUE(tracker->buildChainEvent(&mock_jvmti, &mock_jni, 105, &resolved)); + ASSERT_EQ((u64)kLeakTag, resolved._target_tag); + ASSERT_EQ(0u, resolved._depth); + ASSERT_TRUE(ReferenceChainsTestAccessor::cacheChain(105, std::move(resolved), 105, + kSearchNs)); + + // Real improveChain() path: heapReferenceCallback() sees the already-tagged 105 via 104. + ReferenceChainsTestAccessor::pushPendingExpandForTest(104); + int edges = 0; + ReferenceChainsTestAccessor::expandFrontierForTest(&mock_jvmti, &mock_jni, &edges); + FrontierEntry entry{}; + ASSERT_TRUE(frontier->lookup(105, &entry)); + ASSERT_EQ(104, entry.parent_tag); + ASSERT_EQ(1u, entry.depth); + ASSERT_EQ(1u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + // Terminal reset (runPass()'s canary early-termination branch). + ReferenceChainsTestAccessor::setCandidateCountForTest(0); + ReferenceChainsTestAccessor::setCandidateFoundBitsForTest(0); + + // Dump: the still-live leak chain must survive the drain. + std::vector dump; + ReferenceChainsTestAccessor::drain(&dump); + ASSERT_EQ(1u, dump.size()) << "improveChain invalidation erased a live leak chain"; + EXPECT_EQ((u64)kLeakTag, dump[0]._target_tag); + EXPECT_EQ(kStamp, dump[0]._leak_tag_incarnation); + + // BFS-thread poll: rebuilds the chain from the intact frontier with the deeper path. + LivenessTracker::instance()->klassPopulationResetForTest(); + LivenessTracker::instance()->setGcGenerationsForTest(true); + tracker->pollWatchedTargets(&mock_jvmti, &mock_jni); + LivenessTracker::instance()->setGcGenerationsForTest(false); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + dump.clear(); + ReferenceChainsTestAccessor::drain(&dump); + ASSERT_EQ(1u, dump.size()); + EXPECT_EQ((u64)kLeakTag, dump[0]._target_tag); + EXPECT_EQ(1u, dump[0]._depth) << "rebuild must pick up the improved (deeper) path"; + + tracker->stop(); +} + +// PROF-16316 split semantics: the dump path applies only EVICT (noise eviction still erases at +// dump time, exactly as before), EVICT wins over a REBUILD queued for the same tag, and REBUILD is +// left for the BFS-thread poll, which keeps the old chain when the frontier cannot rebuild it. +TEST_F(ReferenceChainsBfsTest, DumpDrainAppliesEvictAndLeavesRebuildToPoll) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=64")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + const u64 kSearchNs = 777; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kSearchNs); + for (jlong tag = 1; tag <= 3; tag++) { + ReferenceChainEvent event; + event._target_tag = (u64)tag; + ASSERT_TRUE(ReferenceChainsTestAccessor::cacheChain(tag, std::move(event), tag, + kSearchNs)); + } + std::vector snapshot = ReferenceChainsTestAccessor::snapshotResolvedChainTags(); + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(1, snapshot); // noise eviction + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(2, snapshot, true); + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(3, snapshot, true); + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(3, snapshot); // EVICT wins + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(2, snapshot, true); // deduped + EXPECT_EQ(3u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + std::vector dump; + ReferenceChainsTestAccessor::drain(&dump); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(1)) + << "noise-evicted chain must still be erased at dump time"; + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(3)) + << "EVICT queued after REBUILD must erase, not keep, the chain"; + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(2)); + ASSERT_EQ(1u, dump.size()); + EXPECT_EQ(2u, dump[0]._target_tag); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "REBUILD stays pending for the BFS thread"; + EXPECT_EQ(0u, ReferenceChainsTestAccessor::drainingChainInvalidationCount()); + + // No frontier entry for tag 2: the poll's rebuild fails and the old chain is kept. + LivenessTracker::instance()->klassPopulationResetForTest(); + LivenessTracker::instance()->setGcGenerationsForTest(true); + tracker->pollWatchedTargets(&mock_jvmti, &mock_jni); + LivenessTracker::instance()->setGcGenerationsForTest(false); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(2)); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(1)); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(3)); + + tracker->stop(); +} diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index a7045e8582..1b1148ca9d 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -240,10 +240,14 @@ class ReferenceChainsTestAccessor { return std::vector(tags, tags + count); } + // Defaults to EVICT (noise-eviction semantics); pass rebuild=true for the improveChain/ + // reparent/root-kind-upgrade REBUILD kind. static void deferResolvedChainInvalidation( - jlong tag, const std::vector &snapshot) { + jlong tag, const std::vector &snapshot, bool rebuild = false) { ReferenceChainTracker::instance()->deferResolvedChainInvalidation( - tag, snapshot.data(), (int)snapshot.size()); + tag, snapshot.data(), (int)snapshot.size(), + rebuild ? ReferenceChainTracker::ChainInvalidationKind::REBUILD + : ReferenceChainTracker::ChainInvalidationKind::EVICT); } static size_t pendingChainInvalidationCount() { From 4b0879ed40be5fd62a873464a5b2befcdf83eff8 Mon Sep 17 00:00:00 2001 From: Paul Fournillon Date: Fri, 23 Oct 2026 14:23:18 +0200 Subject: [PATCH 2/6] Fix reference-chain revisits and leak candidate admission --- ddprof-lib/src/main/cpp/livenessTracker.cpp | 31 ++++-- .../src/main/cpp/referenceChainAnchors.cpp | 31 ++++-- .../src/main/cpp/referenceChainTraversal.cpp | 24 ++++- .../src/main/cpp/referenceChainWalk.cpp | 10 +- ddprof-lib/src/main/cpp/referenceChains.h | 24 +++-- .../src/test/cpp/livenessTracker_ut.cpp | 78 ++++++++++++++ .../src/test/cpp/referenceChainsBfsTests.inc | 100 ++++++++++++++++++ .../test/cpp/referenceChainsTestAccessors.h | 8 ++ 8 files changed, 274 insertions(+), 32 deletions(-) diff --git a/ddprof-lib/src/main/cpp/livenessTracker.cpp b/ddprof-lib/src/main/cpp/livenessTracker.cpp index 538854d9f6..f3b921dd95 100644 --- a/ddprof-lib/src/main/cpp/livenessTracker.cpp +++ b/ddprof-lib/src/main/cpp/livenessTracker.cpp @@ -882,15 +882,30 @@ void LivenessTracker::insertThreadGen(KlassCountScratch &scratch, return; } } + int idx; if (scratch.thread_count < KlassCountScratch::MAX_THREADS_PER_KLASS) { - int idx = scratch.thread_count++; - scratch.threads[idx].tid = tid; - scratch.threads[idx].ages[0] = age; - scratch.threads[idx].age_count = 1; - scratch.threads[idx].count = 1; // this object is the tid's first this epoch - } - // else: thread table full — additional threads are not tracked, but - // the oldest[] array still captures instances from all threads. + idx = scratch.thread_count++; + } else { + // Keeping only the first tids hides later, larger one-shot cohorts behind + // early small allocations. Replace the smallest observed retained set; + // ties prefer keeping the tid with more distinct surviving ages. + idx = 0; + for (int i = 1; i < scratch.thread_count; i++) { + const auto &candidate = scratch.threads[i]; + const auto &victim = scratch.threads[idx]; + if (candidate.count < victim.count || + (candidate.count == victim.count && candidate.age_count < victim.age_count)) { + idx = i; + } + } + } + // Count only objects observed since this tid entered the scratch. Inherit + // neither the evicted tid's count nor its ages: both feed qualification. + // Reappearing tids can be undercounted at capacity, never overcounted. + scratch.threads[idx].tid = tid; + scratch.threads[idx].ages[0] = age; + scratch.threads[idx].age_count = 1; + scratch.threads[idx].count = 1; } void LivenessTracker::accumulateKlassCount(u32 klass_id, jlong age, diff --git a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp index f46158e822..5ceaf81321 100644 --- a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp @@ -148,11 +148,11 @@ ReferenceChainTracker::collectStaticFieldAnchorsForRotation(int max_count) { // fair pick vector here and is skipped at consumption time via fresh_kept_tags). for (size_t i = 0; i < idx_size; i++) { jlong tag = _static_anchor_index[i]; + jlong own_class_tag = i < _static_anchor_own_class_tags.size() + ? _static_anchor_own_class_tags[i] : 0; FrontierEntry entry{}; if (!frontier->lookupLocked(tag, &entry) || - entry.parent_tag != 0 || - (entry.root_kind != (u8)JVMTI_HEAP_REFERENCE_STATIC_FIELD && - entry.root_kind != (u8)JVMTI_HEAP_REFERENCE_JNI_GLOBAL) || + !isSelectableAnchor(entry, own_class_tag) || (entry.state != FrontierEntryState::FRONTIER && entry.state != FrontierEntryState::EXPANDED) || isQueuedForRotation(tag)) { @@ -201,15 +201,15 @@ ReferenceChainTracker::collectStaticFieldAnchorsForRotation(int max_count) { } pos = (size_t)(it - _static_anchor_index.begin()); } + jlong own_class_tag = pos < _static_anchor_own_class_tags.size() + ? _static_anchor_own_class_tags[pos] : 0; FrontierEntry entry{}; if (!frontier->lookupLocked(tag, &entry) || - entry.parent_tag != 0 || - (entry.root_kind != (u8)JVMTI_HEAP_REFERENCE_STATIC_FIELD && - entry.root_kind != (u8)JVMTI_HEAP_REFERENCE_JNI_GLOBAL) || + !isSelectableAnchor(entry, own_class_tag) || (entry.state != FrontierEntryState::FRONTIER && entry.state != FrontierEntryState::EXPANDED) || isQueuedForRotation(tag) || entry.leak_tag != 0) { - continue; // dead/demoted/queued/leak-tier: first look spent, not + continue; // dead/ineligible/queued/leak-tier: first look spent, not // fresh-kept (the leak tier selects it via the scan if it is leak-tagged) } bool keep = false; // container or not-yet-classified rides the @@ -333,6 +333,23 @@ void ReferenceChainTracker::addToStaticAnchorIndex(jlong tag, root_kind != (u8)JVMTI_HEAP_REFERENCE_JNI_GLOBAL) { return; } + indexAnchorForRotation(tag, own_class_tag); +} + +bool ReferenceChainTracker::isSelectableAnchor(const FrontierEntry &entry, + jlong own_class_tag) const { + if (entry.parent_tag == 0 && + (entry.root_kind == (u8)JVMTI_HEAP_REFERENCE_STATIC_FIELD || + entry.root_kind == (u8)JVMTI_HEAP_REFERENCE_JNI_GLOBAL)) { + return true; + } + auto shape = _class_shape_cache.find(own_class_tag); + return shape != _class_shape_cache.end() && + shape->second == (u8)AnchorClassShape::CONTAINER; +} + +void ReferenceChainTracker::indexAnchorForRotation(jlong tag, + jlong own_class_tag) { // Dedup: a push at first admission and another at upgrade would double-add. if (!_static_anchor_index_tags.insert(tag).second) { return; diff --git a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp index c9e70ed77a..e88f3fded1 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -136,6 +136,18 @@ jvmtiIterationControl JNICALL ReferenceChainTracker::heapRootCallback( default: break; } + // Remember local holders before expansion can reparent them into a deeper + // chain. Known collections still need bounded descend walks after that change + // to observe replacement backing storage. Classification runs outside this + // callback; unknown types are indexed but do not qualify for a walk yet. + if (isTransientRootKind(translated_root_kind) && class_tag != 0 && + *tag_ptr > 0 && !isLeakTag(*tag_ptr)) { + auto shape = ctx->tracker->_class_shape_cache.find(class_tag); + if (shape == ctx->tracker->_class_shape_cache.end() || + shape->second == (u8)AnchorClassShape::CONTAINER) { + ctx->tracker->indexAnchorForRotation(*tag_ptr, class_tag); + } + } return JVMTI_ITERATION_CONTINUE; } @@ -364,7 +376,12 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, std::vector static_anchor_fifo_drained; drainStaticAnchorFifo(STATIC_ANCHOR_FIFO_DRAIN, static_anchor_fifo_drained); for (const AtRiskAnchor &at_risk : static_anchor_fifo_drained) { - static_anchor_tags.push_back(at_risk.tag); + // A reparented collection can also have been selected by the persistent + // container tier. The combined selection is bounded by 48 + 16 entries. + if (std::find(static_anchor_tags.begin(), static_anchor_tags.end(), + at_risk.tag) == static_anchor_tags.end()) { + static_anchor_tags.push_back(at_risk.tag); + } } if (rotation_tags.empty() && leak_accumulation_tags.empty() && stale_expanded_tags.empty() && static_anchor_tags.empty()) { @@ -1045,6 +1062,11 @@ bool ReferenceChainTracker::runPass(jvmtiEnv *jvmti, JNIEnv *jni, if (manual_first_pass) { _search_started = true; store(_search_start_ns, OS::nanotime()); + // Register the candidates that triggered this search before walking. The + // first pass can drain the frontier; without a watched klass it would then + // complete and release its tags before the loop's post-pass poll can + // reconstruct any chain or schedule candidate-scoped walks. + pollWatchedTargets(jvmti, jni); } // Root/stack-ref enumeration alone never discovers a root's transitive children diff --git a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp index 4634f6fb40..af64400cc4 100644 --- a/ddprof-lib/src/main/cpp/referenceChainWalk.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainWalk.cpp @@ -446,9 +446,9 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( // This new path is deeper - replace the shallow root-attached entry with the deeper // chain-attached entry. u32 referrer_klass = ctx->tracker->classTags()->resolve(class_tag); - // Pre-read the CURRENT shape: if improveChain() below succeeds, this root-attached durable - // entry is about to be replaced with a deeper chain-attached one - i.e. it is leaving the - // population collectStaticFieldAnchorsForRotation() can select, at exactly this moment. + // Pre-read the CURRENT shape: improveChain() can remove this entry's + // durable-root eligibility. A known collection remains in the container + // tier; other holders need the FIFO's first look below. FrontierEntry pre_improve_entry{}; bool was_root_attached_durable = ctx->frontier->lookup(*tag_ptr, &pre_improve_entry) && @@ -465,8 +465,8 @@ jint JNICALL ReferenceChainTracker::heapReferenceCallback( ctx->resolved_chain_tag_count, ReferenceChainTracker::ChainInvalidationKind::REBUILD); if (was_root_attached_durable) { - // Demotion push: the replaced entry's static/JNI-global attribution was its only - // anchor-tier eligibility, and it is gone now. + // Ensure a first look even when class-shape reconciliation has not + // yet classified this reparented holder as a collection. ctx->tracker->pushAtRiskStaticAnchor( *tag_ptr, ctx->tracker->classTags()->resolve(class_tag)); } diff --git a/ddprof-lib/src/main/cpp/referenceChains.h b/ddprof-lib/src/main/cpp/referenceChains.h index 01a0341786..9846b8ab6b 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -434,10 +434,8 @@ class ReferenceChainTracker { } } _priority_expand_set; - // A static holder richly referenced from the running graph is EXCLUDED from the static-anchor tier forever once its - // frontier entry is chain-attached (first admitted via a non-root path, or demoted by - // improveChain) - maybeUpgradeRootAttachedRootKind() refuses entries with parent_tag != 0 by - // design, so collectStaticFieldAnchorsForRotation() (parent_tag == 0 filter) can never select it. + // Chain-attached holders without a known collection shape need a first + // descend walk when a static edge or reparenting removes their root eligibility. struct AtRiskAnchor { jlong tag; u32 klass_id; @@ -457,13 +455,14 @@ class ReferenceChainTracker { std::unordered_map _static_anchor_fifo_klass_counts; static constexpr u32 STATIC_ANCHOR_ATRISK_PER_KLASS_CAP = 64; - // Index of root-attached STATIC_FIELD/JNI_GLOBAL frontier entries, so - // collectStaticFieldAnchorsForRotation() iterates O(anchors) instead of scanning the full - // frontier table O(frontier_size). + // Index of durable roots and observed local holders. Known collections remain + // eligible after reparenting, so replacing their storage is still observed. + // Non-collection local holders are excluded after classification. + // collectStaticFieldAnchorsForRotation() iterates these anchors rather than + // scanning the full frontier table. std::vector _static_anchor_index; - // Parallel to _static_anchor_index: the OWN class tag of each anchor object (the class of the - // static field's VALUE, not the holder class). + // Parallel to _static_anchor_index: the runtime class tag of each indexed holder. std::vector _static_anchor_own_class_tags; // Dedup companion for _static_anchor_index (O(1) membership; the population is ~28k on a real JVM @@ -936,14 +935,17 @@ class ReferenceChainTracker { int *edges_admitted, bool *truncated, bool *frontier_cap_hit, u64 *safepoint_ns); - // Selects root-attached durable entries with a tiered cursor: leak-tagged, fresh, container-shaped, - // then everything else fairly. + // Selects durable roots and indexed collections (including reparented ones) + // with a tiered cursor: leak-tagged, fresh, containers, then other roots fairly. std::vector collectStaticFieldAnchorsForRotation(int max_count); + bool isSelectableAnchor(const FrontierEntry &entry, jlong own_class_tag) const; // At-risk anchor push: deduped, capped, and quota-limited per klass. void pushAtRiskStaticAnchor(jlong tag, u32 klass_id); // Add `tag` to _static_anchor_index if its root_kind is a durable anchor-tier kind (STATIC_FIELD // or JNI_GLOBAL). void addToStaticAnchorIndex(jlong tag, jlong own_class_tag, u8 root_kind); + // Shared deduplicated index insertion for durable roots and local holders. + void indexAnchorForRotation(jlong tag, jlong own_class_tag); // True iff `klass` implements java/util/Collection or java/util/Map, directly or transitively // (superclass chain + interfaces of every visited class, depth-bounded, visited set to survive diff --git a/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp b/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp index 5b6e10f90f..688961c772 100644 --- a/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp +++ b/ddprof-lib/src/test/cpp/livenessTracker_ut.cpp @@ -1093,6 +1093,22 @@ TEST_F(SelectLeakCandidatesTest, TopKlassesByGenerationCountEmptyTableReturnsZer // Reaches the private state onGC()'s container-usage gate depends on. class LivenessTrackerTestAccessor { public: + // Exercise the production per-thread accumulator and fold without synthetic + // tid histories. Each pair describes a one-cohort allocating thread. + static void recordThreadCohorts( + LivenessTracker *t, int slot, + const std::vector> &cohorts) { + LivenessTracker::KlassCountScratch scratch{}; + for (const auto &cohort : cohorts) { + for (u32 i = 0; i < cohort.second; i++) { + t->insertThreadGen(scratch, cohort.first, /*age=*/1); + } + } + t->_table_lock.lock(); + t->recordTidTrendSamplesLocked(slot, scratch); + t->_table_lock.unlock(); + } + // Runs one onGC() with the given _container_limit_resolved value and returns the // container usage it recorded. Restores the touched fields afterwards. static u64 containerUsageRecordedByOnGC(LivenessTracker *t, bool limit_resolved) { @@ -1185,6 +1201,68 @@ class LivenessTrackerTestAccessor { } }; +TEST_F(SelectLeakCandidatesTest, LateAllocatingThreadCanQualifyAfterScratchFills) { + LivenessTracker *tracker = LivenessTracker::instance(); + std::vector> cohorts; + for (int tid = 1; tid <= 16; tid++) { + cohorts.emplace_back(tid, 1); + } + // The larger, later worker must not be permanently hidden behind the first + // 16 workers. All its objects share one GC age, as in the external cache test. + cohorts.emplace_back(99, 12); + for (int epoch = 1; epoch <= 20; epoch++) { + int slot; + bool created; + tracker->klassPopulationRecordForTest(1, epoch, epoch, &slot, &created); + LivenessTrackerTestAccessor::recordThreadCohorts(tracker, slot, cohorts); + } + KlassCandidate out[5]; + ASSERT_EQ(1, tracker->selectLeakCandidates(out, 5)); + ASSERT_EQ(1, out[0].qualifying_tid_count); + EXPECT_EQ(99, out[0].qualifying_tids[0]); +} + +TEST_F(SelectLeakCandidatesTest, ScratchOverflowDoesNotInventRetainedInstances) { + LivenessTracker *tracker = LivenessTracker::instance(); + std::vector> cohorts; + for (int tid = 1; tid <= 40; tid++) { + // Below the retained-count bar, with a flat one-age history: replacing + // another tid must never transfer its counts to the incoming worker. + cohorts.emplace_back(tid, 7); + } + for (int epoch = 1; epoch <= 20; epoch++) { + int slot; + bool created; + tracker->klassPopulationRecordForTest(1, epoch, epoch, &slot, &created); + LivenessTrackerTestAccessor::recordThreadCohorts(tracker, slot, cohorts); + } + KlassCandidate out[5]; + EXPECT_EQ(0, tracker->selectLeakCandidates(out, 5)); +} + +TEST_F(SelectLeakCandidatesTest, ScratchOverflowPreservesStrongEarlyThreads) { + LivenessTracker *tracker = LivenessTracker::instance(); + std::vector> cohorts; + for (int tid = 1; tid <= 16; tid++) { + cohorts.emplace_back(tid, 12); + } + for (int tid = 17; tid <= 40; tid++) { + cohorts.emplace_back(tid, 1); + } + for (int epoch = 1; epoch <= 20; epoch++) { + int slot; + bool created; + tracker->klassPopulationRecordForTest(1, epoch, epoch, &slot, &created); + LivenessTrackerTestAccessor::recordThreadCohorts(tracker, slot, cohorts); + } + KlassCandidate out[5]; + ASSERT_EQ(1, tracker->selectLeakCandidates(out, 5)); + ASSERT_GT(out[0].qualifying_tid_count, 0); + for (int i = 0; i < out[0].qualifying_tid_count; i++) { + EXPECT_LE(out[0].qualifying_tids[i], 16); + } +} + TEST(LivenessTrackerSessionTest, ResetSessionClearsOnlyRecordingGates) { LivenessTracker *tracker = LivenessTracker::instance(); LivenessTrackerTestAccessor::setSessionGatesForTest( diff --git a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc index 3b428dceff..088a9d4b71 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc @@ -5,6 +5,7 @@ #include "referenceChainsTestAccessors.h" #include "referenceChainsMockHeap.h" +#include "referenceChainInternal.h" #include "tsc.h" @@ -282,6 +283,105 @@ TEST_F(ReferenceChainsBfsTest, DiscoversObjectRetainedOnlyByStaticField) { tracker->stop(); } +// A qualifying population must be watched before the first walk can decide the +// search is complete. Polling only afterwards loses the frontier and tags. +TEST_F(ReferenceChainsBfsTest, FirstPassWatchesQualifiedLeakBeforeCompleting) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + LivenessTracker *liveness = LivenessTracker::instance(); + liveness->klassPopulationResetForTest(); + liveness->setGcGenerationsForTest(true); + liveness->classMapGenerationSetForTest( + Profiler::instance()->classMap()->generation()); + for (int epoch = 1; epoch <= 20; epoch++) { + int slot; + bool created; + liveness->klassPopulationRecordForTest(1, epoch, epoch, &slot, &created); + liveness->tidTrendRecordForTest(1, 4242, epoch, epoch); + } + // Production mints this from the sampled class; the gtest population seam + // has no live JVM from which to resolve it. + liveness->klassPopulationSetStableClassTagForTest(1, -41); + int root = addNode(); + script = {{JVMTI_HEAP_REFERENCE_JNI_GLOBAL, -1, root, -1}}; + + // Do not explicitly poll: this is the ordering exercised by the production + // first pass, including a walk that drains its entire ordinary queue. + EXPECT_TRUE(tracker->runPass(&mock_jvmti, &mock_jni)); + EXPECT_EQ(SearchState::RUNNING, tracker->searchState()); + EXPECT_EQ(1, ReferenceChainsTestAccessor::candidateCountForTest()); + EXPECT_NE(0, node_tags[root]); + + liveness->klassPopulationResetForTest(); + liveness->setGcGenerationsForTest(false); + tracker->stop(); +} + +TEST_F(ReferenceChainsBfsTest, RootContainerRemainsWalkableAfterReparentingAndMutation) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + int holder = addNode(); + int parent = addNode(); + int table = addNode(); + int payload = addNode(); + node_tags[holder] = tracker->nextTag(); + node_tags[parent] = tracker->nextTag(); + const jlong container_class_tag = -101; + ReferenceChainsTestAccessor::primeClassShapeForTest(-102, false); + for (int node : {holder, parent}) { + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + tracker->frontierTable(), node_tags[node], 0, 0, + FrontierEntryState::EXPANDED, JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + } + ReferenceChainPassContext ctx{}; + ctx.tracker = tracker; + ctx.frontier = tracker->frontierTable(); + ctx.hop_cap = 64; + ctx.budget = 1000; + for (int repeat = 0; repeat < 2; repeat++) { + ReferenceChainsTestAccessor::reportHeapRootForTest( + JVMTI_HEAP_ROOT_STACK_LOCAL, container_class_tag, + &node_tags[holder], &ctx); + ReferenceChainsTestAccessor::reportHeapRootForTest( + JVMTI_HEAP_ROOT_STACK_LOCAL, -102, &node_tags[parent], &ctx); + } + // Classification runs outside the heap callback. Unknown transient roots + // must not get a descend walk before being confirmed as containers. + EXPECT_TRUE(ReferenceChainsTestAccessor::collectStaticFieldAnchorsForRotationForTest(4).empty()); + ReferenceChainsTestAccessor::primeClassShapeForTest(container_class_tag, true); + // A deeper retaining path replaces the initial local-root attribution. + // The container still needs repeated walks when its backing storage changes. + ASSERT_TRUE(tracker->frontierTable()->improveChain( + node_tags[holder], node_tags[parent], 0, 1, 0)); + ReferenceChainsTestAccessor::clearPriorityExpand(); + auto selected = ReferenceChainsTestAccessor::collectStaticFieldAnchorsForRotationForTest(4); + EXPECT_EQ(std::vector{node_tags[holder]}, selected); + int edges = 0; + ReferenceChainsTestAccessor::walkStaticFieldAnchorsForTest( + &mock_jvmti, &mock_jni, selected, 1000, &edges); + + // Mutate AFTER that first walk. No second root callback re-admits the + // holder, and the fresh-anchor lane has already consumed its first look. + const jlong leak_tag = ReferenceChainsTestAccessor::leakTagBase(); + node_tags[payload] = leak_tag; + script = { + {JVMTI_HEAP_REFERENCE_FIELD, holder, table, -1}, + {JVMTI_HEAP_REFERENCE_ARRAY_ELEMENT, table, payload, -1}, + }; + selected = ReferenceChainsTestAccessor::collectStaticFieldAnchorsForRotationForTest(4); + EXPECT_EQ(std::vector{node_tags[holder]}, selected); + ReferenceChainsTestAccessor::walkStaticFieldAnchorsForTest( + &mock_jvmti, &mock_jni, selected, 1000, &edges); + EXPECT_GT(tags_ever_assigned[table], 0); + EXPECT_GT(tags_ever_assigned[payload], 0); + EXPECT_NE(leak_tag, node_tags[payload]); + tracker->stop(); +} + // Regression test for the resolveLoadedClasses() scan-skip guard: it must compare `class_count != // _last_resolved_class_count`, not `class_count > _last_resolved_class_count`. TEST_F(ReferenceChainsBfsTest, ResolveLoadedClassesRescansAfterClassCountShrinksAndPartiallyRegrows) { diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index 1b1148ca9d..14e56d206b 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -803,6 +803,14 @@ class ReferenceChainsTestAccessor { t->_priority_expand.end()); } + // Report a root without running a whole pass, so its scheduling effect can + // be checked independently of the ordinary rotation collectors. + static jvmtiIterationControl reportHeapRootForTest( + jvmtiHeapRootKind kind, jlong class_tag, jlong *tag, void *context) { + return ReferenceChainTracker::heapRootCallback(kind, class_tag, 0, tag, + context); + } + // Simulates a tag left in _priority_expand by a prior pass's truncated expandFrontier() // batch without a full expandFrontier()/JVMTI round-trip. static void pushPriorityExpand(jlong tag) { From 2908999d6579ea7eeffab2f8c828f5f0657de640 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Sat, 24 Oct 2026 20:15:34 +0200 Subject: [PATCH 3/6] Make reference-chain slow e2e tests robust to walk-pacing latency The reference-chain walks are paced by the CPU pain budget; on slow cells the second pass can land late in the tests' dump windows, leaving only the first pass's uncorrelated chains (TL/LeakTag loose variants report out-of-pool or not-found in CI nightlies, PROF-16305). Test-side schedule changes within the existing bounds (no assertion weakened, no retries/skips, static entry caps kept): - cache-leak: growth completes by round 10, rounds 11-16 poll with a small top-up so the BFS drains its frontier backlog and intercepts a leak-tagged payload with slack (<= 36,601 entries, down from 69,001) - failure-only referenceChainDiagnostics() helper appended to the assertNotNull messages of the cache-leak and gc-root tests (debug- instrumented builds only; release returns empty string) - ThreadLocalLeakScenario: keeps full per-round chunk growth (the late-variant correlation needs the whole interception surface; verified on JDK 11) but keeps the filler at a quarter size so pass 0's pain-budget debt is repaid well inside the window - LeakTagCorrelationScenario: front-loaded growth with sparse top-ups, tail rounds quiet; keep-best ladder unchanged --- .../LeakTagCorrelationScenario.java | 45 +++++++++++-- .../ReferenceChainTrackingTest.java | 63 +++++++++++++++---- .../ThreadLocalLeakReferenceChainTest.java | 7 ++- .../ThreadLocalLeakScenario.java | 40 +++++++++--- 4 files changed, 131 insertions(+), 24 deletions(-) diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenario.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenario.java index 36f7d12b1e..db4baa9101 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenario.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenario.java @@ -90,6 +90,31 @@ private LeakTagCorrelationScenario() {} private static final int SEED_CHUNKS = 10; + /** + * Growth schedule of run()'s 25-round loop (load-bearing, see the loop's own comment): the leak + * grows by {@link #CHUNKS_PER_GROWTH_ROUND} chunks in each of rounds 1..GROWTH_ROUNDS, and only + * by one chunk every {@link #QUIET_TOP_UP_EVERY}th round after that. Invariants: + *
    + *
  • Total leak = SEED_CHUNKS + GROWTH_ROUNDS * CHUNKS_PER_GROWTH_ROUND + (25 - + * GROWTH_ROUNDS) / QUIET_TOP_UP_EVERY = 10 + 20 + 3 = 33 chunks, below the 35 of the earlier + * one-chunk-every-round schedule - a smaller working set, never a larger one.
  • + *
  • Every {@link #LEAK_SINK} backing-array replacement (capacity 10 -> 15 -> 22 -> 33 at + * sizes 11/16/23) happens inside the growth rounds, i.e. after the first-pass wait (so the + * resize case LEAK_SINK exists for is still exercised) and before the tail; the tail + * top-ups fill size 31..33 within the final capacity 33, so the tail never replaces + * elementData again.
  • + *
  • The growth rounds alone span 10 GC epochs (one System.gc() per round), i.e. + * KLASS_POPULATION_MIN_FILL_FOR_TREND ring samples of real growth; the tail top-ups keep a + * fresh surviving-age cohort arriving so the real fold's trend does not go flat, and + * debug builds additionally push one seeded rising epoch every round.
  • + *
+ */ + private static final int GROWTH_ROUNDS = 10; + + private static final int CHUNKS_PER_GROWTH_ROUND = 2; + + private static final int QUIET_TOP_UP_EVERY = 5; + /** * Per-round trend-maintenance epoch counter (debug builds only), same pattern as * StaticFieldGrowingCollectionScenario: a one-shot seeded ramp ages out of LivenessTracker's @@ -309,11 +334,23 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD // every scanned dump are accumulated into the kept result. ScanResult best = null; int totalRounds = 25; - // Slow growth per round is deliberate: each chunk clears the allocation sampler's floor on - // its own (see CHUNK_BYTES), so every allocation is sampled, tracked, and taggable - the - // per-round population growth is only the trend signal, not a sampling-volume requirement. + // Front-loaded growth, quiet tail (see GROWTH_ROUNDS): rounds 1..GROWTH_ROUNDS grow the leak, + // every later round is a quiet poll with only the sparse QUIET_TOP_UP_EVERY top-up. Pass 1 + // (the first pass that can intercept a leak tag) is gated by repaying pass 0's CPU + // pain-budget debt, which on a slow cell lands anywhere in the tail or grace period; with + // the growth already done by then, every chunk is tracked, leak-tagged, and sitting in the + // final elementData whenever that pass lands. Each chunk still clears the allocation + // sampler's floor on its own (see CHUNK_BYTES), so every allocation is sampled, tracked, and + // taggable - the per-round growth is only the trend signal, not a sampling-volume + // requirement, and in debug builds the per-round seeded ramp below keeps both trends + // qualifying through the quiet tail. for (int round = 1; round <= totalRounds; round++) { - addChunk(); + int newChunks = round <= GROWTH_ROUNDS + ? CHUNKS_PER_GROWTH_ROUND + : (round % QUIET_TOP_UP_EVERY == 0 ? 1 : 0); + for (int c = 0; c < newChunks; c++) { + addChunk(); + } if (debugBuild) { hysteresisEpoch++; ReferenceChainTestHooks.seedKlassPopulationSample0( diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTrackingTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTrackingTest.java index 28de9d6789..a1d7084d88 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTrackingTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTrackingTest.java @@ -24,6 +24,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -349,7 +350,9 @@ public void shouldReconstructReferrerChainToGcRoot() throws Exception { ReferenceChainAssertions.JfrChainMatch match = null; boolean seededTestKlassTrend = false; int totalRounds = 16; + int roundsRun = 0; for (int round = 1; round <= totalRounds && match == null; round++) { + roundsRun = round; int newInstances = Math.min(round, 10) * 600; List newLinks = new ArrayList<>(newInstances); currentBatch = newLinks; // published before the request - see the allocator's loop @@ -438,7 +441,8 @@ public void shouldReconstructReferrerChainToGcRoot() throws Exception { assertNotNull(match, "Never observed a datadog.ReferenceChain event whose chain[0] is " + ChainLink.class + " after " + gcRootHolder.size() + " retained ChainLink instances across up to " - + totalRounds + " population-growth rounds plus a grace period"); + + totalRounds + " population-growth rounds plus a grace period" + + (match == null ? referenceChainDiagnostics(gcRootHolder.get(0), roundsRun) : "")); // chain[0] is the *target* object's own class, not its holder's: reconstructChain() appends // each visited FrontierEntry's referrer_klass leaf-to-root, and referrer_klass is populated // from the *discovered object's own* GetClassSignature at insertion time. The target here is @@ -503,15 +507,27 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc Files.deleteIfExists(scratchDumpPath); Thread allocator = null; try { - // Same round-growth/retry shape as shouldReconstructReferrerChainToGcRoot() - see that - // method's own comment for why the loop self-adjusts rather than committing to a fixed - // round count, why per-round scale is sized against ObjectSampler's real 256KiB sampling - // floor rather than this test's own "memory=64" request, why totalRounds is capped at - // 16 rather than a larger margin above the 10-round minimum (shared-fork heap headroom), - // and why per-round growth itself is clamped to round 10 (Math.min(round, 10) below). + // Same retry shape as shouldReconstructReferrerChainToGcRoot() - see that method's own + // comment for why the loop self-adjusts rather than committing to a fixed round count, why + // per-round scale is sized against ObjectSampler's real 256KiB sampling floor rather than + // this test's own "memory=64" request, and why totalRounds is capped at 16 rather than a + // larger margin above the 10-round minimum (shared-fork heap headroom). + // + // Growth schedule, unlike the gc-root test: the round*600 growth completes at round + // growthRounds (10) - the population trend is already eligible by then. Rounds 11-16 are + // quiet poll/maintenance rounds adding only quietTopUpEntries each: enough to keep + // CachedPayload's population ring advancing (a stalled ring lets the candidate lapse out of + // selectLeakCandidates()), small enough that the BFS gets those rounds plus the grace + // period to drain its frontier backlog and reach a leak-tagged CachedPayload inside the + // HashMap's Node[] - a map that kept growing through all 16 rounds kept that backlog ahead + // of the walk until the retry budget ran out. Static bound: 10*11/2*600 + 6*600 + 1 seed + // = 36,601 entries, within the previous all-growth schedule's 69,001. ReferenceChainAssertions.JfrChainMatch match = null; boolean seededTestKlassTrend = false; int totalRounds = 16; + int growthRounds = 10; + int quietTopUpEntries = 600; + int roundsRun = 0; // Pre-generate every key up front rather than concatenating a fresh String on every put(). // The key String (plus its backing byte[]) is the same kind of same-instant, @@ -521,7 +537,7 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc // the sampler has nothing else in flight to compete with once round 1 begins. int maxEntries = 0; for (int round = 1; round <= totalRounds; round++) { - maxEntries += Math.min(round, 10) * 600; + maxEntries += round <= growthRounds ? Math.min(round, 10) * 600 : quietTopUpEntries; } String[] keys = new String[maxEntries]; for (int i = 0; i < keys.length; i++) { @@ -560,11 +576,13 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc awaitAllocatorTid(allocator, allocatorTid, allocatorFailure); for (int round = 1; round <= totalRounds && match == null; round++) { + roundsRun = round; // Same *600 growth rate as the gc-root test (not *300, this method's previous value): // CachedPayload and ChainLink are near-identical in size, so halving the growth rate here // bought no headroom and just let CachedPayload's population ring stall short of - // KLASS_POPULATION_MIN_FILL_FOR_TREND within the same totalRounds budget. - int newEntries = Math.min(round, 10) * 600; + // KLASS_POPULATION_MIN_FILL_FOR_TREND within the growth rounds. Past growthRounds only the + // quiet top-up - see the schedule comment above. + int newEntries = round <= growthRounds ? Math.min(round, 10) * 600 : quietTopUpEntries; if (!allocRequests.offer(newEntries)) { throw new IllegalStateException("allocRequests queue rejected entry count " + newEntries); } @@ -624,7 +642,8 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc assertNotNull(match, "Never observed a datadog.ReferenceChain event whose chain[0] is " + CachedPayload.class + " after " + cache.size() + " cached entries across up to " + totalRounds - + " population-growth rounds plus a grace period"); + + " rounds plus a grace period" + + (match == null ? referenceChainDiagnostics(cache.get(keys[0]), roundsRun) : "")); assertEquals(CachedPayload.class.getName(), match.chain.get(0)); assertTrue(match.targetTag > 0, "targetTag should be a valid, non-zero JVMTI tag"); assertTrue(match.depth >= 0, "depth should be a non-negative hop count"); @@ -790,6 +809,28 @@ private static void awaitAllocatorTid(Thread allocator, AtomicLong allocatorTid, } } + /** + * Failure-only diagnostics for a success-path test's {@code assertNotNull(match, ...)} message: + * call it only once the round and grace loops have ended without a match, so a passing run does + * no extra work. In a debug-instrumented build it reports the current leak candidates + * ({@code selectLeakCandidateKlassIds0()}), the representative's frontier state + * ({@code getReferenceChainFrontierStateForTest0()}, -1 = not in the frontier) and resolved + * target tag ({@code getReferenceChainResolvedTargetTagForTest0()}), and how many rounds ran. + * In a release build the seams do not exist, so it returns {@code ""}. + */ + private static String referenceChainDiagnostics(Object representative, int roundsRun) { + if (!ReferenceChainAssertions.isDebugInstrumentedBuild()) { + return ""; + } + return " [diagnostics: candidates=" + + Arrays.toString(ReferenceChainTestHooks.selectLeakCandidateKlassIds0()) + + " repFrontierState=" + + ReferenceChainTestHooks.getReferenceChainFrontierStateForTest0(representative) + + " repResolvedTargetTag=" + + ReferenceChainTestHooks.getReferenceChainResolvedTargetTagForTest0(representative) + + " roundsRun=" + roundsRun + "]"; + } + /** Bounded wait for one allocator round; same fail-fast policy as awaitAllocatorTid(). */ private static void awaitAllocatorRound(Semaphore roundDone, Thread allocator, AtomicReference allocatorFailure) throws InterruptedException { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakReferenceChainTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakReferenceChainTest.java index b0e631a740..e638e397a6 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakReferenceChainTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakReferenceChainTest.java @@ -52,8 +52,11 @@ void shouldCorrelateThreadLocalHeldLeakChain() throws Exception { Deque testLogTail = new ArrayDeque<>(); try { // Same raised budget/ratio rationale as - // LeakTagCorrelationReferenceChainTest (cold external JVM, ~100k filler, - // memory=64:l:1.0 keeping every chunk allocation tracked). + // LeakTagCorrelationReferenceChainTest (cold external JVM, + // memory=64:l:1.0 keeping every chunk allocation tracked). This + // scenario's filler is ~24k nodes, not ~100k (ThreadLocalLeakScenario.FILLER's + // own comment: it bounds the first pass's CPU pain-budget debt), so the + // budget still leaves that first pass untruncated with headroom. String startCommand = "start,memory=64:l:1.0,generations=true," + "referencechains=true:hops=64:budget=200000:ttl=120000:framecap=2000000:" + "pausetarget=60000" diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakScenario.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakScenario.java index 5f7fb799bf..034e612dcd 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakScenario.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ThreadLocalLeakScenario.java @@ -52,6 +52,21 @@ * own class comment (the one-shot root-seeded walk is a per-process * resource, and a one-shot seeded ramp ages out of LivenessTracker's * hysteresis once real fold samples interleave). + * + *

Schedule shape (see {@link #FILLER}): every round appends one chunk to + * the leak thread's ThreadLocal sink — the chunk supply must span the whole + * window because the correlation needs the walk to intercept a leak-tagged + * chunk inside the thread's ThreadLocalMap, and starving the tail rounds + * starves that interception surface (observed as {@code tl-correlation-not-found} + * on JDK 11 for the late-thread variant). The scale filler, in contrast, is + * kept small: the first (root-enumeration) pass charges its whole + * non-safepoint walk time to the tracker's CPU pain budget, and no further + * pass runs until that debt is repaid; until then the only {@code byte[]} + * chains are that first pass's out-of-pool discovered instances. A smaller + * walked heap shortens the debt so the second pass - the one that builds the + * in-pool chain - lands well inside the 25-round + 5-grace window on slow CI + * cells (an ~8.5 s debt at 96,000 nodes was repaid only at about dump 25 of + * 30 locally). */ public final class ThreadLocalLeakScenario { private ThreadLocalLeakScenario() {} @@ -64,8 +79,18 @@ private ThreadLocalLeakScenario() {} private static final ThreadLocal> SINK = ThreadLocal.withInitial(() -> new ArrayList<>()); - /** Scale ingredient - same rationale as LeakTagCorrelationScenario.FILLER. */ - static final Object[][] FILLER = new Object[1500][]; + /** + * Scale ingredient - same rationale as LeakTagCorrelationScenario.FILLER, + * but a quarter of its size (375 x 64 = 24,000 nodes, not 96,000). + * Invariant: the filler is the bulk of the heap the first pass walks (the + * byte[] chunks are reference-free leaves), and that pass's non-safepoint + * time is the CPU pain-budget debt the second pass waits out - the one + * that builds the in-pool chain. The filler must stay small enough that the + * debt is repaid well inside the 25-round + 5-grace window on slow CI + * cells (an ~8.5 s debt at 96,000 nodes repaid only at about dump 25 of 30 + * locally); it still gives the frontier and the tag map real depth. + */ + static final Object[][] FILLER = new Object[375][]; // Distinct from the other scenarios' ids (987301-987304) - each scenario // runs in its own child JVM, but a distinct id keeps logs unambiguous. @@ -181,7 +206,8 @@ public static void run(JavaProfiler profiler, String startCommand, // One round = the main thread offers a token; the leak thread takes it and // appends one chunk to ITS OWN ThreadLocal sink (same persistent-allocator // shape as ReferenceChainTrackingTest's togcroot fixture, so the chunks are - // always allocated on the same qualifying tid). + // always allocated on the same qualifying tid). Every round of the window + // offers a token - see the class comment's schedule-shape paragraph. BlockingQueue rounds = new ArrayBlockingQueue<>(32); Thread leakThread = new Thread(() -> { // No local ever holds the sink: re-fetch it via SINK on every use so @@ -259,6 +285,8 @@ public static void run(JavaProfiler profiler, String startCommand, ScanResult best = null; int totalRounds = 25; for (int round = 1; round <= totalRounds; round++) { + // Every round appends one chunk - the interception-surface invariant in the + // class comment; do not front-load this growth away. if (!rounds.offer(round)) { throw new IllegalStateException("rounds queue rejected round " + round); } @@ -292,10 +320,8 @@ public static void run(JavaProfiler profiler, String startCommand, LEAK_KLASS_ID, hysteresisEpoch * 10, hysteresisEpoch); ReferenceChainTestHooks.seedTidTrendSample0(LEAK_KLASS_ID, leakTid, hysteresisEpoch * 3, hysteresisEpoch); } - if (!rounds.offer(attempt)) { - throw new IllegalStateException( - "rounds queue rejected attempt " + attempt); - } + // No chunk is appended in the grace attempts; the trend maintenance + // above is what keeps the candidate alive. Thread.sleep(1000); System.gc(); profiler.dump(scratchDumpPath); From 37fc26255a8b79dd2668b22bbc3ab663cbf67793 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Sun, 25 Oct 2026 13:15:27 +0100 Subject: [PATCH 4/6] Remove PROF-16305 quarantine from reference-chain slow tests All four reference-chain slow e2e tests leave quarantine: the pr-889 regime plus the W5 test-side pacing fixes and the PROF-16316 improveChain invalidation fix (PR #898) cleared the failures. The gc-root musl-aarch64 line is dropped after a 10/10 clean musl-aarch64 JDK11 container validation of the full stack. --- ddprof-test/quarantine.txt | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt index ee0317adbe..b8ca887202 100644 --- a/ddprof-test/quarantine.txt +++ b/ddprof-test/quarantine.txt @@ -73,16 +73,10 @@ com.datadoghq.profiler.nativesocket.NativeSocketSendRecvSeparateTest.* | PROF-16 com.datadoghq.profiler.nativesocket.NativeSocketStackTraceTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | @TestTemplate invocations (cstack=vm/vmx/dwarf/fp) have no method name in the XML; same root cause as the rest of the package com.datadoghq.profiler.cpu.TlsPrimingTest.* | PROF-16281 | 2026-10-20 | 2027-01-18 | el7-8-release-amd64 | samples_dropped_thread_local exceeds this test's sample count across all RetryingTest attempts (79/195, 50/645, 272/137); counter is shared with every SIGNAL_HANDLER_GUARD_OR_DROP path, so co-located runner load can trip the max(10, samples/20) bound -# PROF-16305 -- the reference-chain slow e2e family is intermittently red in -# the nightly sanitized run 39893765385. The dominant failure never observes a -# CachedPayload chain[0] (only HashMap$Node grace chains): 20 of ~32 eligible -# HotSpot 11+ debug/asan slow jobs. Three sibling tests fail the same run -- -# the two loose child-JVM variants report an out-of-pool correlation tag, and -# the gc-root variant never observes a ChainLink chain[0] on musl-aarch64 -# JDK 11. Scoped to *-slow so a failure in the regular suite still gates. -# These lines go away with the PROF-16305 fix PR (PR #800) once the tests -# are made reliable again. -com.datadoghq.profiler.referencechains.ReferenceChainTrackingTest.shouldReconstructReferrerChainThroughUnboundedCacheLeak | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Never observes a CachedPayload chain[0] (only HashMap$Node chains); failed 20 of ~32 eligible HotSpot 11+ debug/asan slow jobs in nightly run 39893765385 -com.datadoghq.profiler.referencechains.ReferenceChainTrackingTest.shouldReconstructReferrerChainToGcRoot | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Never observes a ChainLink chain[0]; 1 failing job (musl-11-debug-aarch64-slow) in nightly run 39893765385 -com.datadoghq.profiler.referencechains.ThreadLocalLeakReferenceChainTest.shouldCorrelateThreadLocalHeldLeakChain | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Child reports [tl-correlation-tag-out-of-pool]; 3 failing jobs in nightly run 39893765385 -com.datadoghq.profiler.referencechains.LeakTagCorrelationReferenceChainTest.shouldCorrelateLeakChainTargetTagWithLiveObjectLeakTag | PROF-16305 | 2026-10-23 | 2027-01-21 | *-slow | Child reports [correlation-tag-out-of-pool]; 1 failing job in nightly run 39893765385 +# PROF-16305 -- the reference-chain slow e2e family was intermittently red in +# the nightly sanitized run 39893765385. Fixed by the pr-889 regime (cache-leak +# rate 0.80 -> ~0.11 on 21.0.3-tem), the W5 test-side pacing fixes, and the +# PROF-16316 improveChain cache-invalidation fix (PR #898). All four quarantined +# tests were removed from quarantine; the gc-root musl-aarch64 line was dropped +# after the T14 musl-aarch64 JDK11 container check passed. Scoped to *-slow so +# a failure in the regular suite still gates. From e2377293822bd43dedcfb88417682191f2de681b Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Mon, 2 Nov 2026 18:34:01 +0100 Subject: [PATCH 5/6] sphinx: address review feedback on PR #899 --- AGENTS.md | 2 +- .../src/main/cpp/referenceChainAnchors.cpp | 141 ++++++++++++++- .../src/main/cpp/referenceChainTraversal.cpp | 12 +- ddprof-lib/src/main/cpp/referenceChains.cpp | 6 + ddprof-lib/src/main/cpp/referenceChains.h | 39 +++- .../test/cpp/referenceChainsAnchorTests.inc | 171 ++++++++++++++++++ .../src/test/cpp/referenceChainsBfsTests.inc | 7 + .../test/cpp/referenceChainsTestAccessors.h | 32 +++- doc/README.md | 2 +- doc/ReferenceChainsCollectionSummary.md | 2 +- .../LiveHeapReferenceChainsImplementation.md | 35 +++- 11 files changed, 426 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e06254030c..d614ea7218 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -150,7 +150,7 @@ before working on a subsystem. | TLS context (thread-local tracing context, distributed tracing) | [doc/architecture/TLSContext.md](doc/architecture/TLSContext.md) | | Profiling engines (CPU, wall-clock, allocation, native memory, liveness) | [doc/architecture/ProfilingEngines.md](doc/architecture/ProfilingEngines.md) | | JFR integration (FlightRecorder, metadata, constant pools, buffers) | [doc/architecture/JfrIntegration.md](doc/architecture/JfrIntegration.md) | -| Live-heap reference chains (implementation, signal scheduling, design) | [doc/architecture/LiveHeapReferenceChainsImplementation.md](doc/architecture/LiveHeapReferenceChainsImplementation.md), [doc/architecture/ReferenceChainsSignalsExplained.md](doc/architecture/ReferenceChainsSignalsExplained.md), [doc/ReferenceChainsDesign.md](doc/ReferenceChainsDesign.md), [doc/ReferenceChainsCollectionSummary.md](doc/ReferenceChainsCollectionSummary.md) | +| Live-heap reference chains (implementation, anchor rotation over durable roots and capped local collection holders, signal scheduling, design) | [doc/architecture/LiveHeapReferenceChainsImplementation.md](doc/architecture/LiveHeapReferenceChainsImplementation.md), [doc/architecture/ReferenceChainsSignalsExplained.md](doc/architecture/ReferenceChainsSignalsExplained.md), [doc/ReferenceChainsDesign.md](doc/ReferenceChainsDesign.md), [doc/ReferenceChainsCollectionSummary.md](doc/ReferenceChainsCollectionSummary.md) | | Native memory profiling (malloc/free interception, sampling) | [doc/architecture/NativeMemoryProfiling.md](doc/architecture/NativeMemoryProfiling.md) | | Startup sanity checks | [doc/architecture/SanityChecks.md](doc/architecture/SanityChecks.md) | | SIGSEGV-safe library patching | [doc/architecture/SigsegvPatching.md](doc/architecture/SigsegvPatching.md) | diff --git a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp index 5ceaf81321..87e2243ece 100644 --- a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp @@ -333,9 +333,127 @@ void ReferenceChainTracker::addToStaticAnchorIndex(jlong tag, root_kind != (u8)JVMTI_HEAP_REFERENCE_JNI_GLOBAL) { return; } + // A local holder later attached by a durable root now belongs to the uncapped durable share; + // release its slot in the local cap (it stays indexed - the dedup below is a no-op for it). + _local_anchor_tags.erase(tag); indexAnchorForRotation(tag, own_class_tag); } +void ReferenceChainTracker::noteLocalRootHolder(jlong tag, + jlong own_class_tag) { + if (_static_anchor_index_tags.count(tag) > 0) { + return; // already indexed (durable root or earlier local report) + } + auto shape = _class_shape_cache.find(own_class_tag); + if (shape != _class_shape_cache.end()) { + if (shape->second != (u8)AnchorClassShape::CONTAINER) { + return; // classified non-container: never indexed + } + if (_local_anchor_tags.size() < LOCAL_ANCHOR_INDEX_CAP) { + _local_anchor_tags.insert(tag); + indexAnchorForRotation(tag, own_class_tag); + return; + } + // At the cap: defer to resolvePendingLocalAnchors(), which may compact dead local entries + // (it can take the frontier lock; this callback must not). + } + if (_pending_local_anchors.size() >= PENDING_LOCAL_ANCHOR_CAP) { + return; // re-reported by the next root enumeration + } + if (!_pending_local_anchor_tags.insert(tag).second) { + return; + } + _pending_local_anchors.push_back(PendingLocalAnchor{tag, own_class_tag}); +} + +void ReferenceChainTracker::resolvePendingLocalAnchors() { + if (_pending_local_anchors.empty()) { + return; + } + bool compacted = false; + for (const PendingLocalAnchor &pending : _pending_local_anchors) { + if (_static_anchor_index_tags.count(pending.tag) > 0) { + continue; + } + auto shape = _class_shape_cache.find(pending.class_tag); + if (shape == _class_shape_cache.end() || + shape->second != (u8)AnchorClassShape::CONTAINER) { + continue; // non-container, or still unclassified: dropped (see the buffer's comment) + } + if (_local_anchor_tags.size() >= LOCAL_ANCHOR_INDEX_CAP && !compacted) { + compactLocalAnchors(); + compacted = true; + } + if (_local_anchor_tags.size() >= LOCAL_ANCHOR_INDEX_CAP) { + continue; // still full of live containers: the cap holds + } + _local_anchor_tags.insert(pending.tag); + indexAnchorForRotation(pending.tag, pending.class_tag); + } + // Keep capacity: the buffer refills on every root enumeration. + _pending_local_anchors.clear(); + _pending_local_anchor_tags.clear(); +} + +void ReferenceChainTracker::compactLocalAnchors() { + if (_local_anchor_tags.empty()) { + return; + } + std::unordered_set removed; + _frontier->withSharedLock([&](const FrontierTable *frontier) { + size_t idx_size = _static_anchor_index.size(); + for (size_t i = 0; i < idx_size; i++) { + jlong tag = _static_anchor_index[i]; + if (_local_anchor_tags.count(tag) == 0) { + continue; // durable share: not compacted here + } + jlong own_class_tag = i < _static_anchor_own_class_tags.size() + ? _static_anchor_own_class_tags[i] : 0; + FrontierEntry entry{}; + // Same eligibility as collectStaticFieldAnchorsForRotation(): a released (ABANDONED) entry + // is a dead object and can never be selected again. + if (!frontier->lookupLocked(tag, &entry) || + !isSelectableAnchor(entry, own_class_tag) || + (entry.state != FrontierEntryState::FRONTIER && + entry.state != FrontierEntryState::EXPANDED)) { + removed.insert(tag); + } + } + }); + if (removed.empty()) { + return; + } + // Stable in-place compaction of the two parallel vectors keeps relative order, so the fresh + // queue (a suffix of the index) stays a suffix once the same tags leave it too. + size_t out = 0; + for (size_t i = 0; i < _static_anchor_index.size(); i++) { + jlong tag = _static_anchor_index[i]; + if (removed.count(tag) > 0) { + continue; + } + _static_anchor_index[out] = tag; + if (i < _static_anchor_own_class_tags.size()) { + _static_anchor_own_class_tags[out] = _static_anchor_own_class_tags[i]; + } + out++; + } + _static_anchor_index.resize(out); + if (_static_anchor_own_class_tags.size() > out) { + _static_anchor_own_class_tags.resize(out); + } + for (jlong tag : removed) { + _static_anchor_index_tags.erase(tag); + _local_anchor_tags.erase(tag); + } + _static_anchor_fresh_queue.erase( + std::remove_if(_static_anchor_fresh_queue.begin(), + _static_anchor_fresh_queue.end(), + [&](jlong tag) { return removed.count(tag) > 0; }), + _static_anchor_fresh_queue.end()); + // Positions shifted: the collector clamps out-of-range cursors; in-range ones only lose fairness + // for one lap. +} + bool ReferenceChainTracker::isSelectableAnchor(const FrontierEntry &entry, jlong own_class_tag) const { if (entry.parent_tag == 0 && @@ -491,24 +609,31 @@ void ReferenceChainTracker::reconcileAnchorClassShapes(jvmtiEnv *jvmti, if (jni == nullptr) { return; } - if (_static_anchor_own_class_tags.empty()) { + if (_static_anchor_own_class_tags.empty() && _pending_local_anchors.empty()) { return; } - // Collect up to ANCHOR_SHAPE_RECONCILE_BUDGET distinct class tags that appear in the anchor index - // but are not yet classified. + // Collect up to ANCHOR_SHAPE_RECONCILE_BUDGET distinct class tags that appear in the pending + // local-holder buffer or the anchor index but are not yet classified. Pending holders go first: + // resolvePendingLocalAnchors() drops any still unclassified after this call, while indexed + // durable anchors simply wait for a later pass. std::vector unknown; unknown.reserve(8); std::unordered_set seen; - for (jlong class_tag : _static_anchor_own_class_tags) { + auto consider = [&](jlong class_tag) { if (class_tag == 0 || seen.count(class_tag) > 0 || _class_shape_cache.count(class_tag) > 0) { - continue; + return true; } seen.insert(class_tag); unknown.push_back(class_tag); - if ((int)unknown.size() >= ANCHOR_SHAPE_RECONCILE_BUDGET) { - break; - } + return (int)unknown.size() < ANCHOR_SHAPE_RECONCILE_BUDGET; + }; + bool room = true; + for (size_t i = 0; room && i < _pending_local_anchors.size(); i++) { + room = consider(_pending_local_anchors[i].class_tag); + } + for (size_t i = 0; room && i < _static_anchor_own_class_tags.size(); i++) { + room = consider(_static_anchor_own_class_tags[i]); } if (unknown.empty()) { return; diff --git a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp index e88f3fded1..4fe81cfadc 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -138,15 +138,12 @@ jvmtiIterationControl JNICALL ReferenceChainTracker::heapRootCallback( } // Remember local holders before expansion can reparent them into a deeper // chain. Known collections still need bounded descend walks after that change - // to observe replacement backing storage. Classification runs outside this - // callback; unknown types are indexed but do not qualify for a walk yet. + // to observe replacement backing storage. Only confirmed containers enter the + // anchor index (capped); classification runs outside this callback, so an + // unclassified holder waits in a capped pending buffer instead. if (isTransientRootKind(translated_root_kind) && class_tag != 0 && *tag_ptr > 0 && !isLeakTag(*tag_ptr)) { - auto shape = ctx->tracker->_class_shape_cache.find(class_tag); - if (shape == ctx->tracker->_class_shape_cache.end() || - shape->second == (u8)AnchorClassShape::CONTAINER) { - ctx->tracker->indexAnchorForRotation(*tag_ptr, class_tag); - } + ctx->tracker->noteLocalRootHolder(*tag_ptr, class_tag); } return JVMTI_ITERATION_CONTINUE; } @@ -371,6 +368,7 @@ void ReferenceChainTracker::runPassManualWalk(jvmtiEnv *jvmti, JNIEnv *jni, // collectStaticFieldAnchorsForRotation()/ walkStaticFieldAnchors()'s own comments) - not pushed // onto the priority lane, so they are independent of the queue tiers above. reconcileAnchorClassShapes(jvmti, jni); + resolvePendingLocalAnchors(); std::vector static_anchor_tags = collectStaticFieldAnchorsForRotation(STATIC_ANCHOR_ROTATION_BUDGET); std::vector static_anchor_fifo_drained; diff --git a/ddprof-lib/src/main/cpp/referenceChains.cpp b/ddprof-lib/src/main/cpp/referenceChains.cpp index 211686301e..c022d30cc5 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.cpp +++ b/ddprof-lib/src/main/cpp/referenceChains.cpp @@ -894,6 +894,9 @@ void ReferenceChainTracker::restartSearch() { _static_anchor_index.clear(); _static_anchor_own_class_tags.clear(); _static_anchor_index_tags.clear(); + _local_anchor_tags.clear(); + _pending_local_anchors.clear(); + _pending_local_anchor_tags.clear(); _anchor_container_cursor = 0; _anchor_other_cursor = 0; // Fresh lane: nothing admits before the search does, so nothing can have a pending first look @@ -979,6 +982,9 @@ void ReferenceChainTracker::resetSearchStateForTest(jvmtiEnv *jvmti, _static_anchor_index.clear(); _static_anchor_own_class_tags.clear(); _static_anchor_index_tags.clear(); + _local_anchor_tags.clear(); + _pending_local_anchors.clear(); + _pending_local_anchor_tags.clear(); _anchor_container_cursor = 0; _anchor_other_cursor = 0; _static_anchor_fresh_queue.clear(); diff --git a/ddprof-lib/src/main/cpp/referenceChains.h b/ddprof-lib/src/main/cpp/referenceChains.h index 9846b8ab6b..2d3cfeeac5 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -455,9 +455,11 @@ class ReferenceChainTracker { std::unordered_map _static_anchor_fifo_klass_counts; static constexpr u32 STATIC_ANCHOR_ATRISK_PER_KLASS_CAP = 64; - // Index of durable roots and observed local holders. Known collections remain - // eligible after reparenting, so replacing their storage is still observed. - // Non-collection local holders are excluded after classification. + // Index of durable roots and CONFIRMED-container local holders. Known collections remain + // eligible after reparenting, so replacing their storage is still observed. A stack/JNI-local + // root is indexed only once its class is classified as a collection/map (see + // noteLocalRootHolder()/resolvePendingLocalAnchors()); non-collection local holders are never + // indexed, and the local share of the index is capped at LOCAL_ANCHOR_INDEX_CAP. // collectStaticFieldAnchorsForRotation() iterates these anchors rather than // scanning the full frontier table. std::vector _static_anchor_index; @@ -469,6 +471,26 @@ class ReferenceChainTracker { // - see addToStaticAnchorIndex()'s own comment). std::unordered_set _static_anchor_index_tags; + // Subset of _static_anchor_index_tags that were indexed as stack/JNI-local holders (not as + // durable roots). Its size is the local share of the index, capped at LOCAL_ANCHOR_INDEX_CAP so + // per-pass anchor selection stays O(durable anchors + cap) no matter how many locals the + // application holds. A tag later indexed as a durable root leaves this set (uncapped share). + std::unordered_set _local_anchor_tags; + static constexpr size_t LOCAL_ANCHOR_INDEX_CAP = 4096; + + // Local-root holders whose class was not yet classified when root enumeration reported them + // (classification needs JNI, which the heap-root callback cannot use). Drained - indexed if + // confirmed containers, dropped otherwise - by resolvePendingLocalAnchors() after every + // reconcileAnchorClassShapes(); an entry whose class is still unclassified is dropped too and + // re-reported by the next root enumeration. Capped so a burst of locals cannot grow it. + struct PendingLocalAnchor { + jlong tag; + jlong class_tag; + }; + std::vector _pending_local_anchors; + std::unordered_set _pending_local_anchor_tags; + static constexpr size_t PENDING_LOCAL_ANCHOR_CAP = 1024; + // Shape of a class as an anchor candidate: does the class implement java/util/Collection or // java/util/Map (directly or via superclasses/ interfaces)? enum class AnchorClassShape : u8 { UNKNOWN = 0, CONTAINER = 1, NON_CONTAINER = 2 }; @@ -946,6 +968,17 @@ class ReferenceChainTracker { void addToStaticAnchorIndex(jlong tag, jlong own_class_tag, u8 root_kind); // Shared deduplicated index insertion for durable roots and local holders. void indexAnchorForRotation(jlong tag, jlong own_class_tag); + // Heap-root callback hook for a stack/JNI-local holder: indexes a confirmed container (under + // LOCAL_ANCHOR_INDEX_CAP), ignores a classified non-container, and parks an unclassified holder + // (or one met at the cap) in the capped _pending_local_anchors buffer. No JNI, no frontier lock. + void noteLocalRootHolder(jlong tag, jlong own_class_tag); + // BFS thread, after reconcileAnchorClassShapes(): indexes pending holders confirmed as + // containers, drops the rest, and empties the pending buffer. + void resolvePendingLocalAnchors(); + // Removes local-holder index entries that are no longer selectable (dead or demoted), keeping + // _static_anchor_fresh_queue a suffix of the index. Run at most once per + // resolvePendingLocalAnchors() call, only when the local cap is reached. + void compactLocalAnchors(); // True iff `klass` implements java/util/Collection or java/util/Map, directly or transitively // (superclass chain + interfaces of every visited class, depth-bounded, visited set to survive diff --git a/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc b/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc index d39be8edff..6c4e4e525d 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc @@ -809,3 +809,174 @@ TEST_F(ReferenceChainsBfsTest, AtRiskFifoPerClassQuotaKeepsFloodOut) { // offset, the superclass-chain order, the interface-referrer branch - and degrade to the edge KIND // on any undecodable hop, never a fabricated name (the fail-safe contract: a wrong numbering on an // unverified JVM degrades, it does not lie). + +// Local-root holders enter the anchor index only once confirmed as containers: a classified +// non-container is ignored outright, and an unclassified holder waits in the pending buffer and is +// dropped by the resolve step if its class is still unknown or turns out to be a non-container. +TEST_F(ReferenceChainsBfsTest, LocalRootAnchorIndexHoldsOnlyConfirmedContainers) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + + const jlong kNonContainer = -201, kLater = -202, kNever = -203, kContainer = -204; + ReferenceChainsTestAccessor::primeClassShapeForTest(kNonContainer, false); + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + + // Classified non-container: neither indexed nor parked. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(301, kNonContainer); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + // Classified container: indexed directly and counted against the local cap. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(302, kContainer); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::localAnchorCountForTest()); + + // Unclassified holders are parked (deduplicated), not indexed. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(303, kLater); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(303, kLater); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(304, kNever); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + // kLater classifies as a non-container, kNever stays unknown: both are dropped, and the + // pending buffer is emptied every resolve. + ReferenceChainsTestAccessor::primeClassShapeForTest(kLater, false); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + tracker->stop(); +} + +// The local share of the anchor index is capped, and the pending buffer is capped too, so the +// per-pass anchor scan cannot grow with the number of application locals. At the cap, the resolve +// step compacts away local entries whose frontier entry is gone before admitting new ones. +TEST_F(ReferenceChainsBfsTest, LocalRootAnchorIndexIsCappedAndCompactsDeadEntries) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + FrontierTable *frontier = tracker->frontierTable(); + + const size_t cap = ReferenceChainsTestAccessor::kLocalAnchorIndexCap; + const size_t pending_cap = ReferenceChainsTestAccessor::kPendingLocalAnchorCap; + const jlong kContainer = -301; + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + + // A durable anchor is outside the local share and must survive compaction. + const jlong durable = 50; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, durable, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STATIC_FIELD)); + ReferenceChainsTestAccessor::addToStaticAnchorIndexForTest( + durable, -302, JVMTI_HEAP_REFERENCE_STATIC_FIELD); + + // Fill the local share with holders whose frontier entries are then released (dead objects: + // the GC cleanup marks them ABANDONED). + const jlong dead_base = 100; + for (size_t i = 0; i < cap; i++) { + jlong tag = dead_base + (jlong)i; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, tag, 0, 0, FrontierEntryState::EXPANDED, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(tag, kContainer); + frontier->clear(tag); + } + ASSERT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + ASSERT_EQ(cap + 1, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + + // Containers met at the cap are parked, and the pending buffer stops at its own cap. + // Above every inserted tag, so un-inserted parked tags have no frontier entry at all. + const jlong live_base = dead_base + (jlong)cap + 1000; + for (size_t i = 0; i < pending_cap + 10; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(live_base + (jlong)i, + kContainer); + } + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + // Only one parked holder is live; the resolve step compacts the dead local entries and + // admits it. Dead parked holders are indexed too (liveness is checked at selection), but + // all of them fit once the dead share is gone. + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, live_base, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(pending_cap + 1, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_LE(ReferenceChainsTestAccessor::freshAnchorQueueSizeForTest(), + ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + + std::vector selected = + ReferenceChainsTestAccessor::collectStaticFieldAnchorsForRotationForTest(4); + EXPECT_NE(std::find(selected.begin(), selected.end(), durable), selected.end()) + << "durable anchor lost by local-share compaction"; + EXPECT_NE(std::find(selected.begin(), selected.end(), live_base), selected.end()) + << "live local container not admitted after compaction"; + + tracker->stop(); +} + +// When every local entry is live the cap holds: no compaction can free a slot, new holders are +// not indexed, and the pending buffer is still emptied. +TEST_F(ReferenceChainsBfsTest, LocalRootAnchorCapHoldsWhenAllEntriesLive) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + FrontierTable *frontier = tracker->frontierTable(); + + const size_t cap = ReferenceChainsTestAccessor::kLocalAnchorIndexCap; + const jlong kContainer = -401; + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + for (size_t i = 0; i < cap; i++) { + jlong tag = 1000 + (jlong)i; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, tag, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(tag, kContainer); + } + const jlong extra = 1000 + (jlong)cap; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, extra, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(extra, kContainer); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + tracker->stop(); +} + +// A local holder later attached by a durable root moves to the uncapped durable share, freeing its +// local-cap slot without duplicating the index entry. +TEST_F(ReferenceChainsBfsTest, DurableRootUpgradeReleasesLocalAnchorSlot) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + + const jlong kContainer = -501; + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(777, kContainer); + ASSERT_EQ(1u, ReferenceChainsTestAccessor::localAnchorCountForTest()); + ReferenceChainsTestAccessor::addToStaticAnchorIndexForTest( + 777, kContainer, JVMTI_HEAP_REFERENCE_STATIC_FIELD); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + + // restartSearch() resets the local share and the pending buffer with the rest of the index. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(778, -502); + ASSERT_EQ(1u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + ReferenceChainsTestAccessor::restartSearchForTest(); + EXPECT_TRUE(ReferenceChainsTestAccessor::anchorIndexIsEmptyForTest()); + + tracker->stop(); +} diff --git a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc index 088a9d4b71..6a5a5bcff5 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc @@ -352,7 +352,14 @@ TEST_F(ReferenceChainsBfsTest, RootContainerRemainsWalkableAfterReparentingAndMu // Classification runs outside the heap callback. Unknown transient roots // must not get a descend walk before being confirmed as containers. EXPECT_TRUE(ReferenceChainsTestAccessor::collectStaticFieldAnchorsForRotationForTest(4).empty()); + // Neither holder is indexed yet: the unclassified one waits in the pending buffer, the + // classified non-container is never indexed. + EXPECT_EQ(0u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + // Stand-in for reconcileAnchorClassShapes() + the resolve step that follows it in a pass. ReferenceChainsTestAccessor::primeClassShapeForTest(container_class_tag, true); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); // A deeper retaining path replaces the initial local-root attribution. // The container still needs repeated walks when its backing storage changes. ASSERT_TRUE(tracker->frontierTable()->improveChain( diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index 14e56d206b..df546b590d 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -94,6 +94,9 @@ class ReferenceChainsTestAccessor { t->_static_anchor_index.clear(); t->_static_anchor_own_class_tags.clear(); t->_static_anchor_index_tags.clear(); + t->_local_anchor_tags.clear(); + t->_pending_local_anchors.clear(); + t->_pending_local_anchor_tags.clear(); t->_anchor_container_cursor = 0; t->_anchor_other_cursor = 0; t->_class_shape_cache.clear(); @@ -468,9 +471,36 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker *t = ReferenceChainTracker::instance(); return t->_static_anchor_index.empty() && t->_static_anchor_own_class_tags.empty() && - t->_static_anchor_index_tags.empty(); + t->_static_anchor_index_tags.empty() && + t->_local_anchor_tags.empty() && + t->_pending_local_anchors.empty() && + t->_pending_local_anchor_tags.empty(); } + // Local-root anchor seams (see noteLocalRootHolder()/resolvePendingLocalAnchors()). + static void noteLocalRootHolderForTest(jlong tag, jlong class_tag) { + ReferenceChainTracker::instance()->noteLocalRootHolder(tag, class_tag); + } + static void resolvePendingLocalAnchorsForTest() { + ReferenceChainTracker::instance()->resolvePendingLocalAnchors(); + } + static size_t anchorIndexSizeForTest() { + return ReferenceChainTracker::instance()->_static_anchor_index.size(); + } + static size_t localAnchorCountForTest() { + return ReferenceChainTracker::instance()->_local_anchor_tags.size(); + } + static size_t pendingLocalAnchorCountForTest() { + return ReferenceChainTracker::instance()->_pending_local_anchors.size(); + } + static size_t freshAnchorQueueSizeForTest() { + return ReferenceChainTracker::instance()->_static_anchor_fresh_queue.size(); + } + static constexpr size_t kLocalAnchorIndexCap = + ReferenceChainTracker::LOCAL_ANCHOR_INDEX_CAP; + static constexpr size_t kPendingLocalAnchorCap = + ReferenceChainTracker::PENDING_LOCAL_ANCHOR_CAP; + static jlong discoveredTagForTest(int slot, int idx) { return ReferenceChainTracker::instance() ->_candidate_discovered_tags[slot][idx]; diff --git a/doc/README.md b/doc/README.md index abae5579e9..47bc6428d0 100644 --- a/doc/README.md +++ b/doc/README.md @@ -20,7 +20,7 @@ All documentation files use **PascalCase** naming (e.g., `BuildSystemGuide.md`). ### Architecture - [CallTraceStorage](architecture/CallTraceStorage.md) - Triple-buffer architecture for call traces - [JfrIntegration](architecture/JfrIntegration.md) - Native JFR writer: recordings, buffers, metadata, constant pools -- [LiveHeapReferenceChainsImplementation](architecture/LiveHeapReferenceChainsImplementation.md) - Current reference-chain implementation +- [LiveHeapReferenceChainsImplementation](architecture/LiveHeapReferenceChainsImplementation.md) - Current reference-chain implementation, including the anchor index (durable roots plus capped local collection holders) - [NativeMemoryProfiling](architecture/NativeMemoryProfiling.md) - malloc interception and allocation sampling - [ProfilingEngines](architecture/ProfilingEngines.md) - CPU, wall-clock, allocation, native-memory and liveness engines - [RefCountGuard](architecture/RefCountGuard.md) - Lock-free RAII reference-counting primitive used to drain readers before resource reclamation diff --git a/doc/ReferenceChainsCollectionSummary.md b/doc/ReferenceChainsCollectionSummary.md index e2351f29ff..09fb6867ca 100644 --- a/doc/ReferenceChainsCollectionSummary.md +++ b/doc/ReferenceChainsCollectionSummary.md @@ -65,7 +65,7 @@ The tracker addresses this case with bounded rotation: - Leak-accumulation rotation selects parent objects that receive instances of watched classes. - Stale-entry rotation revisits older expanded entries. - Root-kind rotation revisits entries with transient root attribution. -- Static-anchor rotation descends from selected objects held by static fields. +- Anchor rotation descends from selected objects held by static fields or JNI globals, and from a capped set of confirmed collection/map holders first seen as stack or JNI locals. Rotation improves coverage. It does not guarantee that the tracker observes every mutation. diff --git a/doc/architecture/LiveHeapReferenceChainsImplementation.md b/doc/architecture/LiveHeapReferenceChainsImplementation.md index bf382ea2a3..48ec71681b 100644 --- a/doc/architecture/LiveHeapReferenceChainsImplementation.md +++ b/doc/architecture/LiveHeapReferenceChainsImplementation.md @@ -135,7 +135,7 @@ An object can change after its first expansion. For example, a long-lived collec | Leak accumulation | 16 | Select parents that accumulate instances of watched classes. | | Stale expanded entries | 256 | Provide eventual low-priority rechecks across the frontier table. | | Transient root kinds | 16 | Improve root attribution when later observations provide a more durable root kind. | -| Static anchors | 32 selected, plus 16 FIFO entries | Descend from selected objects held by static fields. | +| Anchors | 32 selected, plus 16 FIFO entries | Descend from indexed anchors: objects held by static fields or JNI globals, and confirmed collection/map holders first seen as stack or JNI locals. See [Anchor index](#anchor-index). | Leak accumulation has two levels. The first groups admissions by leaf class and parent class. The second ranks parent object tags within the selected group. @@ -143,6 +143,39 @@ When a class first becomes watched, the tracker scans existing expanded entries Rotation improves coverage but remains bounded and best effort. +### Anchor index + +`collectStaticFieldAnchorsForRotation()` selects descend-walk anchors from an index +(`_static_anchor_index`), not from the full frontier table. The index has two shares: + +- **Durable share.** Root-attached `STATIC_FIELD` and `JNI_GLOBAL` entries, added by + `addToStaticAnchorIndex()`. Such an entry is selectable while it stays root-attached with a + durable root kind. Once it is classified as a collection or map, it stays selectable after + reparenting. A reparented holder of any other class gets one walk from the at-risk FIFO + (`pushAtRiskStaticAnchor()`). +- **Local share.** Holders that root enumeration reports as `STACK_LOCAL` or `JNI_LOCAL` roots. + `heapRootCallback()` passes them to `noteLocalRootHolder()`. A holder is indexed only after + `reconcileAnchorClassShapes()` classifies its class as implementing `java.util.Collection` or + `java.util.Map`. A confirmed collection remains selectable after expansion reparents it into a + deeper chain, so the tracker still observes replacement backing storage such as a resized + `HashMap.table`. Local holders of any other class are never indexed. + +Classification needs JNI, which the heap-root callback cannot use. A holder whose class is not +yet classified waits in `_pending_local_anchors`, capped at 1024 entries. +`resolvePendingLocalAnchors()` runs after each `reconcileAnchorClassShapes()`. It indexes the +confirmed collections and drops everything else, including holders whose class is still +unclassified. The next root enumeration reports a dropped holder again. Reconciliation +classifies pending holder classes before unclassified durable-anchor classes, within the +128-classes-per-pass budget. + +The local share is capped at 4096 entries (`LOCAL_ANCHOR_INDEX_CAP`). This keeps the per-pass +index scan proportional to the durable share plus a constant, regardless of how many locals the +application holds. When the cap is reached, `compactLocalAnchors()` removes local entries that +are no longer selectable, such as entries whose object died, at most once per resolve call. If the share is still +full of live collections, new local holders are not indexed until entries die or the search +restarts. A local holder that a durable root later attaches moves to the durable share and stops +counting against the cap. `restartSearch()` clears both shares and the pending buffer. + ## Search termination The implementation evaluates terminal conditions after each pass, in this priority order From 19eeb3f8db11d4d46a2824ca44c5455564451cff Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 3 Nov 2026 11:52:28 +0100 Subject: [PATCH 6/6] sphinx: address review feedback on PR #899 --- .../src/main/cpp/referenceChainAnchors.cpp | 94 ++++-- ddprof-lib/src/main/cpp/referenceChains.cpp | 4 + ddprof-lib/src/main/cpp/referenceChains.h | 39 ++- .../test/cpp/referenceChainsAnchorTests.inc | 280 ++++++++++++++++++ .../test/cpp/referenceChainsTestAccessors.h | 30 +- .../LiveHeapReferenceChainsImplementation.md | 15 +- 6 files changed, 423 insertions(+), 39 deletions(-) diff --git a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp index 87e2243ece..5fa674e4f1 100644 --- a/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainAnchors.cpp @@ -345,20 +345,50 @@ void ReferenceChainTracker::noteLocalRootHolder(jlong tag, return; // already indexed (durable root or earlier local report) } auto shape = _class_shape_cache.find(own_class_tag); + bool confirmed_container = false; if (shape != _class_shape_cache.end()) { if (shape->second != (u8)AnchorClassShape::CONTAINER) { return; // classified non-container: never indexed } if (_local_anchor_tags.size() < LOCAL_ANCHOR_INDEX_CAP) { - _local_anchor_tags.insert(tag); + _local_anchor_tags.emplace(tag, own_class_tag); indexAnchorForRotation(tag, own_class_tag); return; } // At the cap: defer to resolvePendingLocalAnchors(), which may compact dead local entries // (it can take the frontier lock; this callback must not). + confirmed_container = true; } if (_pending_local_anchors.size() >= PENDING_LOCAL_ANCHOR_CAP) { - return; // re-reported by the next root enumeration + if (!confirmed_container || _pending_local_anchor_tags.count(tag) > 0) { + return; // re-reported by the next root enumeration + } + // A confirmed container is walkable as soon as a local slot frees, so it outranks an entry not + // yet confirmed as a container: evict the oldest such entry (re-reported by the next root + // enumeration). Forward-only cursor: while the buffer is full nothing is appended, the only + // overwrite is at the cursor with a container, and skipped slots are containers, so every slot + // below the cursor stays a confirmed container and the scan totals <= PENDING_LOCAL_ANCHOR_CAP + // per enumeration. + if (_pending_local_evict_cursor > _pending_local_anchors.size()) { + _pending_local_evict_cursor = 0; // defensive; cannot happen while the buffer is full + } + while (_pending_local_evict_cursor < _pending_local_anchors.size()) { + auto s = _class_shape_cache.find( + _pending_local_anchors[_pending_local_evict_cursor].class_tag); + if (s == _class_shape_cache.end() || + s->second != (u8)AnchorClassShape::CONTAINER) { + break; + } + _pending_local_evict_cursor++; + } + if (_pending_local_evict_cursor >= _pending_local_anchors.size()) { + return; // only confirmed containers parked: dropped, re-reported by the next enumeration + } + PendingLocalAnchor &victim = _pending_local_anchors[_pending_local_evict_cursor++]; + _pending_local_anchor_tags.erase(victim.tag); + victim = PendingLocalAnchor{tag, own_class_tag}; + _pending_local_anchor_tags.insert(tag); + return; } if (!_pending_local_anchor_tags.insert(tag).second) { return; @@ -381,47 +411,50 @@ void ReferenceChainTracker::resolvePendingLocalAnchors() { continue; // non-container, or still unclassified: dropped (see the buffer's comment) } if (_local_anchor_tags.size() >= LOCAL_ANCHOR_INDEX_CAP && !compacted) { - compactLocalAnchors(); + // At most one compaction attempt per call. After a compaction that removed nothing (local + // share full of live containers), skip the next LOCAL_ANCHOR_COMPACT_BACKOFF attempts so a + // saturated-but-live share does not pay the locked O(cap) scan every pass. compacted = true; + if (_local_compact_backoff > 0) { + _local_compact_backoff--; + } else { + _local_compact_backoff = + compactLocalAnchors() == 0 ? LOCAL_ANCHOR_COMPACT_BACKOFF : 0; + } } if (_local_anchor_tags.size() >= LOCAL_ANCHOR_INDEX_CAP) { - continue; // still full of live containers: the cap holds + continue; // still full (live containers, or compaction backed off): the cap holds } - _local_anchor_tags.insert(pending.tag); + _local_anchor_tags.emplace(pending.tag, pending.class_tag); indexAnchorForRotation(pending.tag, pending.class_tag); } // Keep capacity: the buffer refills on every root enumeration. _pending_local_anchors.clear(); _pending_local_anchor_tags.clear(); + _pending_local_evict_cursor = 0; } -void ReferenceChainTracker::compactLocalAnchors() { +size_t ReferenceChainTracker::compactLocalAnchors() { if (_local_anchor_tags.empty()) { - return; + return 0; } std::unordered_set removed; _frontier->withSharedLock([&](const FrontierTable *frontier) { - size_t idx_size = _static_anchor_index.size(); - for (size_t i = 0; i < idx_size; i++) { - jlong tag = _static_anchor_index[i]; - if (_local_anchor_tags.count(tag) == 0) { - continue; // durable share: not compacted here - } - jlong own_class_tag = i < _static_anchor_own_class_tags.size() - ? _static_anchor_own_class_tags[i] : 0; + // Only the local share (O(cap)): the durable share is never compacted here. + for (const auto &local : _local_anchor_tags) { FrontierEntry entry{}; // Same eligibility as collectStaticFieldAnchorsForRotation(): a released (ABANDONED) entry // is a dead object and can never be selected again. - if (!frontier->lookupLocked(tag, &entry) || - !isSelectableAnchor(entry, own_class_tag) || + if (!frontier->lookupLocked(local.first, &entry) || + !isSelectableAnchor(entry, local.second) || (entry.state != FrontierEntryState::FRONTIER && entry.state != FrontierEntryState::EXPANDED)) { - removed.insert(tag); + removed.insert(local.first); } } }); if (removed.empty()) { - return; + return 0; } // Stable in-place compaction of the two parallel vectors keeps relative order, so the fresh // queue (a suffix of the index) stays a suffix once the same tags leave it too. @@ -452,6 +485,7 @@ void ReferenceChainTracker::compactLocalAnchors() { _static_anchor_fresh_queue.end()); // Positions shifted: the collector clamps out-of-range cursors; in-range ones only lose fairness // for one lap. + return removed.size(); } bool ReferenceChainTracker::isSelectableAnchor(const FrontierEntry &entry, @@ -604,14 +638,8 @@ bool ReferenceChainTracker::classImplementsContainerOrMap(jvmtiEnv *jvmti, return found; } -void ReferenceChainTracker::reconcileAnchorClassShapes(jvmtiEnv *jvmti, - JNIEnv *jni) { - if (jni == nullptr) { - return; - } - if (_static_anchor_own_class_tags.empty() && _pending_local_anchors.empty()) { - return; - } +std::vector +ReferenceChainTracker::collectUnclassifiedAnchorClassTags() const { // Collect up to ANCHOR_SHAPE_RECONCILE_BUDGET distinct class tags that appear in the pending // local-holder buffer or the anchor index but are not yet classified. Pending holders go first: // resolvePendingLocalAnchors() drops any still unclassified after this call, while indexed @@ -635,6 +663,18 @@ void ReferenceChainTracker::reconcileAnchorClassShapes(jvmtiEnv *jvmti, for (size_t i = 0; room && i < _static_anchor_own_class_tags.size(); i++) { room = consider(_static_anchor_own_class_tags[i]); } + return unknown; +} + +void ReferenceChainTracker::reconcileAnchorClassShapes(jvmtiEnv *jvmti, + JNIEnv *jni) { + if (jni == nullptr) { + return; + } + if (_static_anchor_own_class_tags.empty() && _pending_local_anchors.empty()) { + return; + } + std::vector unknown = collectUnclassifiedAnchorClassTags(); if (unknown.empty()) { return; } diff --git a/ddprof-lib/src/main/cpp/referenceChains.cpp b/ddprof-lib/src/main/cpp/referenceChains.cpp index c022d30cc5..4a57c06212 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.cpp +++ b/ddprof-lib/src/main/cpp/referenceChains.cpp @@ -897,6 +897,8 @@ void ReferenceChainTracker::restartSearch() { _local_anchor_tags.clear(); _pending_local_anchors.clear(); _pending_local_anchor_tags.clear(); + _local_compact_backoff = 0; + _pending_local_evict_cursor = 0; _anchor_container_cursor = 0; _anchor_other_cursor = 0; // Fresh lane: nothing admits before the search does, so nothing can have a pending first look @@ -985,6 +987,8 @@ void ReferenceChainTracker::resetSearchStateForTest(jvmtiEnv *jvmti, _local_anchor_tags.clear(); _pending_local_anchors.clear(); _pending_local_anchor_tags.clear(); + _local_compact_backoff = 0; + _pending_local_evict_cursor = 0; _anchor_container_cursor = 0; _anchor_other_cursor = 0; _static_anchor_fresh_queue.clear(); diff --git a/ddprof-lib/src/main/cpp/referenceChains.h b/ddprof-lib/src/main/cpp/referenceChains.h index 2d3cfeeac5..f6892cc880 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -474,15 +474,31 @@ class ReferenceChainTracker { // Subset of _static_anchor_index_tags that were indexed as stack/JNI-local holders (not as // durable roots). Its size is the local share of the index, capped at LOCAL_ANCHOR_INDEX_CAP so // per-pass anchor selection stays O(durable anchors + cap) no matter how many locals the - // application holds. A tag later indexed as a durable root leaves this set (uncapped share). - std::unordered_set _local_anchor_tags; + // application holds. A tag later indexed as a durable root leaves this map (uncapped share). + // Maps each local holder tag to its own runtime class tag at admission time, so + // compactLocalAnchors() can evaluate isSelectableAnchor() by iterating only the local share + // (O(cap)) instead of scanning the whole, uncapped index. + std::unordered_map _local_anchor_tags; static constexpr size_t LOCAL_ANCHOR_INDEX_CAP = 4096; + // compactLocalAnchors() attempts to skip after one that removed nothing (local share full of + // live containers): bounds the steady-state locked O(cap) scan to one in every + // LOCAL_ANCHOR_COMPACT_BACKOFF + 1 at-cap resolve passes. While backing off, a container met at + // the cap is dropped from the pending buffer and re-reported by the next root enumeration; dead + // locals hold their slots for at most this many extra at-cap passes. + static constexpr int LOCAL_ANCHOR_COMPACT_BACKOFF = 8; + // At-cap compaction attempts still to skip. Set by a no-progress compaction, decremented by each + // skipped attempt, zeroed by a productive one; untouched while the local share is under the cap. + int _local_compact_backoff = 0; // Local-root holders whose class was not yet classified when root enumeration reported them // (classification needs JNI, which the heap-root callback cannot use). Drained - indexed if // confirmed containers, dropped otherwise - by resolvePendingLocalAnchors() after every // reconcileAnchorClassShapes(); an entry whose class is still unclassified is dropped too and - // re-reported by the next root enumeration. Capped so a burst of locals cannot grow it. + // re-reported by the next root enumeration. Capped so a burst of locals cannot grow it. When the + // buffer is full, a confirmed container met at the local cap evicts the oldest entry not + // confirmed as a container (that entry is re-reported by the next root enumeration); an + // unclassified holder met at a full buffer, or a container met at a buffer holding only + // confirmed containers, is dropped and re-reported. struct PendingLocalAnchor { jlong tag; jlong class_tag; @@ -490,6 +506,9 @@ class ReferenceChainTracker { std::vector _pending_local_anchors; std::unordered_set _pending_local_anchor_tags; static constexpr size_t PENDING_LOCAL_ANCHOR_CAP = 1024; + // noteLocalRootHolder() eviction cursor (forward-only): every _pending_local_anchors slot below it + // holds a confirmed container. Reset together with the buffer. + size_t _pending_local_evict_cursor = 0; // Shape of a class as an anchor candidate: does the class implement java/util/Collection or // java/util/Map (directly or via superclasses/ interfaces)? @@ -970,15 +989,17 @@ class ReferenceChainTracker { void indexAnchorForRotation(jlong tag, jlong own_class_tag); // Heap-root callback hook for a stack/JNI-local holder: indexes a confirmed container (under // LOCAL_ANCHOR_INDEX_CAP), ignores a classified non-container, and parks an unclassified holder - // (or one met at the cap) in the capped _pending_local_anchors buffer. No JNI, no frontier lock. + // (or one met at the cap) in the capped _pending_local_anchors buffer; at a full buffer a + // confirmed container evicts the oldest not-confirmed entry. No JNI, no frontier lock. void noteLocalRootHolder(jlong tag, jlong own_class_tag); // BFS thread, after reconcileAnchorClassShapes(): indexes pending holders confirmed as // containers, drops the rest, and empties the pending buffer. void resolvePendingLocalAnchors(); // Removes local-holder index entries that are no longer selectable (dead or demoted), keeping - // _static_anchor_fresh_queue a suffix of the index. Run at most once per - // resolvePendingLocalAnchors() call, only when the local cap is reached. - void compactLocalAnchors(); + // _static_anchor_fresh_queue a suffix of the index. Scans only the local share. Run at most once + // per resolvePendingLocalAnchors() call, only when the local cap is reached and the backoff + // (_local_compact_backoff) is not active. Returns the number of removed entries. + size_t compactLocalAnchors(); // True iff `klass` implements java/util/Collection or java/util/Map, directly or transitively // (superclass chain + interfaces of every visited class, depth-bounded, visited set to survive @@ -995,6 +1016,10 @@ class ReferenceChainTracker { // pass via one GetObjectsWithTags call (class objects are tagged with their class tags) and // classifies each. void reconcileAnchorClassShapes(jvmtiEnv *jvmti, JNIEnv *jni); + // Pure (no JNI) collection step of reconcileAnchorClassShapes(): up to + // ANCHOR_SHAPE_RECONCILE_BUDGET distinct non-zero class tags absent from _class_shape_cache, + // pending local holders first (buffer order), then indexed anchors (index order). Read-only. + std::vector collectUnclassifiedAnchorClassTags() const; // Pops up to max_count AtRiskAnchor entries off _static_anchor_fifo's front into `out` // (appending), decrementing each popped entry's class occupancy in // _static_anchor_fifo_klass_counts (erased at zero, so the map tracks the FIFO's live contents), diff --git a/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc b/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc index 6c4e4e525d..5f7b773ba0 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsAnchorTests.inc @@ -907,6 +907,8 @@ TEST_F(ReferenceChainsBfsTest, LocalRootAnchorIndexIsCappedAndCompactsDeadEntrie ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); EXPECT_EQ(pending_cap + 1, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + // A productive compaction leaves the compaction backoff disarmed. + EXPECT_EQ(0, ReferenceChainsTestAccessor::localCompactBackoffForTest()); EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); EXPECT_LE(ReferenceChainsTestAccessor::freshAnchorQueueSizeForTest(), ReferenceChainsTestAccessor::anchorIndexSizeForTest()); @@ -980,3 +982,281 @@ TEST_F(ReferenceChainsBfsTest, DurableRootUpgradeReleasesLocalAnchorSlot) { tracker->stop(); } + +// When the local share is at the cap and full of live containers, a compaction that removes nothing +// arms a backoff: the next LOCAL_ANCHOR_COMPACT_BACKOFF at-cap resolves skip the locked scan (the +// cap holds, the parked containers are dropped and re-reported). Dead locals are reclaimed by the +// first compaction after the backoff runs out, which disarms it. Under the cap the backoff is +// neither consulted nor changed. +TEST_F(ReferenceChainsBfsTest, LocalRootAnchorCompactBackoffSkipsNoProgressCompactions) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + FrontierTable *frontier = tracker->frontierTable(); + EXPECT_EQ(0, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + + const size_t cap = ReferenceChainsTestAccessor::kLocalAnchorIndexCap; + const int backoff = ReferenceChainsTestAccessor::kLocalCompactBackoff; + ASSERT_GT(backoff, 1); + const jlong kContainer = -451, kLater = -452; + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + + const jlong local_base = 1000; + for (size_t i = 0; i < cap; i++) { + jlong tag = local_base + (jlong)i; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, tag, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(tag, kContainer); + } + ASSERT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + + // Parked containers carry no frontier entry: they are only checked for admission. + jlong next_parked = local_base + (jlong)cap + 10000; + + // No-progress compaction arms the backoff; the cap holds. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(next_parked++, kContainer); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(backoff, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + + // A skipped at-cap attempt decrements once per resolve, however many containers were parked. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(next_parked++, kContainer); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(next_parked++, kContainer); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(backoff - 1, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + + // Under the cap the backoff is untouched: free one local slot via a durable-root upgrade, then + // admit a holder that becomes a confirmed container while parked. + ReferenceChainsTestAccessor::addToStaticAnchorIndexForTest( + local_base, kContainer, JVMTI_HEAP_REFERENCE_STATIC_FIELD); + ASSERT_EQ(cap - 1, ReferenceChainsTestAccessor::localAnchorCountForTest()); + const jlong later = next_parked++; + ASSERT_TRUE(ReferenceChainsTestAccessor::insertFrontierEntry( + frontier, later, 0, 0, FrontierEntryState::FRONTIER, + JVMTI_HEAP_REFERENCE_STACK_LOCAL)); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(later, kLater); + ASSERT_EQ(1u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + ReferenceChainsTestAccessor::primeClassShapeForTest(kLater, true); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + const size_t index_size = cap + 1; // the upgraded durable entry stays indexed + EXPECT_EQ(index_size, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(backoff - 1, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + + // Kill part of the local share. While the backoff is armed the dead entries keep their slots. + const size_t dead = 100; + for (size_t i = 1; i <= dead; i++) { + frontier->clear(local_base + (jlong)i); + } + for (int k = backoff - 1; k > 0; k--) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(next_parked++, kContainer); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(index_size, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(k - 1, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + } + + // Backoff exhausted: the next at-cap resolve compacts the dead share, disarms the backoff and + // admits the parked container. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(next_parked++, kContainer); + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(cap - dead + 1, ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ(index_size - dead + 1, ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(0, ReferenceChainsTestAccessor::localCompactBackoffForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + tracker->stop(); +} + +// A full pending buffer gives confirmed containers (met at the local cap) priority over entries not +// yet confirmed as containers: the oldest such entry is evicted through a forward-only cursor, dedup +// stays exact, and once only confirmed containers are parked a further container is dropped (to be +// re-reported by the next root enumeration). +TEST_F(ReferenceChainsBfsTest, LocalRootAnchorPendingBufferEvictsForConfirmedContainer) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + + const size_t cap = ReferenceChainsTestAccessor::kLocalAnchorIndexCap; + const size_t pending_cap = ReferenceChainsTestAccessor::kPendingLocalAnchorCap; + ASSERT_GT(pending_cap, 8u); + const jlong kContainer = -601, kUnknown = -602; + ReferenceChainsTestAccessor::primeClassShapeForTest(kContainer, true); + + // Fill the local cap so confirmed containers park instead of being indexed directly. + for (size_t i = 0; i < cap; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(10000 + (jlong)i, kContainer); + } + ASSERT_EQ(cap, ReferenceChainsTestAccessor::localAnchorCountForTest()); + + // Fill the pending buffer with unclassified holders. + const jlong unk_base = 20000; + for (size_t i = 0; i < pending_cap; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(unk_base + (jlong)i, kUnknown); + } + ASSERT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + ASSERT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + // The first container evicts the oldest unclassified entry, in place. + const jlong c_base = 30000; + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(c_base, kContainer); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_TRUE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(c_base)); + EXPECT_EQ(c_base, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(0)); + EXPECT_FALSE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(unk_base)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(unk_base + 1)); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + // The second evicts the next-oldest; the parked container below the cursor is kept. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(c_base + 1, kContainer); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_EQ(c_base, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(0)); + EXPECT_EQ(c_base + 1, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(1)); + EXPECT_FALSE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(unk_base + 1)); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + // Dedup: re-reporting a parked container or a parked unclassified holder changes nothing. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(c_base, kContainer); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(unk_base + 5, kUnknown); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_EQ(c_base, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(0)); + EXPECT_EQ(c_base + 1, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(1)); + EXPECT_EQ(unk_base + 2, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(2)); + EXPECT_TRUE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(unk_base + 2)); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + // A new unclassified holder at a full buffer is dropped (no eviction). + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(40000, kUnknown); + EXPECT_FALSE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(40000)); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + // Evict every remaining unclassified entry. + for (size_t i = 2; i < pending_cap; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(c_base + (jlong)i, kContainer); + } + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + for (size_t i = 0; i < pending_cap; i++) { + EXPECT_EQ(c_base + (jlong)i, ReferenceChainsTestAccessor::pendingLocalAnchorTagForTest(i)); + EXPECT_FALSE( + ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(unk_base + (jlong)i)); + } + + // Only confirmed containers are parked: a further container is dropped. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(50000, kContainer); + EXPECT_FALSE(ReferenceChainsTestAccessor::pendingLocalAnchorContainsForTest(50000)); + EXPECT_EQ(pending_cap, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + // Resolve empties the buffer and rewinds the cursor. + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalEvictCursorForTest()); + + tracker->stop(); +} + +// The per-pass classification request (the pure first step of reconcileAnchorClassShapes()) holds +// exactly ANCHOR_SHAPE_RECONCILE_BUDGET distinct class tags, in pending-buffer order; holders whose +// classes did not make the batch stay unclassified and are dropped by the resolve step. +TEST_F(ReferenceChainsBfsTest, AnchorShapeReconcileBudgetCapsRequestedClasses) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + + const int budget = ReferenceChainsTestAccessor::kAnchorShapeReconcileBudget; + const int extra = 10; + ASSERT_GT(ReferenceChainsTestAccessor::kPendingLocalAnchorCap, (size_t)(budget + extra + 1)); + ASSERT_GT(ReferenceChainsTestAccessor::kLocalAnchorIndexCap, (size_t)(budget + 1)); + + // Holder 61000 shares the first holder's class: the request must list it once. + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(60000, -7000); + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(61000, -7000); + for (int i = 1; i < budget + extra; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(60000 + i, -(jlong)(7000 + i)); + } + ASSERT_EQ((size_t)(budget + extra + 1), + ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + + std::vector req = ReferenceChainsTestAccessor::unclassifiedAnchorClassTagsForTest(); + ASSERT_EQ((size_t)budget, req.size()); + for (int i = 0; i < budget; i++) { + EXPECT_EQ(-(jlong)(7000 + i), req[i]); + } + + // Classify the requested batch as containers; the remainder stays unclassified and is dropped. + for (jlong class_tag : req) { + ReferenceChainsTestAccessor::primeClassShapeForTest(class_tag, true); + } + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + EXPECT_EQ((size_t)(budget + 1), ReferenceChainsTestAccessor::localAnchorCountForTest()); + EXPECT_EQ((size_t)(budget + 1), ReferenceChainsTestAccessor::anchorIndexSizeForTest()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + EXPECT_TRUE(ReferenceChainsTestAccessor::unclassifiedAnchorClassTagsForTest().empty()); + + tracker->stop(); +} + +// Pending holders' classes are requested before indexed anchors' classes (the pending ones are +// dropped if left unclassified; indexed anchors just wait), and the index scan stops once the +// budget is exhausted. +TEST_F(ReferenceChainsBfsTest, AnchorShapeReconcilePrefersPendingHolders) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + + const int budget = ReferenceChainsTestAccessor::kAnchorShapeReconcileBudget; + const int fresh_pending = 4; + ASSERT_GT(ReferenceChainsTestAccessor::kPendingLocalAnchorCap, (size_t)(budget + fresh_pending)); + ASSERT_GT(ReferenceChainsTestAccessor::kLocalAnchorIndexCap, (size_t)(budget + 1)); + // Class-tag ranges below must stay disjoint. + ASSERT_LT(budget + 20, 500); + + // Phase 1: durable anchors with unclassified classes, then a full budget of pending classes. + const int durable_first = 20; + for (int i = 0; i < durable_first; i++) { + ReferenceChainsTestAccessor::addToStaticAnchorIndexForTest( + 80000 + i, -(jlong)(9000 + i), JVMTI_HEAP_REFERENCE_STATIC_FIELD); + } + for (int i = 0; i < budget; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(81000 + i, -(jlong)(9500 + i)); + } + std::vector req = ReferenceChainsTestAccessor::unclassifiedAnchorClassTagsForTest(); + ASSERT_EQ((size_t)budget, req.size()); + for (int i = 0; i < budget; i++) { + EXPECT_EQ(-(jlong)(9500 + i), req[i]); + } + for (int i = 0; i < durable_first; i++) { + EXPECT_EQ(std::find(req.begin(), req.end(), -(jlong)(9000 + i)), req.end()) + << "durable class requested ahead of pending classes"; + } + + // Phase 2: nothing classified, so the resolve drops every pending holder. A few new pending + // classes come first, then durable classes fill the rest of the budget in index order. + ReferenceChainsTestAccessor::resolvePendingLocalAnchorsForTest(); + ASSERT_EQ(0u, ReferenceChainsTestAccessor::pendingLocalAnchorCountForTest()); + for (int i = 0; i < fresh_pending; i++) { + ReferenceChainsTestAccessor::noteLocalRootHolderForTest(82000 + i, -(jlong)(9800 + i)); + } + for (int i = durable_first; i < budget + durable_first; i++) { + ReferenceChainsTestAccessor::addToStaticAnchorIndexForTest( + 80000 + i, -(jlong)(9000 + i), JVMTI_HEAP_REFERENCE_STATIC_FIELD); + } + req = ReferenceChainsTestAccessor::unclassifiedAnchorClassTagsForTest(); + ASSERT_EQ((size_t)budget, req.size()); + for (int i = 0; i < fresh_pending; i++) { + EXPECT_EQ(-(jlong)(9800 + i), req[i]); + } + for (int j = 0; j < budget - fresh_pending; j++) { + EXPECT_EQ(-(jlong)(9000 + j), req[fresh_pending + j]); + } + + tracker->stop(); +} diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index df546b590d..feaa904f93 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h +++ b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h @@ -97,6 +97,8 @@ class ReferenceChainsTestAccessor { t->_local_anchor_tags.clear(); t->_pending_local_anchors.clear(); t->_pending_local_anchor_tags.clear(); + t->_local_compact_backoff = 0; + t->_pending_local_evict_cursor = 0; t->_anchor_container_cursor = 0; t->_anchor_other_cursor = 0; t->_class_shape_cache.clear(); @@ -474,7 +476,9 @@ class ReferenceChainsTestAccessor { t->_static_anchor_index_tags.empty() && t->_local_anchor_tags.empty() && t->_pending_local_anchors.empty() && - t->_pending_local_anchor_tags.empty(); + t->_pending_local_anchor_tags.empty() && + t->_local_compact_backoff == 0 && + t->_pending_local_evict_cursor == 0; } // Local-root anchor seams (see noteLocalRootHolder()/resolvePendingLocalAnchors()). @@ -500,6 +504,30 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::LOCAL_ANCHOR_INDEX_CAP; static constexpr size_t kPendingLocalAnchorCap = ReferenceChainTracker::PENDING_LOCAL_ANCHOR_CAP; + static constexpr int kLocalCompactBackoff = + ReferenceChainTracker::LOCAL_ANCHOR_COMPACT_BACKOFF; + static int localCompactBackoffForTest() { + return ReferenceChainTracker::instance()->_local_compact_backoff; + } + static size_t pendingLocalEvictCursorForTest() { + return ReferenceChainTracker::instance()->_pending_local_evict_cursor; + } + // Caller keeps idx < pendingLocalAnchorCountForTest(). + static jlong pendingLocalAnchorTagForTest(size_t idx) { + return ReferenceChainTracker::instance()->_pending_local_anchors[idx].tag; + } + static bool pendingLocalAnchorContainsForTest(jlong tag) { + return ReferenceChainTracker::instance() + ->_pending_local_anchor_tags.count(tag) > 0; + } + + // Reconcile collection step (see collectUnclassifiedAnchorClassTags()). + static constexpr int kAnchorShapeReconcileBudget = + ReferenceChainTracker::ANCHOR_SHAPE_RECONCILE_BUDGET; + static std::vector unclassifiedAnchorClassTagsForTest() { + return ReferenceChainTracker::instance() + ->collectUnclassifiedAnchorClassTags(); + } static jlong discoveredTagForTest(int slot, int idx) { return ReferenceChainTracker::instance() diff --git a/doc/architecture/LiveHeapReferenceChainsImplementation.md b/doc/architecture/LiveHeapReferenceChainsImplementation.md index 48ec71681b..4de8724476 100644 --- a/doc/architecture/LiveHeapReferenceChainsImplementation.md +++ b/doc/architecture/LiveHeapReferenceChainsImplementation.md @@ -166,14 +166,21 @@ yet classified waits in `_pending_local_anchors`, capped at 1024 entries. confirmed collections and drops everything else, including holders whose class is still unclassified. The next root enumeration reports a dropped holder again. Reconciliation classifies pending holder classes before unclassified durable-anchor classes, within the -128-classes-per-pass budget. +128-classes-per-pass budget. When the pending buffer is full, a holder already confirmed as a +collection (met while the local share is at its cap) evicts the oldest entry not confirmed as a +collection. An unclassified holder that meets a full buffer is dropped, and so is a confirmed +collection when the buffer holds only confirmed collections. Either way the next root enumeration +reports it again. The local share is capped at 4096 entries (`LOCAL_ANCHOR_INDEX_CAP`). This keeps the per-pass index scan proportional to the durable share plus a constant, regardless of how many locals the application holds. When the cap is reached, `compactLocalAnchors()` removes local entries that -are no longer selectable, such as entries whose object died, at most once per resolve call. If the share is still -full of live collections, new local holders are not indexed until entries die or the search -restarts. A local holder that a durable root later attaches moves to the durable share and stops +are no longer selectable, such as entries whose object died, at most once per resolve call. It +scans only the local share, so its cost does not grow with the durable share. If the share is +still full of live collections, new local holders are not indexed until entries die or the search +restarts. A compaction that removes nothing skips the next 8 at-cap compaction attempts +(`LOCAL_ANCHOR_COMPACT_BACKOFF`), so a full share of live collections does not pay the scan on +every pass. Dead entries can therefore keep their slots for up to 8 extra passes. A local holder that a durable root later attaches moves to the durable share and stops counting against the cap. `restartSearch()` clears both shares and the pending buffer. ## Search termination