diff --git a/ddprof-lib/src/main/cpp/referenceChainEvents.cpp b/ddprof-lib/src/main/cpp/referenceChainEvents.cpp index 7290ce94a0..cfc2515ad4 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. @@ -511,6 +512,25 @@ void ReferenceChainTracker::invalidateResolvedChain(jlong source_tag) { } } +bool ReferenceChainTracker::replaceResolvedChain(jlong source_tag, ReferenceChainEvent &&event, + u64 current_search_ns) { + ExclusiveLockGuard guard(&_resolved_chains_lock); + auto it = _resolved_chains.find(source_tag); + // Replace only: a dump may have evicted the entry (dead owner) since the rebuild scan, and a + // rebuild must never resurrect it; an entry from another search generation is a different + // object under the same numeric tag. source_search_ns is deliberately not re-stamped. + if (it == _resolved_chains.end() || + it->second.source_search_ns != current_search_ns) { + return false; + } + it->second.event = std::move(event); + it->second.source_tag = source_tag; + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations rebuilt " + "source_tag=%lld depth=%u", + (long long)source_tag, it->second.event._depth); + return true; +} + jlong ReferenceChainTracker::resolvedChainTargetTagForTest(jlong object_tag) { if (object_tag <= 0) { return 0; @@ -572,7 +592,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 +603,66 @@ void ReferenceChainTracker::deferResolvedChainInvalidation( if (!was_cached) { return; } + // Cache keys are frontier tags (>= 0), and the kind is encoded in-band (EVICT = tag, + // REBUILD = ~tag), which is unambiguous only for non-negative tags: EVICT(-t) would read as + // REBUILD(t - 1) and REBUILD(-t) as EVICT(t - 1), acting on another tag's cached chain. A + // negative tag is therefore never queued - a defined policy in every build type, not an assert. + if (source_tag < 0) { + TEST_LOG("ReferenceChainTracker::deferResolvedChainInvalidation ignored negative " + "source_tag=%lld", + (long long)source_tag); + 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. + // Distinct pending tags stay <= MAX_PENDING_CHAIN_INVALIDATIONS (2 * MAX_RESOLVED_CHAINS): + // eligible tags come from one pre-traversal snapshot (<= MAX_RESOLVED_CHAINS; cache inserts are + // serialized with traversals on the engine thread and dumps only remove keys), and REBUILD + // entries left pending - by the dump drain, or requeued by applyPendingChainInvalidations() over + // its per-poll cap - are a subset of the cache at the last apply (another <= MAX_RESOLVED_CHAINS). 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 +674,111 @@ 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. + // At most MAX_CHAIN_REBUILDS_PER_POLL heap walks per poll keep this BFS-thread step's latency + // bounded; eligible REBUILD entries over the cap are requeued (idempotent - a later rebuild + // replaces the same entry under the same generation guard). + jlong rebuild_tags[MAX_CHAIN_REBUILDS_PER_POLL]; + 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(); + + // Eligible REBUILD entries over the cap are packed in place at the front of the scratch + // buffer (deferred <= i), preserving their FIFO order. + size_t deferred = 0; + for (size_t i = 0; i < _draining_chain_invalidations.size(); i++) { + const jlong pending = _draining_chain_invalidations[i]; + const jlong tag = pendingChainInvalidationTag(pending); + if (!isPendingChainRebuild(pending)) { + if (_resolved_chains.erase(tag) > 0) { + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations 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) { + continue; // no-op REBUILD: nothing current to rebuild + } + if (rebuild_count < MAX_CHAIN_REBUILDS_PER_POLL) { + rebuild_tags[rebuild_count++] = tag; + } else { + _draining_chain_invalidations[deferred++] = pending; + } + } + if (deferred > 0) { + // Lock order: _resolved_chains_lock (held) before the pending lock. + _pending_chain_invalidations_lock.lock(); + for (size_t i = 0; i < deferred; i++) { + const jlong pending = _draining_chain_invalidations[i]; + const jlong tag = pendingChainInvalidationTag(pending); + const jlong evict = + encodePendingChainInvalidation(tag, ChainInvalidationKind::EVICT); + bool queued = false; + for (jlong existing : _pending_chain_invalidations) { + // A newer EVICT wins; a newer REBUILD for the same tag is a duplicate. + if (existing == evict || existing == pending) { + queued = true; + break; + } + } + if (queued) { + continue; + } + 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(pending); + } else { + // Unreachable under the 2 * MAX_RESOLVED_CHAINS bound; the cached chain is kept. + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations requeue overflow, " + "keeping cached chain source_tag=%lld", + (long long)tag); + } + } + _pending_chain_invalidations_lock.unlock(); + TEST_LOG("ReferenceChainTracker::applyPendingChainInvalidations deferred=%zu rebuilds", + deferred); + } + _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(); + replaceResolvedChain(tag, std::move(event), current_search_ns); + } +} + void ReferenceChainTracker::clearPendingChainInvalidations() { ExclusiveLockGuard cache_guard(&_resolved_chains_lock); _pending_chain_invalidations_lock.lock(); @@ -759,7 +918,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..f6b2057839 100644 --- a/ddprof-lib/src/main/cpp/referenceChains.h +++ b/ddprof-lib/src/main/cpp/referenceChains.h @@ -586,11 +586,29 @@ 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. - static constexpr int MAX_PENDING_CHAIN_INVALIDATIONS = MAX_RESOLVED_CHAINS; + // Chain invalidations deferred by heap callbacks. The ChainInvalidationKind is stored in-band + // (see encodePendingChainInvalidation()): EVICT entries hold the source tag, REBUILD entries its + // bitwise complement. At most one entry per tag (EVICT wins over REBUILD, duplicates dropped). + // Bound: new deferrals are limited to tags in the immutable pre-traversal resolved-cache + // snapshot (<= MAX_RESOLVED_CHAINS), and the REBUILD entries applyPendingChainInvalidations() + // requeues over its per-poll budget are a subset of the cache at apply time (another + // <= MAX_RESOLVED_CHAINS), so distinct pending tags stay <= 2 * MAX_RESOLVED_CHAINS. + // Ownership: + // - _pending_chain_invalidations: every access (heap-callback producers, both drains, the apply + // requeue, clearPendingChainInvalidations()) is under _pending_chain_invalidations_lock + // (full-barrier acquire/release). + // - _draining_chain_invalidations (scratch): touched only while _resolved_chains_lock is held, + // which serializes both drainers and clearPendingChainInvalidations(); heap callbacks never + // touch it. Drains fill it while also holding the pending lock (partition in the dump drain, + // swap in the apply drain) and consume it after releasing that lock; it is empty between + // drains. + // - Lock order: _resolved_chains_lock before _pending_chain_invalidations_lock. + // Both buffers are reserved to MAX_PENDING_CHAIN_INVALIDATIONS up front, so no deferral, drain + // or requeue reallocates. + static constexpr int MAX_PENDING_CHAIN_INVALIDATIONS = 2 * MAX_RESOLVED_CHAINS; + // Per-poll cap on applyPendingChainInvalidations()' buildChainEvent() heap walks; eligible + // REBUILD entries over the cap stay pending for later polls. + static constexpr int MAX_CHAIN_REBUILDS_PER_POLL = 8; std::vector _pending_chain_invalidations; std::vector _draining_chain_invalidations; SpinLock _pending_chain_invalidations_lock; @@ -1007,19 +1025,59 @@ class ReferenceChainTracker { // Remove a cached chain so pollWatchedTargets rebuilds it on the next poll. void invalidateResolvedChain(jlong source_tag); + // Rebuild publish step (applyPendingChainInvalidations()): replaces the cached chain for + // source_tag with `event` only if an entry exists and was cached under current_search_ns. + // Never inserts (a dump may have evicted the entry since the scan: must not resurrect it), never + // overwrites an entry another search generation cached under the same numeric tag, and keeps + // the entry's source_search_ns. Takes only _resolved_chains_lock. Returns whether it replaced. + bool replaceResolvedChain(jlong source_tag, ReferenceChainEvent &&event, + u64 current_search_ns); + // 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 every EVICT entry like the dump path, drops + // REBUILD entries whose tag is no longer cached or was cached by another search generation, and + // rebuilds at most MAX_CHAIN_REBUILDS_PER_POLL current-generation chains from the frontier via + // buildChainEvent() (overwriting on success, keeping the old chain on failure). Eligible REBUILD + // entries over that cap are requeued in order for later polls. 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..e2a4e5e0e3 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsBfsTests.inc @@ -1135,3 +1135,409 @@ 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(); +} + +// Polls once the way the BFS thread does (gc generations enabled, klass population reset). +static void pollOnceForTest(ReferenceChainTracker *tracker, jvmtiEnv *jvmti, JNIEnv *jni) { + LivenessTracker::instance()->klassPopulationResetForTest(); + LivenessTracker::instance()->setGcGenerationsForTest(true); + tracker->pollWatchedTargets(jvmti, jni); + LivenessTracker::instance()->setGcGenerationsForTest(false); +} + +// Caches a plain (non-leak) chain for tag under search_ns. +static bool cachePlainChainForTest(jlong tag, u32 depth, u64 search_ns) { + ReferenceChainEvent event; + event._target_tag = (u64)tag; + event._depth = depth; + return ReferenceChainsTestAccessor::cacheChain(tag, std::move(event), tag, search_ns); +} + +// Per-poll rebuild budget: one poll performs at most MAX_CHAIN_REBUILDS_PER_POLL buildChainEvent() +// heap walks; eligible REBUILD entries over the budget stay pending for later polls instead of being +// dropped, and neither the requeue nor the swap reallocates the reserved buffers. +TEST_F(ReferenceChainsBfsTest, PollAmortizesChainRebuildsAcrossPolls) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=64")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + const u64 kSearchNs = 779; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kSearchNs); + const int per = ReferenceChainsTestAccessor::maxChainRebuildsPerPoll(); + ASSERT_GT(per, 0); + const int n = 2 * per + 1; + for (jlong tag = 1; tag <= n; tag++) { + ASSERT_TRUE(cachePlainChainForTest(tag, 0, kSearchNs)); + } + std::vector snapshot = ReferenceChainsTestAccessor::snapshotResolvedChainTags(); + for (jlong tag = 1; tag <= n; tag++) { + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(tag, snapshot, true); + } + ASSERT_EQ((size_t)n, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + EXPECT_EQ((size_t)(n - per), ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "rebuilds over the per-poll budget must stay pending"; + EXPECT_EQ(0u, ReferenceChainsTestAccessor::drainingChainInvalidationCount()); + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::drainingChainInvalidationCount()); + + // No frontier entries: every rebuild fails and keeps its cached chain. + for (jlong tag = 1; tag <= n; tag++) { + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(tag)) << "tag " << tag; + } + const size_t max_pending = (size_t)ReferenceChainsTestAccessor::maxPendingChainInvalidations(); + EXPECT_GE(ReferenceChainsTestAccessor::pendingChainInvalidationCapacity(), max_pending); + EXPECT_GE(ReferenceChainsTestAccessor::drainingChainInvalidationCapacity(), max_pending); + + tracker->stop(); +} + +// The budget counts only eligible REBUILDs: every EVICT is applied in the same poll (even ones queued +// after the budget is used up), and no-op REBUILDs (other search generation) are dropped, not +// requeued. +TEST_F(ReferenceChainsBfsTest, PollAppliesAllEvictsAndDropsNoopRebuildsUnderCap) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=64")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + const u64 kSearchNs = 780; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kSearchNs); + const int per = ReferenceChainsTestAccessor::maxChainRebuildsPerPoll(); + const int n = per + 3; + for (jlong tag = 1; tag <= n; tag++) { + ASSERT_TRUE(cachePlainChainForTest(tag, 0, kSearchNs)); + } + for (jlong tag = 101; tag <= 103; tag++) { + ASSERT_TRUE(cachePlainChainForTest(tag, 0, kSearchNs)); + } + ASSERT_TRUE(cachePlainChainForTest(201, 0, kSearchNs + 1)); + std::vector snapshot = ReferenceChainsTestAccessor::snapshotResolvedChainTags(); + + for (jlong tag = 1; tag <= n; tag++) { + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(tag, snapshot, true); + } + for (jlong tag = 101; tag <= 103; tag++) { + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(tag, snapshot); + } + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(201, snapshot, true); + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(4, snapshot, true); // duplicate + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(2, snapshot); // EVICT wins + ASSERT_EQ((size_t)(n + 3 + 1), ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + for (jlong tag = 101; tag <= 103; tag++) { + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(tag)) + << "EVICT past the rebuild budget must still be applied, tag " << tag; + } + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(2)); + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(201)) + << "a REBUILD must never touch another generation's chain"; + EXPECT_EQ(kSearchNs + 1, ReferenceChainsTestAccessor::resolvedChainSearchNs(201)); + // Eligible REBUILDs: 1, 3..n (= per + 2). per rebuilt (and failed), 2 requeued; 201 dropped. + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + for (jlong tag = 1; tag <= n; tag++) { + if (tag == 2) { + continue; + } + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(tag)) << "tag " << tag; + } + + tracker->stop(); +} + +// Requeued REBUILDs keep the ordinary encoding, so the one-entry-per-tag rule still holds: a later +// duplicate REBUILD is dropped, a later EVICT replaces the requeued REBUILD, and the dump drain +// applies that EVICT while leaving the other REBUILD for the poll. +TEST_F(ReferenceChainsBfsTest, RequeuedRebuildsKeepOneEntryPerTagAndYieldToEvict) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=64")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + const u64 kSearchNs = 781; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kSearchNs); + const int per = ReferenceChainsTestAccessor::maxChainRebuildsPerPoll(); + const int n = per + 2; + for (jlong tag = 1; tag <= n; tag++) { + ASSERT_TRUE(cachePlainChainForTest(tag, 0, kSearchNs)); + } + std::vector snapshot = ReferenceChainsTestAccessor::snapshotResolvedChainTags(); + for (jlong tag = 1; tag <= n; tag++) { + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(tag, snapshot, true); + } + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + ASSERT_EQ(2u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + const jlong kRequeuedA = per + 1; + const jlong kRequeuedB = per + 2; + + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(kRequeuedA, snapshot, true); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "duplicate REBUILD of a requeued tag must be dropped"; + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(kRequeuedB, snapshot); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "EVICT must replace the requeued REBUILD in place"; + + std::vector dump; + ReferenceChainsTestAccessor::drain(&dump); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(kRequeuedB)); + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(kRequeuedA)); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "the dump drain leaves the requeued REBUILD for the poll"; + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(kRequeuedA)); + + tracker->stop(); +} + +// The REBUILD publish step replaces only: it never inserts (a dump may have evicted the entry since +// the rebuild scan), never overwrites a chain cached under another search generation, and never +// re-stamps source_search_ns. +TEST_F(ReferenceChainsBfsTest, RebuildReplaceNeverResurrectsOrCrossesGeneration) { + Arguments args; + ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=64")); + ReferenceChainTracker *tracker = ReferenceChainTracker::instance(); + ASSERT_FALSE(tracker->start(args)); + const u64 kCurrentNs = 900; + const u64 kStaleNs = 901; + ReferenceChainsTestAccessor::setSearchStartNsForTest(kCurrentNs); + + // (a) Absent tag: no insert. + ReferenceChainEvent absent; + absent._target_tag = 50; + absent._depth = 3; + EXPECT_FALSE(ReferenceChainsTestAccessor::replaceResolvedChain(50, std::move(absent), + kCurrentNs)); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(50)); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::resolvedChainCount()); + + // (b) Cached under another generation: left untouched. + ASSERT_TRUE(cachePlainChainForTest(51, 1, kCurrentNs)); + ReferenceChainEvent stale; + stale._target_tag = 51; + stale._depth = 5; + EXPECT_FALSE(ReferenceChainsTestAccessor::replaceResolvedChain(51, std::move(stale), + kStaleNs)); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::resolvedChainDepth(51)); + EXPECT_EQ(kCurrentNs, ReferenceChainsTestAccessor::resolvedChainSearchNs(51)); + + // (c) Same generation: replaced in place, generation not re-stamped. + ReferenceChainEvent fresh; + fresh._target_tag = 51; + fresh._depth = 5; + EXPECT_TRUE(ReferenceChainsTestAccessor::replaceResolvedChain(51, std::move(fresh), + kCurrentNs)); + EXPECT_EQ(5u, ReferenceChainsTestAccessor::resolvedChainDepth(51)); + EXPECT_EQ(kCurrentNs, ReferenceChainsTestAccessor::resolvedChainSearchNs(51)); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::resolvedChainCount()); + + // (d) Evicted after caching: the rebuild must not resurrect it. + ASSERT_TRUE(cachePlainChainForTest(52, 0, kCurrentNs)); + std::vector snapshot = ReferenceChainsTestAccessor::snapshotResolvedChainTags(); + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(52, snapshot); + ReferenceChainsTestAccessor::drainPendingChainInvalidations(); + ASSERT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(52)); + ReferenceChainEvent late; + late._target_tag = 52; + late._depth = 2; + EXPECT_FALSE(ReferenceChainsTestAccessor::replaceResolvedChain(52, std::move(late), + kCurrentNs)); + EXPECT_FALSE(ReferenceChainsTestAccessor::hasResolvedChainForTag(52)); + + tracker->stop(); +} + +// Dead-owner guard of the rebuild loop: a REBUILD whose rebuilt chain names a stale ownership period +// of its leak tag must not replace the current owner's cached chain. +TEST_F(ReferenceChainsBfsTest, DeadOwnerRebuildKeepsCurrentOwnerChain) { + 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; + 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 queues the REBUILD for 105. + 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()); + + ReferenceChainsTestAccessor::setCandidateCountForTest(0); + ReferenceChainsTestAccessor::setCandidateFoundBitsForTest(0); + // Re-bind the frontier entry to an earlier ownership period of kLeakTag: the rebuild now builds + // a dead-owner chain. (kStamp - 1u may wrap; any value other than the current stamp works.) + frontier->setLeakTag(105, kLeakTag, kStamp - 1u); + + pollOnceForTest(tracker, &mock_jvmti, &mock_jni); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()); + + std::vector dump; + ReferenceChainsTestAccessor::drain(&dump); + ASSERT_EQ(1u, dump.size()) << "dead-owner rebuild replaced (and so lost) the live owner's chain"; + EXPECT_EQ((u64)kLeakTag, dump[0]._target_tag); + EXPECT_EQ(kStamp, dump[0]._leak_tag_incarnation); + EXPECT_EQ(0u, dump[0]._depth) << "cached chain must be kept, not replaced by the stale rebuild"; + + tracker->stop(); +} diff --git a/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h b/ddprof-lib/src/test/cpp/referenceChainsTestAccessors.h index a7045e8582..9c81d50dc9 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() { @@ -270,6 +274,44 @@ class ReferenceChainsTestAccessor { ReferenceChainTracker::instance()->drainPendingChainInvalidations(); } + static int maxChainRebuildsPerPoll() { + return ReferenceChainTracker::MAX_CHAIN_REBUILDS_PER_POLL; + } + + static int maxPendingChainInvalidations() { + return ReferenceChainTracker::MAX_PENDING_CHAIN_INVALIDATIONS; + } + + // The REBUILD publish step of applyPendingChainInvalidations() (replace-only, generation + // checked, never re-stamps source_search_ns). + static bool replaceResolvedChain(jlong tag, ReferenceChainEvent event, u64 search_ns) { + return ReferenceChainTracker::instance()->replaceResolvedChain( + tag, std::move(event), search_ns); + } + + // Cached chain depth for tag, 0 when absent (callers check hasResolvedChainForTag first). + static u32 resolvedChainDepth(jlong tag) { + ReferenceChainTracker *t = ReferenceChainTracker::instance(); + auto it = t->_resolved_chains.find(tag); + return it == t->_resolved_chains.end() ? 0u : (u32)it->second.event._depth; + } + + // Search generation the cached chain for tag was stored under, 0 when absent. + static u64 resolvedChainSearchNs(jlong tag) { + ReferenceChainTracker *t = ReferenceChainTracker::instance(); + auto it = t->_resolved_chains.find(tag); + return it == t->_resolved_chains.end() ? 0u : it->second.source_search_ns; + } + + // Holds/releases the private _pending_chain_invalidations_lock, so a test can prove the + // rebuild publish step never takes it. + static void lockPendingChainInvalidationsForTest() { + ReferenceChainTracker::instance()->_pending_chain_invalidations_lock.lock(); + } + static void unlockPendingChainInvalidationsForTest() { + ReferenceChainTracker::instance()->_pending_chain_invalidations_lock.unlock(); + } + static bool hasResolvedChainForTag(jlong tag) { ReferenceChainTracker *t = ReferenceChainTracker::instance(); return t->_resolved_chains.find(tag) != t->_resolved_chains.end(); diff --git a/ddprof-lib/src/test/cpp/referenceChainsTrackerTests.inc b/ddprof-lib/src/test/cpp/referenceChainsTrackerTests.inc index 44f68b6e8b..5fc67f21e1 100644 --- a/ddprof-lib/src/test/cpp/referenceChainsTrackerTests.inc +++ b/ddprof-lib/src/test/cpp/referenceChainsTrackerTests.inc @@ -578,6 +578,29 @@ TEST_F(ResolvedChainCacheTest, DeferredInvalidationFiltersDeduplicatesAndDrainsE ReferenceChainsTestAccessor::drainingChainInvalidationCount()); } +// The kind is encoded in-band (EVICT = tag, REBUILD = ~tag), which is unambiguous only for +// non-negative tags, so a negative source tag is never queued - in every build type. Real snapshots +// hold only cache keys (>= 0); this hand-built one is the only way to get a negative tag past the +// was-cached filter. +TEST_F(ResolvedChainCacheTest, DeferredInvalidationNeverQueuesNegativeTag) { + ASSERT_TRUE(ReferenceChainsTestAccessor::cacheChain(0, makeEvent(10), 0, 0)); + ASSERT_TRUE(ReferenceChainsTestAccessor::cacheChain(1, makeEvent(11), 1, 0)); + std::vector snapshot{-1, -2}; + + // REBUILD of -2 would encode to ~(-2) == 1: an EVICT of cached tag 1. + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(-2, snapshot, true); + // EVICT of -1 would encode to -1 == ~0: a REBUILD of cached tag 0. + ReferenceChainsTestAccessor::deferResolvedChainInvalidation(-1, snapshot); + EXPECT_EQ(0u, ReferenceChainsTestAccessor::pendingChainInvalidationCount()) + << "a negative source tag must never be queued"; + + ReferenceChainsTestAccessor::drainPendingChainInvalidations(); + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(1)) + << "negative REBUILD aliased tag 1's EVICT"; + EXPECT_TRUE(ReferenceChainsTestAccessor::hasResolvedChainForTag(0)); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::resolvedChainCount()); +} + TEST_F(ResolvedChainCacheTest, DeferredInvalidationIsBoundedByCacheAndKeepsCapacity) { const int cap = ReferenceChainsTestAccessor::maxResolvedChains(); for (int i = 0; i < cap; i++) { @@ -847,6 +870,51 @@ TEST_F(ResolvedChainCacheTest, ResolvedTargetTagSeamLockOrder) { EXPECT_EQ(77, result.load()); } +// The REBUILD publish step (replaceResolvedChain()) takes only _resolved_chains_lock: it must +// complete while _pending_chain_invalidations_lock is held (no inversion of the documented +// resolved -> pending lock order) and must wait while the cache lock is held. +TEST_F(ResolvedChainCacheTest, RebuildReplaceTakesOnlyResolvedChainsLock) { + ReferenceChainsTestAccessor::setSearchStartNsForTest(1000); + ASSERT_TRUE(ReferenceChainsTestAccessor::cacheChain(5, makeEvent(77), 5, 1000)); + + // Pending lock held: the replace must not need it. + ReferenceChainsTestAccessor::lockPendingChainInvalidationsForTest(); + std::atomic done{false}; + std::atomic result{false}; + std::thread first([&] { + ReferenceChainEvent event = makeEvent(77); + event._depth = 1; + result.store(ReferenceChainsTestAccessor::replaceResolvedChain(5, std::move(event), 1000)); + done.store(true); + }); + const bool completed_under_pending_lock = waitForSeamThread(done, 10000); + ReferenceChainsTestAccessor::unlockPendingChainInvalidationsForTest(); + first.join(); + EXPECT_TRUE(completed_under_pending_lock) + << "replaceResolvedChain() must not take _pending_chain_invalidations_lock"; + EXPECT_TRUE(result.load()); + EXPECT_EQ(1u, ReferenceChainsTestAccessor::resolvedChainDepth(5)); + + // Cache lock held: the replace must wait for it. + ReferenceChainsTestAccessor::lockResolvedChainsForTest(); + done.store(false); + result.store(false); + std::thread second([&] { + ReferenceChainEvent event = makeEvent(77); + event._depth = 2; + result.store(ReferenceChainsTestAccessor::replaceResolvedChain(5, std::move(event), 1000)); + done.store(true); + }); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + EXPECT_FALSE(done.load()) << "replaceResolvedChain() must wait on _resolved_chains_lock"; + ReferenceChainsTestAccessor::unlockResolvedChainsForTest(); + + EXPECT_TRUE(waitForSeamThread(done, 10000)); + second.join(); + EXPECT_TRUE(result.load()); + EXPECT_EQ(2u, ReferenceChainsTestAccessor::resolvedChainDepth(5)); +} + // Leak-tag identity is (tag, ownership-period stamp): a chain cached during the tag's current // ownership is answered by the seam and emitted by the dump. TEST_F(ResolvedChainCacheTest, LeakTagChainOfCurrentOwnerIsResolvedAndEmitted) {