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/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..e88f3fded1 100644 --- a/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp +++ b/ddprof-lib/src/main/cpp/referenceChainTraversal.cpp @@ -129,12 +129,25 @@ 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: 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; } @@ -363,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()) { @@ -1044,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 5acd946065..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) && @@ -457,15 +457,16 @@ 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. + // 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)); } @@ -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..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 @@ -586,10 +585,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; @@ -933,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 @@ -1010,16 +1015,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/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 6295e26850..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) { @@ -1135,3 +1235,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..14e56d206b 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() { @@ -799,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) { 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. 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);