diff --git a/.claude/commands/build-and-summarize b/.claude/commands/build-and-summarize index 3923dc7df0..a10bd3d468 100755 --- a/.claude/commands/build-and-summarize +++ b/.claude/commands/build-and-summarize @@ -1,84 +1,44 @@ #!/usr/bin/env bash -set -euo pipefail +# Runs Gradle with the full output captured to a timestamped log so an agent +# session never ingests a multi-thousand-line build log. Prints the status +# line; on failure, extracts the "What went wrong" blocks and failing-test +# lines. The log file remains the source of truth for anything rarer. +# +# Usage: build-and-summarize [gradle-task...] (default: build) +# +# No live tail and no summary artifacts: a harness tool result arrives +# atomically after the run anyway, so streaming filters only added a +# background process to manage, and the failures section below is everything +# an agent needs before deciding to grep the full log. +set -u -mkdir -p build/logs build/reports/claude .claude/out -STAMP="$(date +%Y%m%d-%H%M%S)" - -# Args (default to 'build') ARGS=("$@") -if [ "${#ARGS[@]}" -eq 0 ]; then - ARGS=(build) -fi +[ "${#ARGS[@]}" -eq 0 ] && ARGS=(build) -# Label for the log file from the first arg +STAMP="$(date +%Y%m%d-%H%M%S)" LABEL="$(printf '%s' "${ARGS[0]}" | tr '/:' '__')" LOG="build/logs/${STAMP}-${LABEL}.log" +mkdir -p "$(dirname "$LOG")" -# Ensure we clean the tail on exit -tail_pid="" -cleanup() { [ -n "${tail_pid:-}" ] && kill "$tail_pid" 2>/dev/null || true; } -trap cleanup EXIT INT TERM - -echo "▶ Logging full Gradle output to: $LOG" -echo "▶ Running: ./gradlew ${ARGS[*]} -i --console=plain" -echo " (Console output here is minimized; the full log is in the file.)" -echo - -# Start Gradle fully redirected to the log (no stdout/stderr to this session) -# Use stdbuf to make the output line-buffered in the log for timely tailing. -( stdbuf -oL -eL ./gradlew "${ARGS[@]}" -i --console=plain ) >"$LOG" 2>&1 & -gradle_pid=$! +# One log per build, hundreds of KB each, and agent sessions generate many: +# prune to the newest 20 so the directory stays bounded. +ls -t build/logs/*.log 2>/dev/null | tail -n +21 | xargs rm -f 2>/dev/null || true -# Minimal live progress: follow the log and print only interesting lines -# - Task starts -# - Final build status -# - Test summary lines -# tail itself is the background job, so $! is its PID and killing it also ends -# awk. (Backgrounding "tail | awk" would make $! the PID of awk instead: killing -# awk leaves tail following a log that never grows again, so it never notices, -# and since it inherits this script's stderr, a caller piping our output never -# sees end of input.) -tail -n0 -F "$LOG" 2>/dev/null > >(awk ' - /^> Task / { print; fflush(); next } - /^BUILD (SUCCESSFUL|FAILED)/ { print; fflush(); next } - /[0-9]+ tests? (successful|failed|skipped)/ { print; fflush(); next } -') & -tail_pid=$! - -# Wait for Gradle to finish -wait "$gradle_pid" +echo "▶ ./gradlew ${ARGS[*]} -i --console=plain (full log: $LOG)" +./gradlew "${ARGS[@]}" -i --console=plain >"$LOG" 2>&1 status=$? -# Stop the tail and print a compact summary from the log -kill "$tail_pid" 2>/dev/null || true -tail_pid="" - -echo -echo "=== Summary ===" -# Grab the last BUILD line and nearest test summary lines -awk ' - /^BUILD (SUCCESSFUL|FAILED)/ { lastbuild=$0 } - /[0-9]+ tests? (successful|failed|skipped)/ { tests=$0 } - END { - if (lastbuild) print lastbuild; - if (tests) print tests; - } -' "$LOG" || true - -echo -if [ $status -eq 0 ]; then +if [ "$status" -eq 0 ]; then echo "✔ Gradle completed. Full log at: $LOG" else echo "✖ Gradle failed with status $status. Full log at: $LOG" + echo + echo "=== What went wrong ===" + sed -n '/^\* What went wrong:/,/^\* Try:/p' "$LOG" | head -40 + echo + echo "=== Failing tasks/tests ===" + grep -E '^> Task .+FAILED$' "$LOG" | head -10 + grep -E '[0-9]+ tests completed(, [0-9]+ failed)?' "$LOG" | head -5 + grep -E '[a-zA-Z0-9_.$]+ > [A-Za-z0-9_.$()]+ FAILED$' "$LOG" | head -20 fi - -# Hand over to your logs analyst agent — keep the main session output tiny. -echo -echo "Delegating to gradle-logs-analyst agent…" -# If your CLI supports non-streaming, set it here to avoid verbose output. -# Example (uncomment if supported): export CLAUDE_NO_STREAM=1 - -# Sub-agents would inherit the full parent env, including CLAUDECODE, which -# apparently causes problems with some versions of Claude (e.g. 4.6). -unset CLAUDECODE -claude "Act as the gradle-logs-analyst agent to parse the build log at: $LOG. Generate the required Gradle summary artifacts as specified in the gradle-logs-analyst agent definition." +exit "$status" diff --git a/.gitignore b/.gitignore index 38ff268849..61e98e96a3 100644 --- a/.gitignore +++ b/.gitignore @@ -51,4 +51,16 @@ __pycache__/ # CLAUDE.md is auto-generated from AGENTS.md bootstrap instructions CLAUDE.md -.sphinx +# OS/editor cruft +.DS_Store + +# AI review/agent tooling scratch state +.sphinx/ +.skill-builder-temp/ +.claude/scheduled_tasks.lock + +# Python bytecode cache +*.pyc + +# AI investigation scratch state (never committed) +.investigations/ diff --git a/AGENTS.md b/AGENTS.md index 3efe487869..d5de249d0b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,12 +43,11 @@ You are the **Main Orchestrator** for this repository. ### Goals - When I ask you to build, you MUST: - 1) run the Gradle task with plain console and increased verbosity, - 2) capture stdout into `build/logs/-.log`, - 3) **delegate** parsing to the sub-agent `gradle-log-analyst`, - 4) respond in chat with only a short status and the two output file paths: - - `build/reports/claude/gradle-summary.md` - - `build/reports/claude/gradle-summary.json` + 1) invoke the wrapper `./.claude/commands/build-and-summarize ` — + it runs Gradle with `--console=plain -i`, captures the full output into + `build/logs/-.log`, and prints only the status line; + 2) on failure, read the wrapper's extracted "What went wrong" section and the + failing task/test lines; grep the log file for anything rarer. ### Rules - **Never** paste large log chunks into the chat. @@ -57,13 +56,6 @@ You are the **Main Orchestrator** for this repository. - Assume macOS/Linux unless I explicitly say Windows; if Windows, use PowerShell equivalents. - If a step fails, print the failing command and a one-line hint, then stop. -### Implementation Hints -- For builds, always use: `--console=plain -i` (or `-d` if I ask for debug). -- Use `mkdir -p build/logs build/reports/claude` before writing. -- Timestamp format: `$(date +%Y%m%d-%H%M%S)`. -- After the build finishes, call the sub-agent like: - "Use `gradle-log-analyst` to parse LOG_PATH; write the two reports; reply with only a 3–6 line status and the two relative file paths." - ### Shortcuts I Expect - `./gradlew ` to do everything in one step. - If I just say "build assembleDebugJar", interpret that as the shortcut above. diff --git a/ddprof-lib/src/main/cpp/javaApi.cpp b/ddprof-lib/src/main/cpp/javaApi.cpp index f9365be145..59ab1ec452 100644 --- a/ddprof-lib/src/main/cpp/javaApi.cpp +++ b/ddprof-lib/src/main/cpp/javaApi.cpp @@ -1181,11 +1181,8 @@ Java_com_datadoghq_profiler_JavaProfiler_resetKlassPopulationForTest0( // recordings (each Profiler::start() clearAll() bumps the generation) // before this test's seeding, and no GC may land between start() and // the seeded selectLeakCandidates() call. After the wipe above there is - // no stale-namespace population data left, and - // classMapGenerationSetForTest() itself invalidates every tracking entry's - // cached_klass_id when the generation changes (the same thing - // cleanup_table()'s reset branch does), so adopting the current - // generation is safe. + // no stale-namespace population data left to protect, so adopting the + // current generation is safe. LivenessTracker::instance()->classMapGenerationSetForTest( Profiler::instance()->classMap()->generation()); } @@ -1242,48 +1239,21 @@ Java_com_datadoghq_profiler_JavaProfiler_pollReferenceChainTargets0( ReferenceChainTracker::instance()->finishLoopIterationSerialized(jvmti, env); } -// Counts only the drained chains whose leaf hop (hops are stored leaf-first) is `leaf`'s class, -// so a chain left cached by an earlier test in the same JVM cannot satisfy the caller. The class -// id is resolved with the same GetClassSignature + normalizeClassSignature() + lookupClass() -// sequence the chain collector uses (LivenessTracker::resolveKlassId()). +// Drains and returns the number of reference-chain events queued by +// pollReferenceChainTargets0() so far (the same queue Profiler::dump() drains in production +// to write datadog.ReferenceChain JFR events) - lets a test assert a chain was actually +// reconstructed without needing a real JFR dump. extern "C" DLLEXPORT jint JNICALL -Java_com_datadoghq_profiler_JavaProfiler_drainReferenceChainEventCountForLeaf0( - JNIEnv *env, jclass unused, jobject leaf) { +Java_com_datadoghq_profiler_JavaProfiler_drainReferenceChainEventCount0( + JNIEnv *env, jclass unused) { ProfiledThread::initCurrentThreadSignalSafe(); jvmtiEnv *jvmti = VM::jvmti(); - if (leaf == nullptr || jvmti == nullptr) { - return 0; - } - u32 leaf_id = 0; - jclass clz = env->GetObjectClass(leaf); - if (clz != nullptr) { - char *sig = nullptr; - if (jvmti->GetClassSignature(clz, &sig, nullptr) == JVMTI_ERROR_NONE && - sig != nullptr) { - const char *name = nullptr; - size_t name_len = 0; - if (ObjectSampler::normalizeClassSignature(sig, &name, &name_len)) { - int id = Profiler::instance()->lookupClass(name, name_len); - if (id > 0) { - leaf_id = (u32)id; - } - } - jvmti->Deallocate((unsigned char *)sig); - } - env->DeleteLocalRef(clz); - } - if (leaf_id == 0) { + if (jvmti == nullptr) { return 0; } std::vector events; ReferenceChainTracker::instance()->drainPendingChainEvents(&events); - jint count = 0; - for (const ReferenceChainEvent &event : events) { - if (!event._hops.empty() && event._hops[0].klass_id == leaf_id) { - count++; - } - } - return count; + return (jint)events.size(); } extern "C" DLLEXPORT void JNICALL @@ -1483,8 +1453,8 @@ Java_com_datadoghq_profiler_JavaProfiler_pollReferenceChainTargets0( JNIEnv *env, jclass unused) {} extern "C" DLLEXPORT jint JNICALL -Java_com_datadoghq_profiler_JavaProfiler_drainReferenceChainEventCountForLeaf0( - JNIEnv *env, jclass unused, jobject leaf) { +Java_com_datadoghq_profiler_JavaProfiler_drainReferenceChainEventCount0( + JNIEnv *env, jclass unused) { return 0; } diff --git a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java index ccd57efee0..aae738a477 100644 --- a/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java +++ b/ddprof-lib/src/main/java/com/datadoghq/profiler/JavaProfiler.java @@ -544,8 +544,16 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l /** * Test seam (debug native builds only - a no-op returning {@code false}/{@code 0}/an - * empty array in release builds): lets LivenessTracker's leak-candidate detection be tested - * without depending on the probabilistic JVMTI heap sampler. + * empty array in release builds). This and every reference-chain/liveness seam below + * (plus {@link #getTid()}) are deliberately package-private: they mutate state of the + * process-wide profiler singleton, so they must not be callable by application code + * sharing the JVM with a debug native build. Tests outside this package reach them only + * through the test-source bridge {@code com.datadoghq.profiler.ReferenceChainTestHooks} + * (ddprof-test), same pattern as {@code ProfilerOwnedBlockHooks}. Collectively the seams + * decouple LivenessTracker's leak-candidate detection from ReferenceChainTracker's chain + * reconstruction, so each is verifiable end-to-end without depending on both the + * probabilistic JVMTI heap sampler and the BFS search organically producing the right + * conditions in the same test run. *

* Enables/disables LivenessTracker's per-klass population tracking directly, * bypassing {@code initialize()}'s live-JVM requirement. Returns {@code true} on @@ -558,28 +566,26 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l * history for {@code klassId} directly into LivenessTracker's ring buffer, * bypassing real allocation sampling. Repeated calls (with distinct * {@code epoch} values) build up a trend {@link #selectLeakCandidateKlassIds0()} - * can then rank. + * can then rank, letting a test assert a slope signal would be generated for a + * chosen klass id without waiting on real GC epochs. */ static native void seedKlassPopulationSample0(int klassId, int count, long epoch); /** - * Test seam (debug native builds only): seeds one epoch's worth of per-tid trend - * history for {@code klassId}, the (klass, tid) qualification - * selectLeakCandidates() requires on top of the klass-level ramp seeded by - * {@link #seedKlassPopulationSample0}. Repeated calls with the same rising shape - * (and the same {@code tid}) qualify {@code tid} as a leak-site thread. {@code tid} - * must come from {@link #getTid()} whenever the scenario relies on leak tagging of - * real tracked instances. + * Test seam (debug native builds only): seeds one epoch's worth of per-tid trend history for + * {@code klassId} - the (klass, tid) qualification selectLeakCandidates() requires on top of + * the klass-level ramp. {@code tid} must be the leaking thread's real profiler tid from + * {@link #getTid()} whenever the scenario also relies on leak tagging of real tracked + * instances. */ static native void seedTidTrendSample0(int klassId, int tid, int count, long epoch); /** * Test seam (debug native builds only): wires {@code representative} in as {@code klassId}'s * leak-candidate representative directly (a fresh weak global ref owned by LivenessTracker), - * bypassing the real allocation-sampling path. Combined with - * {@link #seedKlassPopulationSample0} and {@link #tagAsReferenceChainRoot0}, lets - * {@link #pollReferenceChainTargets0()}'s bridging step run without the real sampler or - * root-seeded walk. + * bypassing the real allocation-sampling path. Lets a test join a synthetic slope signal to + * a real, directly-tagged object so {@link #pollReferenceChainTargets0()}'s bridging step can + * be exercised end-to-end. */ static native void setKlassPopulationRepresentativeForTest0(int klassId, Object representative); @@ -590,10 +596,10 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l static native void resetKlassPopulationForTest0(); /** - * Test seam (debug native builds only): returns the klass ids LivenessTracker's - * real leak-candidate ranking (positive population slope, top 5) currently - * selects - the same call ReferenceChainTracker's restart gate and target-polling - * bridge use in production. Does not require a reference-chain search to be running. + * Test seam (debug native builds only): returns the klass ids LivenessTracker's real + * leak-candidate ranking (positive population slope, top 5) currently selects - the same + * call the restart gate and target-polling bridge use in production, exposed so a test can + * assert a slope signal was generated without a reference-chain search also running. */ static native int[] selectLeakCandidateKlassIds0(); @@ -601,10 +607,12 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l * Test seam (debug native builds only): tags {@code target} and inserts it * directly as a reference-chain frontier root, bypassing ReferenceChainTracker's * normal discovery path (a root-seeded FollowReferences walk) and - * LivenessTracker's leak-candidate selection entirely. Returns the assigned - * frontier tag (matching the {@code target_tag} a resulting - * {@code datadog.ReferenceChain} event reports), or {@code 0} on failure (reference - * chains disabled, or the frontier table is at capacity). + * LivenessTracker's leak-candidate selection entirely. Lets a test drive + * {@link #runReferenceChainPass0()}/{@link #pollReferenceChainTargets0()} against + * a known, caller-chosen live object. Returns the assigned frontier tag (matching + * the {@code target_tag} a resulting {@code datadog.ReferenceChain} event + * reports), or {@code 0} on failure (reference chains disabled, or the frontier + * table is at capacity). */ static native long tagAsReferenceChainRoot0(Object target); @@ -626,19 +634,21 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l static native void pollReferenceChainTargets0(); /** - * Test seam (debug native builds only): drains the reference-chain events queued by - * {@link #pollReferenceChainTargets0()} so far (the same queue {@code Profiler.dump()} drains - * in production to write {@code datadog.ReferenceChain} JFR events) and returns how many of - * them have {@code leaf}'s class as their leaf (first) hop. Chains for other targets, e.g. left - * cached by an earlier test in the same JVM, are not counted. + * Test seam (debug native builds only): drains and returns the number of + * reference-chain events queued by {@link #pollReferenceChainTargets0()} so far + * (the same queue {@code Profiler.dump()} drains in production to write + * {@code datadog.ReferenceChain} JFR events) - lets a test assert a chain was + * actually reconstructed without needing a real JFR dump. */ - static native int drainReferenceChainEventCountForLeaf0(Object leaf); + static native int drainReferenceChainEventCount0(); /** * Test seam (debug native builds only): resets ReferenceChainTracker's search/frontier state - * back to a brand-new tracker's, releasing any tags a previous search still held. The tracker - * is a process-wide singleton and runPass() walks from the roots only once per search, so an - * in-process test needing its own root-seeded walk must call this first. + * back to a brand-new tracker's, releasing any tags a previous search still held. Since the + * tracker is a process-wide singleton, an in-process test that needs its own genuine first + * root-seeded walk (runPass() only re-walks from the roots once per search's whole lifetime) + * calls this at the start of its test body to force one, rather than depending on being the + * first reference-chain test to run in a shared test JVM. */ static native void resetReferenceChainSearchForTest0(); @@ -684,15 +694,17 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l * Test seam (debug native builds only): seeds one heap-floor-ring sample - the input to * {@code LivenessTracker::secondsToOOM()}'s time-to-OOM projection - directly, bypassing the * real {@code GarbageCollectionFinish} callback. {@code timestampNs} values are only ever - * compared against each other, never against a real wall clock, so any strictly increasing - * sequence is valid. + * compared against each other, never against a real wall clock, so a test may use any + * self-consistent, strictly increasing sequence to build an arbitrary rising or flat + * heap-usage-over-time history without waiting on real GCs. */ static native void heapFloorRecordForTest0(long usedBytes, long timestampNs); /** * Test seam (debug native builds only): overrides the max-heap-size {@code secondsToOOM()} - * projects against, bypassing the real {@code Runtime.maxMemory()} resolution, so the - * projection does not depend on the test JVM's {@code -Xmx}. + * projects against, bypassing the real {@code Runtime.maxMemory()} resolution - so a test can + * exercise the projection deterministically, independent of whatever {@code -Xmx} this JVM's + * own shared, no-{@code forkEvery} fork happens to run with. */ static native void setMaxHeapBytesForTest0(long maxHeapBytes); @@ -730,20 +742,23 @@ private static native void setTraceContext0(long localRootSpanId, long spanId, l * Test seam (debug native builds only): reports whether ReferenceChainTracker's search-restart * gate ({@code canAffordNewSearch()} -> {@code hasLeakSignal()}) would currently allow a * fresh/terminal search to start - in particular, whether {@code secondsToOOM()}'s urgent-OOM - * bypass opens this gate even with zero per-klass leak candidates. Unlike - * {@link #runReferenceChainPass0()}, which calls {@code runPass()} unconditionally, this - * evaluates the gate without running a pass. This is not a read-only query: under the native - * engine lock it may account a completed search's pain and restart/reset terminal search - * state. + * bypass opens this gate even with zero per-klass leak candidate (confirmable in the same test + * via {@link #selectLeakCandidateKlassIds0()}). Unlike {@link #runReferenceChainPass0()}, which + * calls {@code runPass()} unconditionally, this evaluates the real gate without executing a + * BFS pass - but it is NOT a read-only query: under the native engine lock it may account a + * completed search's pain and restart/reset terminal search state, and opening the gate on + * the urgent-OOM bypass spends the episode's one-shot urgency entitlement. Do not treat it + * as a pure predicate (e.g. inside an assertion) when a scenario's search state matters. */ static native boolean shouldRunPassForTest0(); /** * Test seam (debug native builds only): the number of BFS passes run for the current/most - * recent reference-chain search. Waiting for it to advance past the value read before creating - * an object guarantees a later match came from a pass whose {@code expandFrontier()} (and - * therefore {@code collectStaleExpandedEntriesForRotation()}) ran strictly after the object - * existed, not from a pass racing its creation. + * recent reference-chain search. Lets a test note this count before creating an object, then + * wait for it to advance before trusting a match against that object - the only way to be + * certain the match came from a pass whose own {@code expandFrontier()} (and therefore {@code + * collectStaleExpandedEntriesForRotation()}) ran strictly after the object existed, rather + * than from the same pass racing the object's creation. */ static native int referenceChainPassesRunForTest0(); diff --git a/ddprof-lib/src/test/cpp/referenceChainJfrRoundtrip_ut.cpp b/ddprof-lib/src/test/cpp/referenceChainJfrRoundtrip_ut.cpp index b69987128f..2f56827d37 100644 --- a/ddprof-lib/src/test/cpp/referenceChainJfrRoundtrip_ut.cpp +++ b/ddprof-lib/src/test/cpp/referenceChainJfrRoundtrip_ut.cpp @@ -238,6 +238,8 @@ class ReferenceChainJfrRoundtripTest : public ::testing::Test { // same DDPROF_REFERENCE_CHAIN_ROUNDTRIP_JFR to both sides (ddprof-lib and // ddprof-test build.gradle.kts) and deletes the file before this gtest runs; // the OS temp dir is only a fallback for running the binary by hand. +// The file is consume-once: ProducesValidStandaloneJfrWithChainEvent unlinks it before +// writing and the Java test deletes it after reading. static std::string chainRoundtripJfrPath() { const char *configured = getenv("DDPROF_REFERENCE_CHAIN_ROUNDTRIP_JFR"); if (configured != nullptr && *configured != 0) { @@ -280,6 +282,10 @@ static u32 decodeVar32(const char *data, int *off) { } TEST_F(ReferenceChainJfrRoundtripTest, ProducesValidStandaloneJfrWithChainEvent) { + // Drop any fixture left by an earlier (possibly different-revision) run before any ASSERT + // below can bail out, so a failed run never leaves a stale file for the Java parser test + // (ReferenceChainJfrParserTest) to validate. ENOENT is expected and ignored. + unlink(chainRoundtripJfrPath().c_str()); Arguments args; ASSERT_FALSE(args.parse("referencechains=true:hops=64:budget=1000")); // Skip OS/CPU info, JVM info, system properties and native library @@ -321,9 +327,9 @@ TEST_F(ReferenceChainJfrRoundtripTest, ProducesValidStandaloneJfrWithChainEvent) // serializes into the T_CLASS checkpoint - this is the exact API // Profiler::lookupClass() already-existing tests use (e.g. // referenceChains_ut.cpp's ReconstructsChainForSyntheticGraph). - // The constants feed both the lookup and the .expected sidecar below, so - // the companion Java parser test reads the seeded values from a single - // source instead of duplicating them. + // The constants feed the lookup below; the companion Java parser test + // (ReferenceChainJfrParserTest) hardcodes the same values, so a change to + // the seeding must update both sides - a deliberate two-module diff. static constexpr const char *kLeafClass = "com/test/ChainLeaf"; static constexpr const char *kMiddleClass = "com/test/ChainMiddle"; static constexpr const char *kRootClass = "com/test/ChainRoot"; @@ -394,28 +400,9 @@ TEST_F(ReferenceChainJfrRoundtripTest, ProducesValidStandaloneJfrWithChainEvent) // same finalization every real recording chunk goes through. } - // The values this test seeds, written next to the recording: the companion - // Java parser test (ReferenceChainJfrParserTest) asserts against this - // sidecar instead of hard-coding them, so a change to the seeding shows - // up as a deliberate gtest-only diff rather than an opaque assertion - // failure in the other module. The Java side maps the slashed internal - // names to JMC's dotted IMCType full names. - { - std::string expectedPath = path + ".expected"; - FILE *expected = fopen(expectedPath.c_str(), "w"); - ASSERT_NE(nullptr, expected) << "could not open " << expectedPath; - fprintf(expected, "chain.length=%d\n", 3); - fprintf(expected, "chain.0=%s\n", kLeafClass); - fprintf(expected, "chain.1=%s\n", kMiddleClass); - fprintf(expected, "chain.2=%s\n", kRootClass); - fprintf(expected, "edges.0=field\n"); - fprintf(expected, "edges.1=element\n"); - fprintf(expected, "edges.2=jni_global\n"); - fprintf(expected, "targetTag=%lld\n", (long long)event._target_tag); - fprintf(expected, "depth=%u\n", event._depth); - fprintf(expected, "leakTagIncarnation=%u\n", event._leak_tag_incarnation); - fclose(expected); - } + // No .expected sidecar: the Java parser test hardcodes the seeded values + // (chain names, edges labels, targetTag=3, depth=2), so writing a sidecar + // nothing reads would only leave a stale file in the temp/build dir. FILE *f = fopen(path.c_str(), "rb"); ASSERT_NE(nullptr, f) << "expected " << path << " to have been written"; diff --git a/ddprof-stresstest/build.gradle.kts b/ddprof-stresstest/build.gradle.kts index 550baee43b..67896f2b7d 100644 --- a/ddprof-stresstest/build.gradle.kts +++ b/ddprof-stresstest/build.gradle.kts @@ -101,6 +101,45 @@ tasks.register("chaosJar") { } } +// --- reference-chains repro app --------------------------------------------- +// Plain, dependency-free demo app for manually reproducing the reference-chains +// feature: launched directly with `-agentpath:libjavaProfiler.so=start,...` +// (no dd-trace-java agent, no dynamic attach), so it deliberately has zero +// compile/runtime dependency on ddprof-lib or dd-trace - the profiler is +// entirely out-of-process from this app's own point of view. + +sourceSets { + create("repro") +} + +dependencies { + // ddprof-lib's Java API only - not the native library itself (that's the one + // already loaded in-process via -agentpath). Used solely to call + // JavaProfiler.getInstance().dump(...) periodically: Profiler::dump() + // (profiler.cpp) is the only code path that drains ReferenceChainTracker's + // pending chain-event queue and actually writes datadog.ReferenceChain into + // the JFR file - nothing does this automatically on a timer, and in + // production it's dd-trace-java's own recording-chunk rotation that calls + // it. Without this dependency+call, chain events get built and enqueued + // (visible in TEST_LOG output) but are silently dropped when the process + // exits, and the JFR file never shows a single datadog.ReferenceChain event. + "reproImplementation"(project(mapOf("path" to ":ddprof-lib", "configuration" to "debug"))) +} + +tasks.register("reproJar") { + group = "build" + description = "Demo app that leaks memory in a controlled way, for manually reproducing reference-chains via -agentpath" + archiveFileName.set("refchains_repro.jar") + from(sourceSets["repro"].output) + from({ + configurations["reproRuntimeClasspath"].map { if (it.isDirectory) it else zipTree(it) } + }) + duplicatesStrategy = DuplicatesStrategy.EXCLUDE + manifest { + attributes("Main-Class" to "com.datadoghq.profiler.repro.ReferenceChainLeakDemo") + } +} + tasks.register("runStressTests") { dependsOn(tasks.named("jmhJar")) diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java index 7e846c02d1..a8e51e8666 100644 --- a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/Main.java @@ -92,6 +92,8 @@ private static Antagonist create(String name) { return new WeakRefWaveAntagonist(); case "dump-storm": return new DumpStormAntagonist(); + case "reference-chain-leak": + return new ReferenceChainLeakAntagonist(); case "reapply-context-value": return new ReapplyContextValueAntagonist(); // Deferred: dlopen-churn (needs per-arch dummy .so built in CI prep). diff --git a/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReferenceChainLeakAntagonist.java b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReferenceChainLeakAntagonist.java new file mode 100644 index 0000000000..99db2303f1 --- /dev/null +++ b/ddprof-stresstest/src/chaos/java/com/datadoghq/profiler/chaos/ReferenceChainLeakAntagonist.java @@ -0,0 +1,132 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + */ +package com.datadoghq.profiler.chaos; + +import java.time.Duration; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Grows an ever-referenced cache to give the reference-chains + * feature (LivenessTracker/ReferenceChainTracker) a real, steadily growing + * population to detect and walk. Memory stays bounded: once the live entry + * count reaches {@link #CAP_ENTRIES} the antagonist switches to a bounded + * recycle cycle - it evicts the oldest {@link #RECYCLE_EVICT_ENTRIES} entries + * and then regrows to the cap - so the population keeps going through rising + * legs for the whole run instead of freezing at a flat count (a cache that + * grows to a fixed size and then sits flat stops being a growing population: + * the trend detection's earliest-vs-recent ring comparison reads a ~zero slope + * and the feature gets no leak signal for the rest of a multi-hour run). The + * recycle fraction keeps at least 3/4 of the cap alive at all times, so a long + * chaos duration still produces a bounded leak instead of eventually OOMing + * the JVM out from under the other antagonists sharing its heap. + * + *

Same object shape as {@code LeakingCacheScenario.CachedPayload} + * (ddprof-test's reference-chain integration test) — plain long fields + * rather than a nested array, so a single instance clears the allocation + * sampler's real size floor without a second, competing heap allocation per + * entry. + */ +public final class ReferenceChainLeakAntagonist implements Antagonist { + + // ~64MiB of CachedEntry payloads at the cap (entries are a little + // over 300 bytes with object header/field overhead) — enough for the + // liveness tracker's population-growth trend detection to have a real, + // sustained trend to lock onto, without meaningfully competing with the + // other antagonists' own memory budget for the run's whole duration. + private static final int CAP_ENTRIES = 200_000; + + // Entries evicted per recycle cycle once the cap is reached (25% of the + // cap): large enough that each regrow leg is a visible rising population + // trend, small enough that 3/4 of the population stays alive at all times. + private static final int RECYCLE_EVICT_ENTRIES = 50_000; + + private static final int BATCH_SIZE = 200; + private static final long BATCH_INTERVAL_MS = 50L; + + private final Map cache = new ConcurrentHashMap<>(); + private volatile boolean running; + private Thread growthDriver; + private long nextKey; + private long evictKey; + private long liveEntries; + + @Override + public String name() { + return "reference-chain-leak"; + } + + @Override + public void start() { + running = true; + growthDriver = new Thread(this::growthLoop, "chaos-refchain-leak"); + growthDriver.setDaemon(true); + growthDriver.start(); + } + + @Override + public void stopGracefully(Duration timeout) { + running = false; + try { + growthDriver.join(timeout.toMillis()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + + private void growthLoop() { + // liveEntries, not cache.size(): ConcurrentHashMap.size() sums every counter cell, too + // costly per put/remove. Exact here - this thread is the only writer, keys are unique + // and evictions only remove what a previous put added, so the counter always matches. + while (running) { + if (liveEntries >= CAP_ENTRIES) { + // Cap reached: recycle phase - evict the oldest entries, then the loop's + // growth phase regrows them with fresh keys, giving the population trend + // detection a repeated rising signal instead of a flat count. + long evictTarget = Math.min(evictKey + RECYCLE_EVICT_ENTRIES, nextKey); + for (; running && evictKey < evictTarget; evictKey++) { + cache.remove("refchain-leak-" + evictKey); + liveEntries--; + MemoryGovernor.pace(); + } + } + while (running && liveEntries < CAP_ENTRIES) { + for (int i = 0; i < BATCH_SIZE && liveEntries < CAP_ENTRIES; i++) { + String key = "refchain-leak-" + (nextKey++); + cache.put(key, new CachedEntry(key)); + liveEntries++; + MemoryGovernor.pace(); + } + try { + Thread.sleep(BATCH_INTERVAL_MS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + try { + Thread.sleep(BATCH_INTERVAL_MS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } + } + + static final class CachedEntry { + final String key; + long p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12, p13, p14, p15; + long p16, p17, p18, p19, p20, p21, p22, p23, p24, p25, p26, p27, p28, p29, p30, p31; + + CachedEntry(String key) { + this.key = key; + } + } +} diff --git a/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java b/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java new file mode 100644 index 0000000000..9415bb57a1 --- /dev/null +++ b/ddprof-stresstest/src/repro/java/com/datadoghq/profiler/repro/ReferenceChainLeakDemo.java @@ -0,0 +1,327 @@ +/* + * Copyright 2026, Datadog, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + */ +package com.datadoghq.profiler.repro; + +import com.datadoghq.profiler.JavaProfiler; +import java.io.IOException; +import java.lang.management.ManagementFactory; +import java.lang.management.MemoryUsage; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.Map; + +/** + * Standalone demo app for manually reproducing the reference-chains feature end-to-end. + * Meant to be launched directly with the native agent loaded at JVM startup, e.g. + * + * 

+ * java -agentpath:/path/to/libjavaProfiler.so=start,memory=64:l,generations=true,\
+ *   referencechains=true:hops=64:budget=10000:ttl=120000:framecap=2000000:\
+ *   pausetarget=5000:painbudget=100,jfr,file=repro.jfr \
+ *   -jar refchains_repro.jar repro.jfr /path/to/libjavaProfiler.so
+ *
+ * + *

Takes the JFR output path as {@code args[0]} (must match {@code file=} above) and the + * agent's own {@code .so} path as {@code args[1]} (must match the {@code -agentpath} value + * above), and periodically calls {@link JavaProfiler#dump}. Both are load-bearing: the dump call + * is the only code path that drains {@code ReferenceChainTracker}'s pending chain-event queue + * into the JFR file (nothing drains it on a timer), and passing the {@code .so} path explicitly + * makes {@code getInstance()} attach to the already-loaded {@code -agentpath} agent instead of + * extracting the jar's bundled copy to a temp file - a second, never-{@code start()}-ed + * {@code Profiler} singleton whose {@code dump()} calls would be silent no-ops. Each periodic + * dump goes to its own file, {@code .snapshot.}, because a drained chain + * event is written exactly once: reusing one snapshot path would permanently lose every earlier + * window's chains. A bounded run's final dump goes to {@code .snapshot} (the path + * the sweep scripts read); inspect all snapshot files together for the complete chain evidence. + * + *

An optional {@code args[2]} duration in seconds switches from the default "run forever" + * manual mode to a bounded session that dumps once more and prints a single {@code [metrics]} + * line at exit (throughput, round latency, heap growth) - see + * {@code utils/compare-refchains-repro.sh}, which runs this app twice (referencechains on vs. + * off) over the same duration and diffs those lines against each run's safepoint/GC logs. + *

+ *

Deliberately omits {@code firstpassbudget}: the tracker auto-scales its first pass's budget + * from the plain {@code budget} value ({@code AUTO_FIRST_PASS_BUDGET_MULTIPLIER}, capped at + * {@code AUTO_FIRST_PASS_BUDGET_CAP}, referenceChains.h) rather than reusing {@code budget} as-is + * for that first, cold root-seeded walk - reusing it directly truncates every pass before it gets + * anywhere near this app's {@code CachedEntry} population. Pass {@code firstpassbudget=N} + * explicitly only to override that auto-scaled default. + * + *

Grows an ever-referenced, never-evicted {@code HashMap}-backed cache (same shape as + * ddprof-test's {@code LeakingCacheScenario}). Two deliberate choices, both found by running + * without them and never seeing a chain get discovered: + * + *

    + *
  • Explicit {@code System.gc()} every round. {@code LivenessTracker}'s + * population-growth trend needs at least {@code KLASS_POPULATION_MIN_FILL_FOR_TREND} (10) + * distinct GC epochs of samples before it attempts a slope, and {@code _gc_epoch} only advances + * on a {@code GarbageCollectionFinish} callback. A slow allocation rate against a large default + * heap can go many minutes without a GC, so the trend never moves on its own.
    • + *
  • No hard stop. A cache that grows to a fixed cap and then sits flat stops being a + * growing population the instant it stops growing - {@code computeKlassPopulationSlope} compares + * the earliest vs. most recent third of its 30-epoch ring, so a long enough flat period pushes + * the slope back to zero. This app keeps growing forever, self-throttling against a heap-usage + * watermark (see {@link #run}) so the trend stays real without ever exhausting the heap.
    • + *
+ * + *

Seeds the cache on the very first line of {@code main} - before validating arguments, + * loading/attaching to the agent library, printing anything, or sleeping - so the agent's one-shot, root-seeded first pass (which fires roughly a second after + * the agent's background thread starts, i.e. before {@code Agent_OnLoad} even returns control to + * {@code main}) has a real chance of finding {@code cache} already non-empty. See + * {@code LeakingCacheScenario}'s comment for why that ordering is load-bearing. + */ +public final class ReferenceChainLeakDemo { + + // Heap-usage watermarks gating growth pace, mirroring the chaos harness's own + // MemoryGovernor - a much simpler version since this app has only one grower to pace, + // not several antagonists sharing a budget. + private static final double HIGH_WATERMARK = 0.70; + private static final double CRITICAL_WATERMARK = 0.85; + + private static final int NORMAL_BATCH_SIZE = 2_000; + private static final int THROTTLED_BATCH_SIZE = 100; + private static final long ROUND_INTERVAL_MS = 300L; + private static final long THROTTLED_ROUND_INTERVAL_MS = 2_000L; + + // How often to call JavaProfiler.dump() to drain any pending reference-chain + // events into the JFR file - see this class's own header comment for why this + // call, not just this app's own memory growth, is load-bearing for the repro. + private static final int DUMP_EVERY_N_ROUNDS = 10; + + // Periodic snapshot files kept in the default run-forever mode (see the dump block in + // run()): older periodic snapshots are deleted once this many newer ones exist, so a + // multi-hour manual session cannot fill /tmp with full-window dumps. + private static final int KEEP_PERIODIC_SNAPSHOTS = 5; + + private static final String USAGE = + "usage: ReferenceChainLeakDemo [duration-seconds] " + + "(both required args must match the -agentpath:=...jfr,file= values; " + + "duration-seconds - a non-negative whole number of seconds - runs a bounded, " + + "metrics-reporting session instead of forever)"; + + public static void main(String[] args) throws Exception { + // Must stay first - see the class header: seeding races the agent's one-shot, + // root-seeded first pass. + Map cache = new HashMap<>(); + seed(cache, 300); + if (args.length < 2) { + System.err.println(USAGE); + System.exit(1); + } + long durationSeconds = args.length >= 3 ? parseDurationSeconds(args[2]) : -1; + run(cache, Paths.get(args[0]), args[1], durationSeconds); + } + + // Malformed input (e.g. an empty/garbled $DURATION from the wrapper scripts) must print the + // usage and exit non-zero, not die with an uncaught NumberFormatException. Also rejects + // values whose deadline (seconds * 1e9 ns, see run()) would overflow a long. + private static long parseDurationSeconds(String arg) { + long seconds; + try { + seconds = Long.parseLong(arg.trim()); + } catch (NumberFormatException e) { + seconds = -1; + } + if (seconds < 0 || seconds > Long.MAX_VALUE / 1_000_000_000L) { + System.err.println("invalid duration-seconds '" + arg + "'"); + System.err.println(USAGE); + System.exit(1); + } + return seconds; + } + + // FlightRecorder::dump() (flightRecorder.cpp) rejects dumping the continuous recording to + // its own file= path ("Can not dump recording to itself"), so drains must target a + // different path; this is that path's suffix. Periodic drains append "." so each + // window's drained chain events survive (see this class's header comment); the final + // bounded-run drain uses the bare suffix. + private static final String SNAPSHOT_SUFFIX = ".snapshot"; + + // Tracked so a bounded run (durationSeconds >= 0) can report throughput/latency/memory + // deltas at exit for A/B comparison against a referencechains=false baseline - see + // utils/compare-refchains-repro.sh. + private static long roundCount = 0; + private static long totalEntriesAdded = 0; + private static long totalRoundNanos = 0; + private static long maxRoundNanos = 0; + + private static void run(Map cache, Path recordingPath, String agentSoPath, long durationSeconds) throws InterruptedException, IOException { + // Must differ from recordingPath itself - see SNAPSHOT_SUFFIX's comment. Final + // (bounded-run) drain target; periodic drains use snapshotPath + "." + round. + Path snapshotPath = recordingPath.resolveSibling(recordingPath.getFileName() + SNAPSHOT_SUFFIX); + // Must pass the exact .so path -agentpath already loaded: JavaProfiler.getInstance() + // with no libLocation extracts the jar's own bundled copy to a fresh temp file and + // System.load()s *that* instead, producing a second, never-started Profiler singleton + // completely disconnected from the real profiling session - dump() calls against it + // are silent no-ops. Passing the same path here lets the dynamic linker dedup the + // load to the already-mapped library, i.e. the real, already-running singleton. + JavaProfiler profiler; + try { + profiler = JavaProfiler.getInstance(agentSoPath, null); + } catch (Exception e) { + throw new IllegalStateException( + "JavaProfiler.getInstance() failed - was this launched with " + + "-agentpath:libjavaProfiler.so=start,...?", e); + } + + System.out.println("[repro] pid=" + pid() + + " - growing cache " + (durationSeconds >= 0 ? ("for " + durationSeconds + "s") : "forever") + + ", forcing a GC every round so the profiler's " + + "population-growth trend detection has real epochs to work with; watch " + + "heapUsedMb/heapMaxMb and attach/dump once a chain shows up"); + + long startNanos = System.nanoTime(); + long deadlineNanos = durationSeconds >= 0 ? startNanos + durationSeconds * 1_000_000_000L : Long.MAX_VALUE; + long startHeapUsedMb = heapUsedMb(); + + long nextKey = 300; + int round = 0; + while (System.nanoTime() < deadlineNanos) { + long roundStartNanos = System.nanoTime(); + round++; + double heapFraction = heapUsedFraction(); + int batchSize = heapFraction >= HIGH_WATERMARK ? THROTTLED_BATCH_SIZE : NORMAL_BATCH_SIZE; + long roundIntervalMs = heapFraction >= HIGH_WATERMARK ? THROTTLED_ROUND_INTERVAL_MS : ROUND_INTERVAL_MS; + + if (heapFraction < CRITICAL_WATERMARK) { + for (int i = 0; i < batchSize; i++) { + String key = "leak-" + (nextKey++); + cache.put(key, new CachedEntry(key)); + } + totalEntriesAdded += batchSize; + } else { + System.out.println("[repro] heap usage critical (" + pct(heapFraction) + + ") - pausing growth this round, attach/dump now if you haven't"); + } + + // Forces a GC every round regardless of watermark state, so _gc_epoch (and + // therefore the population-growth ring) keeps advancing even while paused/ + // throttled - see this class's own header comment for why that's load-bearing. + System.gc(); + + if (round % 10 == 0 || heapFraction >= HIGH_WATERMARK) { + System.out.println("[repro] round=" + round + " cache.size()=" + cache.size() + + " heapUsedMb=" + heapUsedMb() + " heapFraction=" + pct(heapFraction)); + } + + if (round % DUMP_EVERY_N_ROUNDS == 0) { + // Distinct file per drain: an event drained here is written only to this file. + Path roundSnapshotPath = + snapshotPath.resolveSibling(snapshotPath.getFileName() + "." + round); + profiler.dump(roundSnapshotPath); + // Run-forever mode writes a new snapshot every DUMP_EVERY_N_ROUNDS rounds and + // each dump contains the full recording window, so unbounded retention fills + // /tmp during long manual repro sessions. Keep only the most recent + // KEEP_PERIODIC_SNAPSHOTS periodic snapshots; the bounded-run .snapshot dump + // (the path the sweep scripts read) is never touched. + if (durationSeconds < 0) { + long expireRound = round - (long) KEEP_PERIODIC_SNAPSHOTS * DUMP_EVERY_N_ROUNDS; + if (expireRound > 0) { + Files.deleteIfExists(snapshotPath.resolveSibling( + snapshotPath.getFileName() + "." + expireRound)); + } + } + System.out.println("[repro] dumped " + roundSnapshotPath); + } + + long roundNanos = System.nanoTime() - roundStartNanos; + roundCount++; + totalRoundNanos += roundNanos; + maxRoundNanos = Math.max(maxRoundNanos, roundNanos); + + Thread.sleep(roundIntervalMs); + } + + if (durationSeconds >= 0) { + profiler.dump(snapshotPath); + System.out.println("[repro] dumped " + snapshotPath); + reportMetrics(startNanos, startHeapUsedMb); + } + } + + // Printed with a distinct "[metrics]" prefix so utils/compare-refchains-repro.sh can grep + // it out of the two runs' stdout (referencechains=true vs. false) and diff throughput/ + // latency/memory-growth directly, without needing to touch the safepoint/GC logs for those. + private static void reportMetrics(long startNanos, long startHeapUsedMb) { + long wallNanos = System.nanoTime() - startNanos; + double wallSeconds = wallNanos / 1_000_000_000.0; + double avgRoundMs = roundCount == 0 ? 0 : (totalRoundNanos / (double) roundCount) / 1_000_000.0; + double maxRoundMs = maxRoundNanos / 1_000_000.0; + double entriesPerSec = wallSeconds > 0 ? totalEntriesAdded / wallSeconds : 0; + long heapGrowthMb = heapUsedMb() - startHeapUsedMb; + + // Locale.ROOT: a comma-decimal default locale would emit 0,450, which + // the parsing scripts' [0-9.]+ patterns silently read as zero. + System.out.println("[metrics] wallSeconds=" + String.format(java.util.Locale.ROOT, "%.1f", wallSeconds) + + " rounds=" + roundCount + + " entriesAdded=" + totalEntriesAdded + + " entriesPerSec=" + String.format(java.util.Locale.ROOT, "%.1f", entriesPerSec) + + " avgRoundMs=" + String.format(java.util.Locale.ROOT, "%.3f", avgRoundMs) + + " maxRoundMs=" + String.format(java.util.Locale.ROOT, "%.3f", maxRoundMs) + + " heapUsedMbStart=" + startHeapUsedMb + + " heapUsedMbEnd=" + heapUsedMb() + + " heapGrowthMb=" + heapGrowthMb); + } + + private static void seed(Map cache, int count) { + for (int i = 0; i < count; i++) { + String key = "seed-" + i; + cache.put(key, new CachedEntry(key)); + } + } + + private static String pid() { + String name = ManagementFactory.getRuntimeMXBean().getName(); + int at = name.indexOf('@'); + return at >= 0 ? name.substring(0, at) : name; + } + + private static long heapUsedMb() { + return ManagementFactory.getMemoryMXBean().getHeapMemoryUsage().getUsed() / (1024 * 1024); + } + + private static double heapUsedFraction() { + MemoryUsage heap = ManagementFactory.getMemoryMXBean().getHeapMemoryUsage(); + long max = heap.getMax(); + if (max <= 0) { + // Heap max undefined (-1) in some container/ergonomics setups: fall back to + // Runtime.maxMemory(), and if that is unbounded too (Long.MAX_VALUE), report the high + // watermark so growth runs throttled - never treat "unknown limit" as "empty heap". + long runtimeMax = Runtime.getRuntime().maxMemory(); + if (runtimeMax <= 0 || runtimeMax == Long.MAX_VALUE) { + return HIGH_WATERMARK; + } + max = runtimeMax; + } + return (double) heap.getUsed() / (double) max; + } + + private static String pct(double fraction) { + return String.format("%.1f%%", fraction * 100.0); + } + + /** + * Same shape as {@code LeakingCacheScenario.CachedPayload}/{@code + * ReferenceChainLeakAntagonist.CachedEntry}: plain long fields, not a nested array, so a + * single instance clears the allocation sampler's real size floor without a second, + * competing heap allocation per entry. + */ + static final class CachedEntry { + final String key; + long p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12, p13, p14, p15; + long p16, p17, p18, p19, p20, p21, p22, p23, p24, p25, p26, p27, p28, p29, p30, p31; + + CachedEntry(String key) { + this.key = key; + } + } +} diff --git a/ddprof-test/quarantine.txt b/ddprof-test/quarantine.txt index 6944b1e105..288dbf0f05 100644 --- a/ddprof-test/quarantine.txt +++ b/ddprof-test/quarantine.txt @@ -71,3 +71,4 @@ com.datadoghq.profiler.nativesocket.NativeSocketRemoteAddressTest.* | PROF-16014 com.datadoghq.profiler.nativesocket.NativeSocketRestartTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner com.datadoghq.profiler.nativesocket.NativeSocketSendRecvSeparateTest.* | PROF-16014 | 2026-09-21 | 2026-12-20 | el7-8-release-amd64 | NativeSocketEvent count 0: doTcpTransfer() never blocks long enough to be sampled on this runner 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 diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java index e0f79c7046..78ea02983f 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProcessProfilerTest.java @@ -60,8 +60,10 @@ protected final LaunchResult launch(String target, List jvmArgs, String /** * Same as the 6-arg overload above, but with a caller-supplied wait timeout instead of the - * default 10 seconds, for child scenarios that run substantially longer than an attach/init - * check. + * hardcoded 10 seconds - needed by scenarios that run substantially longer than a plain + * attach/init check, e.g. {@code ExternalProcessReferenceChainTest}'s population-growth loop + * (up to 25 rounds plus a grace period, ~20s+ observed in-process, plus a whole separate + * JVM's own startup/classloading cost on top). */ protected final LaunchResult launch(String target, List jvmArgs, String commands, Map env, long timeoutSeconds, Function onStdoutLine, Function onStderrLine) throws Exception { String javaHome = System.getenv("JAVA_TEST_HOME"); diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java index acecb8fef4..7e2f880f08 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java @@ -74,8 +74,10 @@ public static double scaledSize(JfrEvent item) { protected JavaProfiler profiler; private Path jfrDump; - // Set before getProfilerCommand() is consulted, so a subclass can vary its - // command per @Test method instead of needing one test class per configuration. + // Set at the very start of setupProfiler(), before getProfilerCommand() is + // consulted, so a subclass can branch its command on the current test + // method (e.g. distinct hop/budget/frontier-cap values per @Test) without + // needing separate test classes per configuration. protected TestInfo testInfo; private Duration cpuInterval; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java index 6681ddc1d8..b22c6f42e4 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ExternalLauncher.java @@ -38,18 +38,26 @@ * holds samples rooted at each of the three thread entry points; see {@code EntryFrameTest} *

  • leak-cache "<start command>|||<scratch dump path>" - starts the profiler with * the given start command, then runs {@link LeakingCacheScenario} in this process and exits - * directly (no readiness/stdin-signal handshake, unlike the other modes above) once it - * prints its one result line. Used by {@code ExternalProcessReferenceChainTest}; a separate - * JVM is required because the one-shot root-seeded BFS walk is a process-wide, once-ever - * resource.
    • + * directly once it prints its one result line (no readiness/stdin-signal handshake, unlike + * the modes above). Used by {@code ExternalProcessReferenceChainTest} to prove the + * reference-chains mechanism against a genuinely separate JVM - a separate process is + * load-bearing there because the one-shot root-seeded BFS walk is a process-wide, + * once-ever resource. *
  • leak-static-field "<start command>|||<scratch dump path>" - same packing and - * lifecycle as {@code leak-cache} above, but runs {@link StaticFieldGrowingCollectionScenario} - * instead: a {@code static final List} field appended to (never reassigned) after - * its owning class has already been swept once by {@code admitStaticFieldRoots()}.
    • + * lifecycle as {@code leak-cache} above, but runs {@link StaticFieldGrowingCollectionScenario}: + * a {@code static final List} field appended to (never reassigned) after its owning + * class has already been swept once by {@code admitStaticFieldRoots()} - mirroring the real + * leak generator found in the {@code prof-analyzer-hotdog-jb} pod. *
  • leak-correlation "<start command>|||<scratch dump path>" - same packing and - * lifecycle as {@code leak-cache} above, but runs {@link LeakTagCorrelationScenario} - * instead, asserting the emitted chain's targetTag is a leak-tag-pool tag matching a + * lifecycle as {@code leak-cache} above, but runs {@link LeakTagCorrelationScenario}: a + * static-collection leak coexisting with ephemeral stack-local noise and a large live filler + * graph, asserting the emitted chain's targetTag is a leak-tag-pool tag matching a * HeapLiveObject's leakTag.
    • + *
  • threadlocal-leak / threadlocal-leak-late "<start command>|||<scratch dump path>" - + * same packing and lifecycle as {@code leak-cache} above, but runs + * {@link ThreadLocalLeakScenario}: a growing {@code byte[]} collection held only through the + * leaking thread's {@code ThreadLocal} map; the {@code -late} variant starts the leak thread + * only after the recording began (see that scenario's own comment).
    • * */ public class ExternalLauncher { @@ -236,15 +244,20 @@ public static void main(String[] args) throws Exception { } } } else if (args[0].equals("leak-cache")) { + // "|||" packed into one args[1] string rather + // than extending launch()'s generic (target, commands) contract with a 3rd + // argument every other mode would have to ignore. String[] packed = unpackScenarioArgs("leak-cache", args); String commands = packed[0]; String scratchPath = packed[1]; JavaProfiler instance = JavaProfiler.getInstance(); - // run() starts the profiler itself: it must seed its cache fixture first. + // The scenario itself calls execute(commands) - it needs to seed its cache fixture + // *before* starting the profiler. LeakingCacheScenario.run(instance, commands, Paths.get(scratchPath)); System.out.flush(); - // Skips the "[ready]" + stdin handshake below: the mode is one-shot and its - // result line has already been printed. + // Exits here rather than falling through to the shared "[ready]" + stdin-signal + // handshake: this mode runs to completion in one shot and has already printed its + // result line. System.exit(0); } else if (args[0].equals("leak-static-field")) { // Same packing as leak-cache (unpackScenarioArgs()). @@ -274,9 +287,10 @@ public static void main(String[] args) throws Exception { System.out.flush(); System.exit(0); } else if (args[0].equals("threadlocal-leak-late")) { - // Same packing as threadlocal-leak, but the leak thread is - // started only AFTER the recording began, exercising - // Profiler::onThreadStart's registerThreadObject() wiring. + // Same packing as threadlocal-leak (unpackScenarioArgs()), but the leak thread is + // started only AFTER the recording began - the regression shape for + // Profiler::onThreadStart's registerThreadObject() wiring + // (ThreadLocalLeakScenario.run()'s lateThread comment). String[] packed = unpackScenarioArgs("threadlocal-leak-late", args); String commands = packed[0]; String scratchPath = packed[1]; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java index 55766f1890..6ce81762cc 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/JavaProfilerApiSurfaceTest.java @@ -13,6 +13,7 @@ import java.util.HashSet; import java.util.Set; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; public class JavaProfilerApiSurfaceTest { @@ -25,20 +26,33 @@ public void ownedBlockHooksAreNotPublicApiBeforeTaskBlockInstrumentation() throw assertNotPublic(JavaProfiler.class.getDeclaredMethod("blockExit", long.class)); } + // State-mutating hooks into the process-wide profiler singleton: reachable from tests only + // through the test-source bridge ReferenceChainTestHooks, never from application code. + private static final Set REFERENCE_CHAIN_TEST_SEAMS = new HashSet<>(Arrays.asList( + "getTid", "setGcGenerationsEnabled0", "seedKlassPopulationSample0", + "seedTidTrendSample0", "setKlassPopulationRepresentativeForTest0", + "resetKlassPopulationForTest0", "selectLeakCandidateKlassIds0", + "tagAsReferenceChainRoot0", "runReferenceChainPass0", "pollReferenceChainTargets0", + "drainReferenceChainEventCount0", "resetReferenceChainSearchForTest0", + "getReferenceChainPendingPositionForTest0", "getReferenceChainPendingSizeForTest0", + "getReferenceChainFrontierStateForTest0", "getReferenceChainResolvedTargetTagForTest0", + "heapFloorRecordForTest0", "setMaxHeapBytesForTest0", "setHeapFloorRecordingForTest0", + "heapFloorResetForTest0", "heapFloorRisingForTest0", + "shouldRunPassForTest0", "referenceChainPassesRunForTest0")); + @Test public void referenceChainTestSeamsAreNotPublicApi() { - Set seams = new HashSet<>(Arrays.asList( - "getTid", "setGcGenerationsEnabled0", "seedKlassPopulationSample0", - "seedTidTrendSample0", "selectLeakCandidateKlassIds0", "tagAsReferenceChainRoot0", - "runReferenceChainPass0", "pollReferenceChainTargets0", - "drainReferenceChainEventCountForLeaf0")); + Set found = new HashSet<>(); for (Method m : JavaProfiler.class.getDeclaredMethods()) { - if (seams.contains(m.getName()) || m.getName().endsWith("ForTest0")) { + if (REFERENCE_CHAIN_TEST_SEAMS.contains(m.getName())) { + found.add(m.getName()); assertFalse(Modifier.isPublic(m.getModifiers()), - m.getName() + " is a test seam and must not be public API" - + " (tests reach it through ReferenceChainTestHooks)"); + m.getName() + " is a state-mutating test seam and must stay non-public; " + + "tests reach it via ReferenceChainTestHooks"); } } + assertEquals(REFERENCE_CHAIN_TEST_SEAMS, found, + "every listed reference-chain test seam must still exist on JavaProfiler"); } private static void assertNotPublic(Method method) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java b/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java index c37170fa2a..fb5d5f427c 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/ReferenceChainTestHooks.java @@ -6,15 +6,14 @@ package com.datadoghq.profiler; /** - * Test bridge for the package-private reference-chain/liveness test seams on {@link JavaProfiler}, - * so tests outside {@code com.datadoghq.profiler} can call them without the seams becoming public - * API of the shipped jar. The native seams exist only in -DDEBUG builds (debug/asan/tsan); release - * builds get no-op stubs. + * Test bridge for {@link JavaProfiler}'s package-private reference-chain / liveness test seams + * (same pattern as {@link ProfilerOwnedBlockHooks}). The seams mutate the process-wide profiler + * singleton's state, so they stay non-public in the shipped library; tests in other packages + * reach them only through this class. All are debug-native-build only, no-ops in release builds. */ public final class ReferenceChainTestHooks { private ReferenceChainTestHooks() {} - /** The calling thread's profiler tid. Call it on the allocating (leaking) thread. */ public static int getTid() { return JavaProfiler.getTid(); } @@ -55,8 +54,8 @@ public static void pollReferenceChainTargets0() { JavaProfiler.pollReferenceChainTargets0(); } - public static int drainReferenceChainEventCountForLeaf0(Object leaf) { - return JavaProfiler.drainReferenceChainEventCountForLeaf0(leaf); + public static int drainReferenceChainEventCount0() { + return JavaProfiler.drainReferenceChainEventCount0(); } public static void resetReferenceChainSearchForTest0() { @@ -110,10 +109,7 @@ public static boolean heapFloorRisingForTest0() { return JavaProfiler.heapFloorRisingForTest0(); } - /** - * Not a read-only query: under the native engine lock the gate may account a completed - * search's pain and restart/reset terminal search state. - */ + /** Not a pure predicate - see {@link JavaProfiler}'s own {@code shouldRunPassForTest0()} doc. */ public static boolean shouldRunPassForTest0() { return JavaProfiler.shouldRunPassForTest0(); } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java index db78767791..187a715777 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/AggressiveLeakReferenceChainTest.java @@ -6,8 +6,8 @@ package com.datadoghq.profiler.referencechains; import com.datadoghq.profiler.AbstractProfilerTest; -import com.datadoghq.profiler.ReferenceChainTestHooks; import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.ReferenceChainTestHooks; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -30,7 +30,7 @@ * seed {@code LivenessTracker}'s aggregate heap-floor-ring history and its max-heap-bytes cache * directly, so {@code secondsToOOM()}'s projection can be driven under * {@code OOM_URGENT_THRESHOLD_S} (referenceChains.h) deterministically, decoupled from this JVM's - * real {@code -Xmx} and from real GC timing. {@code shouldRunPassForTest0()} then reads + * real {@code -Xmx} and from real GC timing. {@code shouldRunPassForTest0()} then evaluates * {@code ReferenceChainTracker::shouldRunPass()} - the search-restart gate {@code hasLeakSignal()} * feeds - directly, rather than running a real pass, so this class never depends on * {@code selectLeakCandidates()} ranking anything, a real JVMTI heap walk, or a JFR event. @@ -58,7 +58,25 @@ protected boolean isPlatformSupported() { private static void assumeDebugBuild() { assumeTrue(ReferenceChainAssertions.isDebugInstrumentedBuild(), "heapFloorRecordForTest0/setMaxHeapBytesForTest0/shouldRunPassForTest0 only exist in a " - + "-DDEBUG native build - debug/asan/tsan (javaApi.cpp's #ifdef DEBUG guard)"); + + "-DDEBUG native build (debug/asan/tsan - javaApi.cpp's #ifdef DEBUG guard)"); + } + + /** + * Undoes this class's synthetic seeding so later tests in this shared, no-forkEvery JVM never + * see it. {@code resetKlassPopulationForTest0()} also zeroes LivenessTracker's heap-floor ring + * (head/fill - {@code klassPopulationResetForTest()}, livenessTracker.h), so + * {@code secondsToOOM()} reports "insufficient fill" until real GC samples build up again; it + * runs while heap-floor recording is still disabled so no real sample can interleave. + * {@code resetReferenceChainSearchForTest0()} then clears any urgency latch and spent one-shot + * entitlement ({@code _urgent_latched}/{@code _urgent_search_spent}) plus the search state the + * urgent bypass started - after the ring reset, so the BFS thread cannot re-latch from the + * stale projection. Recording is re-enabled last. + */ + private static void restoreHeapFloorDefaults() { + ReferenceChainTestHooks.setMaxHeapBytesForTest0(-1); + ReferenceChainTestHooks.resetKlassPopulationForTest0(); + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + ReferenceChainTestHooks.setHeapFloorRecordingForTest0(true); } /** @@ -120,12 +138,7 @@ public void shouldOpenSearchGateOnAggressiveHeapWideGrowthWithNoLeakCandidate() "The passes that just ran must still have had zero per-klass candidates - the " + "aggregate urgent-OOM projection was the only possible trigger"); } finally { - // Reset the ring while recording is still disabled so the restore below cannot - // let a real GC interleave a sample with the reset, and so the next test in this - // JVM does not inherit this test's seeded samples into its secondsToOOM(). - ReferenceChainTestHooks.heapFloorResetForTest0(); - ReferenceChainTestHooks.setMaxHeapBytesForTest0(-1); - ReferenceChainTestHooks.setHeapFloorRecordingForTest0(true); + restoreHeapFloorDefaults(); } } @@ -151,14 +164,20 @@ public void shouldNotOpenSearchGateOnFlatHeapFloorWithNoLeakCandidate() { assertTrue(candidates == null || candidates.length == 0, "This test's own precondition: no per-klass candidate should exist either"); - assertFalse(ReferenceChainTestHooks.shouldRunPassForTest0(), + // shouldRunPassForTest0() is NOT a pure predicate (see JavaProfiler's own javadoc): under the + // native engine lock it may account a finished search's pain, restart terminal search state, + // or spend the urgent one-shot entitlement. Reset the search right before it so any mutation + // lands on a fresh tracker (_search_started == false) instead of whatever search an earlier + // class in this shared JVM left behind. Only the search is reset here, not the population: + // resetKlassPopulationForTest0() also zeroes the heap-floor ring seeded above, which would + // turn this control into a trivial "insufficient fill" pass. + ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); + boolean gateOpen = ReferenceChainTestHooks.shouldRunPassForTest0(); + assertFalse(gateOpen, "Expected the search-restart gate to stay closed: no heap growth and no per-klass " + "candidate give hasLeakSignal() nothing to trust"); } finally { - // Same order as the seeding test above: ring reset first, still disabled. - ReferenceChainTestHooks.heapFloorResetForTest0(); - ReferenceChainTestHooks.setMaxHeapBytesForTest0(-1); - ReferenceChainTestHooks.setHeapFloorRecordingForTest0(true); + restoreHeapFloorDefaults(); } } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationReferenceChainTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationReferenceChainTest.java index d292f78753..1b1f372fe8 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationReferenceChainTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationReferenceChainTest.java @@ -39,55 +39,63 @@ * AND matches a {@code datadog.HeapLiveObject}'s {@code leakTag}. See * {@link LeakTagCorrelationScenario}'s own header for what each ingredient reproduces. * - *

    Diagnostics on failure: the child's {@code TEST_LOG} stream (queue depths, batch - * sizes, interception counts) arrives on this process's stdout consumer. A failing run embeds - * a filtered tail of it in the assertion message, so the failure mode is diagnosable from the - * test report alone. + *

    Diagnostics on failure: unlike the pod-debugging loop this scenario exists to + * replace, the child's {@code TEST_LOG} stream (queue depths, batch sizes, interception + * counts - the same evidence collected off a live pod's logs) arrives on this process's + * stdout consumer. A failing run embeds a filtered tail of it in the assertion message, so + * the failure mode is diagnosable without redeploying anything. */ @Tag("slow") public class LeakTagCorrelationReferenceChainTest extends AbstractProcessProfilerTest { + // Mirrored from referenceChains.h (LEAK_TAG_BASE/LEAK_TAG_POOL_SIZE) - the contract under + // test, asserted against the runtime event values the child reports. + private static final long LEAK_TAG_BASE = 0x40000000L; + private static final long LEAK_TAG_POOL_SIZE = 256; + @Test void shouldCorrelateLeakChainTargetTagWithLiveObjectLeakTag() throws Exception { assertCorrelationFound("pausetarget=60000"); } /** - * The tight-window variant of the correlation use case: the same scenario under a tight - * pass window (pausetarget in single-digit ms, narrower than a large tag map's - * GetObjectsWithTags cost floor). Full correlation cannot be the pass criterion at this - * window width: the scenario's trend gates are tuned for the loose window's crawl/GC - * interleaving, and under tight windows the leak klass's real-fold trend can flip within a - * single run (chains arrive via the plain crawl path while pool tagging/interception loses - * the race, reported as [correlation-tag-out-of-pool]). What this variant asserts - * deterministically is the machinery the window exists to exercise: the leak-accumulation - * rotation must engage and select FRONTIER-state (un-expanded backlog) holders. Without - * that, the rotation tier selects nothing while known parent candidates pile up in the - * backlog. GetObjectsWithTags batch control is - * covered unit-level (gotwWindowNs() widening): a real child JVM's local tag map - * (~100k filler) keeps the per-call floor under the nominal window, so the widening - * regime cannot be reproduced end-to-end locally without a 242k-entry tag map. + * The pod-regime variant: the same scenario under a pod-like tight pass window (pausetarget in + * single-digit ms, as on the production-like pod whose expand window was ~10ms against a + * 14-41ms GetObjectsWithTags floor at a 242k-entry tag map, ev-leaktag-onpod-round4). Full + * correlation cannot be the pass criterion at this window width: the trend gates were tuned + * for the loose window's crawl/GC interleaving, and under tight windows the leak klass's + * real-fold trend can flip within a single run - the same knife-edge the pod itself lives on + * (chains arrive via the plain crawl path while pool tagging/interception loses the race; + * observed: [correlation-tag-out-of-pool] with zero interceptions). What CAN be asserted + * deterministically is the machinery this window exists to exercise: the leak-accumulation + * rotation must engage and select FRONTIER-state (un-expanded backlog) holders - the round-4 + * fix whose absence made that tier select zero on the pod while holding 51k known parent + * candidates. GetObjectsWithTags batch control is covered unit-level (gotwWindowNs() + * widening): a real child JVM's ~100k-entry tag map keeps the per-call floor under the nominal + * window, so the widening regime cannot be reproduced end-to-end locally. */ @Test void shouldRotateFrontierLeakHoldersUnderPodLikeTightWindows() throws Exception { assertCorrelationFound("pausetarget=10", /*requireFrontierRotation=*/ true); } + /** + * Drives the leak-correlation child JVM and asserts the correlated chain, with the + * referencechains pausetarget given by {@code pauseTargetOption} (everything else held + * fixed across the two callers so the variant differs in exactly the pass-window regime). + */ private void assertCorrelationFound(String pauseTargetOption) throws Exception { assertCorrelationFound(pauseTargetOption, /*requireFrontierRotation=*/ false); } /** * Drives the leak-correlation child JVM and asserts the correlated chain, with the - * referencechains pausetarget given by {@code pauseTargetOption} (everything else held - * fixed across the two callers so the variant differs in exactly the pass-window regime). - * With {@code requireFrontierRotation} (the tight-window variant), the - * leak-accumulation rotation's FRONTIER-state selection is additionally asserted, and - * the correlation result markers that the tight window legitimately produces without the - * full correlation completing (not-found, tag-out-of-pool - see the tight variant's own - * comment) are accepted instead of failed. This variant runs only on DEBUG-instrumented - * native builds and forces reference-chains debug level 2 in the child; the loose variant - * runs on every build. + * referencechains pausetarget given by {@code pauseTargetOption} (everything else held fixed + * across the two callers so the variant differs in exactly the pass-window regime). With + * {@code requireFrontierRotation} (the tight-window pod-regime variant), FRONTIER-state + * rotation selection is additionally asserted, and the pod regime's legitimate partial + * outcomes (not-found, tag-out-of-pool) are accepted instead of failed - but only if the + * child logged at least one leak-tag interception/correlation or chain-reconstruction attempt. */ private void assertCorrelationFound(String pauseTargetOption, boolean requireFrontierRotation) throws Exception { @@ -110,16 +118,22 @@ private void assertCorrelationFound(String pauseTargetOption, // long run cannot grow this without limit. Deque testLogTail = new ArrayDeque<>(); AtomicBoolean frontierRotationSeen = new AtomicBoolean(false); + // Hard evidence for the tight variant's escape hatch: at least one leak-tag interception / + // correlation or chain-reconstruction attempt happened in the child. Set while lines stream + // in (not scanned from testLogTail, whose evict-oldest cap can drop early lines). + AtomicBoolean correlationActivitySeen = new AtomicBoolean(false); try { // Same raised budget as ExternalProcessReferenceChainTest's own scenarios: a genuinely // fresh, cold external JVM's reachable graph is far larger than the in-process tests', // and this scenario adds ~100k filler objects on top. The pausetarget varies by caller. - // memory=64:l:1.0 - the explicit liveness-sample ratio is load-bearing: the default - // (10%, arguments.h's _live_samples_ratio) makes LivenessTracker::track() drop 90% of - // tracked instances, and with only tens of 1MB leak chunks the (klass, tid) population - // becomes a per-run lottery (observed as intermittent "tagLeakInstances ZERO" runs). - // 1.0 makes LeakTagCorrelationScenario's "every allocation is sampled, tracked, and - // taggable" assumption true. + // memory=64:l:1.0 - the explicit liveness-sample ratio is load-bearing: without it the + // default is 10% and LivenessTracker::track() drops 90% of tracked instances + // probabilistically. This scenario's leak cohort is only tens of 1MB chunks, so the + // default makes the (klass, tid) population a per-run Bernoulli lottery - observed live + // as the intermittent "tagLeakInstances ZERO" runs where zero chunks survived the filter + // (passing runs' stable "tagged=5" is exactly the ~10% tail of ~50 chunks). 1.0 makes the + // scenario's documented assumption ("every allocation is sampled, tracked, and taggable") + // actually true. String startCommand = "start,memory=64:l:1.0,generations=true," + "referencechains=true:hops=64:budget=200000:ttl=120000:framecap=2000000:" + pauseTargetOption @@ -148,9 +162,7 @@ private void assertCorrelationFound(String pauseTargetOption, || line.startsWith(LeakTagCorrelationScenario.TAG_OUT_OF_POOL_MARKER) || line.startsWith(LeakTagCorrelationScenario.NO_LIVE_OBJECT_MARKER) || line.startsWith(LeakTagCorrelationScenario.TRANSIENT_CHAIN_MARKER) - || line.startsWith(LeakTagCorrelationScenario.NOISE_CHAIN_MARKER) - || line.startsWith(LeakTagCorrelationScenario.NOT_OWNER_VERIFIED_MARKER) - || line.startsWith(LeakTagCorrelationScenario.STALE_OWNER_CHAIN_MARKER)) { + || line.startsWith(LeakTagCorrelationScenario.NOISE_CHAIN_MARKER)) { resultLine.set(line); } if (line.startsWith("[TEST::INFO]")) { @@ -158,6 +170,12 @@ private void assertCorrelationFound(String pauseTargetOption, && line.contains("state=FRONTIER")) { frontierRotationSeen.set(true); } + if (line.contains("leak-tag intercepted:") // heapReferenceCallback interception + || line.contains("correlateAdmittedLeakTag frontier_tag=") // admitted correlation + || line.contains("buildChainEvent target_tag=") // chain reconstructed + || line.contains("reconstructChain")) { // reconstruction attempted, failed + correlationActivitySeen.set(true); + } // The reader thread can still be running after a join timeout, so every access // to the tail is synchronized (diagnostics() reads a snapshot). synchronized (testLogTail) { @@ -171,8 +189,7 @@ private void assertCorrelationFound(String pauseTargetOption, }, null); - assertTrue(result.inTime, "Child process did not exit within the wait timeout" - + diagnostics(testLogTail)); + assertTrue(result.inTime, "Child process did not exit within the wait timeout"); assertEquals(0, result.exitCode, "Child process exited with a non-zero code"); assertNotNull(resultLine.get(), "Child process never printed a recognizable result marker" + " on stdout" + diagnostics(testLogTail)); @@ -180,8 +197,9 @@ private void assertCorrelationFound(String pauseTargetOption, if (requireFrontierRotation) { assertTrue(frontierRotationSeen.get(), "leak-accumulation rotation never selected a FRONTIER-state (un-expanded) " - + "holder under the tight window - a dead rotation tier (zero selections) " - + "looks exactly like this" + diagnostics(testLogTail)); + + "holder under the pod-like tight window - the round-4 regression this " + + "variant exists to catch (tier dead: zero selections) would look exactly " + + "like this" + diagnostics(testLogTail)); } if (line.startsWith(LeakTagCorrelationScenario.FOUND_MARKER)) { long targetTag = Long.parseLong( @@ -193,8 +211,15 @@ private void assertCorrelationFound(String pauseTargetOption, } if (requireFrontierRotation && (line.equals(LeakTagCorrelationScenario.NOT_FOUND_MARKER) || line.startsWith(LeakTagCorrelationScenario.TAG_OUT_OF_POOL_MARKER))) { - // The tight window legitimately produces these without full correlation completing - // (see the tight variant's comment); the loose variant still fails on them. + // The pod regime legitimately produces these without full correlation completing. The + // escape hatch is NOT a blanket pass: the correlation machinery must have run at least + // once, so a fully broken path (tagging never firing, interception always dropping, no + // chain ever reconstructed) still fails here. + assertTrue(correlationActivitySeen.get(), + "Tight-window outcome " + line + " accepted by the pod-regime escape hatch, but " + + "the child logged no leak-tag interception/correlation and no chain " + + "reconstruction attempt - the correlation path is broken outright, not " + + "losing the pod-regime race" + diagnostics(testLogTail)); return; } fail("Leak-tag correlation did not succeed: " + line + diagnostics(testLogTail)); @@ -229,7 +254,8 @@ private static String diagnostics(Deque testLogTailRef) { } else if (s.contains("intercepted") || s.contains("correlateAdmittedLeakTag") || s.contains("recordDiscoveredInstance") || s.contains("auto-mark") || s.contains("rotation_candidates") - || s.contains("collectLeakAccumulationCandidatesForRotation selected")) { + || s.contains("collectLeakAccumulationCandidatesForRotation selected") + || s.contains("buildChainEvent target_tag=") || s.contains("reconstructChain")) { leakTagLines.add(s); } } 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 346728f387..36f7d12b1e 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 @@ -13,19 +13,13 @@ import org.openjdk.jmc.common.item.IItemIterable; import org.openjdk.jmc.common.item.IMemberAccessor; import org.openjdk.jmc.common.item.IType; -import org.openjdk.jmc.common.item.ItemFilters; import org.openjdk.jmc.flightrecorder.JfrLoaderToolkit; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; - -import static com.datadoghq.profiler.referencechains.ReferenceChainAssertions.LEAK_TAG_BASE; -import static com.datadoghq.profiler.referencechains.ReferenceChainAssertions.LEAK_TAG_POOL_SIZE; /** * The "leak-tag correlation" use case, run inside a genuinely separate child JVM by @@ -36,8 +30,9 @@ * matching a {@code datadog.HeapLiveObject} event's {@code leakTag} for the same object - * the chain-to-live-heap-sample correlation the whole leak-tag pool exists to provide. * - *

    Each ingredient reproduces, in miniature, a condition of large deployments that - * smaller local tests do not reach: + *

    Each ingredient exists to reproduce, in miniature, a condition that only showed + * against real large deployments before this scenario existed (all observed live on a + * production-like pod, each escaping every earlier local test): *

      *
    • Scale ({@link #FILLER}): ~100k live filler objects grow the JVMTI tag map * enough that {@code GetObjectsWithTags} has a real per-call cost floor, the BFS @@ -56,12 +51,7 @@ * chunk's chain must carry a {@code targetTag} in the leak-tag pool's range - * mirrored here from {@code LEAK_TAG_BASE}/{@code LEAK_TAG_POOL_SIZE} * (referenceChains.h) - and at least one {@code datadog.HeapLiveObject} event for a - * {@code byte[]} must carry the same value in {@code leakTag} (joined on - * {@code (tag, leakTagIncarnation)}, since leak tags are recycled). On -DDEBUG builds the - * chain must also be the one {@code getReferenceChainResolvedTargetTagForTest0} resolves for - * a live {@link #LEAK_SINK} chunk (instance identity, not just class), and after that owner - * chunk is dropped and its tag released, its chain must stop being emitted under that - * {@code (tag, leakTagIncarnation)}.
      • + * {@code byte[]} must carry the same value in {@code leakTag}. *
    * *

    Same separate-process rationale as {@code LeakingCacheScenario}'s own class comment @@ -72,15 +62,14 @@ public final class LeakTagCorrelationScenario { private LeakTagCorrelationScenario() {} /** - * The leak: a static collection appended to for the process's whole life - the - * canonical unmaintained-singleton retention shape, DELIBERATELY a growing - * {@code ArrayList} and not a fixed holder. An ArrayList's backing - * {@code elementData} array is REPLACED on growth, and an EXPANDED frontier node's - * children set is frozen at expansion time (expandFrontier()'s own comment), so - * discovering contents added after a resize is exactly the hard production case - * this scenario must prove works: the live holder's re-walk has to admit each new - * backing array, and only then can its new chunks be admitted and leak-tag - * intercepted. + * The leak: a static collection appended to for the process's whole life - the canonical + * unmaintained-singleton retention shape, deliberately a GROWING ArrayList: an ArrayList's + * backing {@code elementData} array is REPLACED on growth, and an EXPANDED frontier node's + * children set is frozen at expansion time, so discovering contents added after a resize is + * the hard production case this scenario must prove works - the live holder's re-walk has to + * admit each new backing array, and only then can its new chunks be admitted and leak-tag + * intercepted. (An earlier fixed-array version sidestepped that requirement and still failed, + * which is what exposed the rotation starvation this fixture now encodes.) */ static final List LEAK_SINK = new ArrayList<>(); @@ -102,13 +91,11 @@ private LeakTagCorrelationScenario() {} private static final int SEED_CHUNKS = 10; /** - * Per-round trend-maintenance epoch counter (debug builds only) - same - * pattern and rationale as StaticFieldGrowingCollectionScenario's own: - * a one-shot seeded ramp ages out of LivenessTracker's hysteresis once - * enough real fold samples interleave (observed live: the byte[] candidate - * dropped right after the first leak-tag interceptions, stranding the - * correlated discoveries with no poll loop to build their chains), so - * each round pushes one fresh rising epoch to keep the candidates alive. + * Per-round trend-maintenance epoch counter (debug builds only), same pattern as + * StaticFieldGrowingCollectionScenario: a one-shot seeded ramp ages out of LivenessTracker's + * hysteresis once enough real fold samples interleave (observed live: the byte[] candidate + * dropped right after the first leak-tag interceptions), so each round pushes one fresh rising + * epoch to keep the candidates alive. */ private static int hysteresisEpoch = 0; @@ -116,27 +103,32 @@ private static void addChunk() { LEAK_SINK.add(new byte[CHUNK_BYTES]); } + // Mirrored from referenceChains.h (LEAK_TAG_BASE/LEAK_TAG_POOL_SIZE): the pool range a + // correlated chain's targetTag must fall into, and the range a matching HeapLiveObject's + // leakTag must fall into. This is the contract under test, asserted against runtime + // event values - not constants compared to constants. + private static final long LEAK_TAG_BASE = 0x40000000L; + private static final long LEAK_TAG_POOL_SIZE = 256; + /** - * Transient handoff channel for the noise representative - deliberately a local of - * {@code run()} (captured by the noise thread's lambda), NOT a static field: any static - * field holding a {@link NoisePayload} would make that one instance genuinely, - * durably static-retained, and the profiler would be RIGHT to emit a static-rooted chain - * for it - found the hard way on this scenario's very first run, which correctly reported - * {@code NoisePayload@depth0:static_field}. After the handoff the only JVM-heap reference - * to the representative is the noise thread's live frame, which is exactly the - * transient-rooted retention this class exists to exercise. + * Transient handoff channel for the noise representative - deliberately a local of {@code run()} + * (captured by the noise thread's lambda), NOT a static field: any static field holding a + * {@link NoisePayload} would make that instance genuinely, durably static-retained, and the + * profiler would be RIGHT to emit a static-rooted chain for it - the scenario's very first run + * correctly reported {@code NoisePayload@depth0:static_field}. After the handoff the only + * JVM-heap reference to the representative is the noise thread's live frame, exactly the + * transient-rooted retention this class exercises. */ private static final class NoiseHandoff { volatile NoisePayload representative; - // The noise thread's own profiler tid - published alongside the payload so - // the seeding below can qualify exactly that thread for the noise klass - // (selectLeakCandidates()'s per-(klass, tid) gate matches the tid that - // actually allocated the noise instances, and that is this thread). + // The noise thread's own profiler tid, published alongside the payload so the seeding below + // can qualify exactly that thread (the per-(klass, tid) gate matches the tid that actually + // allocated the noise instances). volatile int tid; void publish(NoisePayload payload, int tid) { - // tid FIRST (volatile ordering): a caller that sees `representative` set - // is then also guaranteed to see the tid published with it. + // tid FIRST (volatile ordering): a caller that sees `representative` set is then also + // guaranteed to see the tid published with it. this.tid = tid; representative = payload; } @@ -186,32 +178,12 @@ NoisePayload take() { public static final String NOISE_CHAIN_MARKER = "[correlation-noise-chain-emitted] "; /** - * Printed (with the tag) if no correlated in-pool byte[] chain was the chain the identity seam - * resolves for a live {@link #LEAK_SINK} chunk. -DDEBUG builds only. - */ - public static final String NOT_OWNER_VERIFIED_MARKER = "[correlation-not-owner-verified] "; - - /** - * Printed (with the tag) if, after the verified owner chunk was dropped, every dump still - * emitted a byte[] chain under the owner's {@code (tag, leakTagIncarnation)} - the dead - * owner's cached chain outliving its tag (ownership-stamp regression). -DDEBUG builds only. + * One dump's scan outcome - payload fields per one dump; the violation lists are accumulated + * across every scan by keepBest(), regardless of which scan's payload is kept. */ - public static final String STALE_OWNER_CHAIN_MARKER = "[correlation-stale-owner-chain] "; - - /** Dumps the stale-owner check waits through for the dropped chunk's tag to be released. */ - private static final int STALE_CHECK_ATTEMPTS = 10; - - /** - * Scan outcome: payload fields from one dump; transientViolations/noiseChains accumulated - * across all scans by keepBest(). - */ - private static final class ScanResult { + static final class ScanResult { ReferenceChainAssertions.ChainMatch payloadChain; boolean payloadChainInPool; - // In-pool and (on -DDEBUG builds) resolved by the identity seam for a live LEAK_SINK chunk. - boolean payloadChainOwnerVerified; - // LEAK_SINK index of that chunk at scan time; -1 when not verified or on release builds. - int payloadOwnerIndex = -1; // ReferenceChain.leakTagIncarnation of payloadChain; -1 when the field is absent. long payloadChainIncarnation = -1; int liveObjectMatchesForChainTag; @@ -267,50 +239,51 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD } boolean debugBuild = ReferenceChainAssertions.isDebugInstrumentedBuild(); + // Declared at method scope: the per-round and final-attempt maintenance + // seeding below (in their own debugBuild blocks) reuse these tids. int leakTid = 0; int noiseTid = 0; if (debugBuild) { // Representatives FIRST, seeds second: since the leak-tag pool redesign, candidate - // matching, leak-tag assignment (tagLeakInstances scanning the live-heap tracking - // table), and discovered-instance recording are all keyed on the REAL klass id - - // setKlassPopulationRepresentativeForTest0 resolves each representative's real id - // and aliases the synthetic id to it, and the hysteresis seeds below then land in - // the real entry (seeding first would leave them under the synthetic id, where they - // authorize nothing but hasLeakSignal's candidate-count check). - // The leak representative is durably reachable via LEAK_SINK for the rest of the - // process's life; the noise representative only via the noise thread's frame (the - // handoff's own reference is dropped by take()) - exactly what makes one a valid - // leak candidate and the other a gate test. - // The noise thread publishes asynchronously: wait for it, or take() below could - // return null (no alias to the real NoisePayload klass) and handoff.tid could be 0 - // (no real allocating tid qualified) - noise suppression would then go unexercised. - // Plain volatile read, NOT take(): take() drops the handoff's reference. - long noisePublicationDeadline = System.nanoTime() + 30_000_000_000L; - while (handoff.representative == null && System.nanoTime() < noisePublicationDeadline) { - Thread.sleep(10); + // matching, leak-tag assignment, and discovered-instance recording are all keyed on the + // REAL klass id - setting the representative registers the synthetic->real alias, so the + // seeds below land in the real entry (seeding first would leave them under the synthetic + // id, where they authorize nothing but hasLeakSignal's candidate-count check). + // The leak representative is durably reachable via LEAK_SINK; the noise representative + // only via the noise thread's frame - exactly what makes one a valid leak candidate and + // the other a gate test. + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + LEAK_KLASS_ID, LEAK_SINK.get(0)); + // The noise thread publishes asynchronously; wait (bounded) rather than racing take() + // into a null representative / tid 0. + for (int i = 0; i < 200 && handoff.representative == null; i++) { + Thread.sleep(25); } - if (handoff.representative == null || handoff.tid <= 0) { - throw new IllegalStateException("Noise thread did not publish a representative and tid"); + NoisePayload noiseRepresentative = handoff.take(); + noiseTid = handoff.tid; + if (noiseRepresentative == null || noiseTid == 0) { + throw new IllegalStateException("noise thread never published its representative/tid" + + " within 5s (representative=" + (noiseRepresentative == null ? "null" : "present") + + ", tid=" + noiseTid + ")"); } - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(LEAK_KLASS_ID, LEAK_SINK.get(0)); - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(NOISE_KLASS_ID, handoff.take()); - // Per-(klass, tid) qualification seeds: selectLeakCandidates() now also - // requires a qualifying ALLOCATING THREAD, and tagLeakInstances() scopes leak - // tags to exactly those tids' instances - so the seeded qualification must - // name the REAL allocating tids (this thread for the leak chunks, the noise - // thread for the noise payloads) or no tracked instance would ever match - // the tagging scope. Same epoch-aligned ramp shape as the klass seeds - // (scaled to fit the per-tid u8 ring); the synthetic flag keeps these - // ramps immune to the real fold's absent-tid decay between rounds. + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + NOISE_KLASS_ID, noiseRepresentative); + noiseRepresentative = null; // keep retention transient: drop this frame's reference + // Per-(klass, tid) seeds must name the REAL allocating tids (this thread for the leak + // chunks, the noise thread for the noise payloads) or no tracked instance would ever match + // the tagging scope. Same epoch-aligned ramp shape as the klass seeds (scaled to fit the + // per-tid u8 ring); the synthetic flag keeps these ramps immune to the real fold's + // absent-tid decay between rounds. leakTid = ReferenceChainTestHooks.getTid(); - noiseTid = handoff.tid; for (int epoch = 1; epoch <= SEED_EPOCHS_FOR_HYSTERESIS; epoch++) { ReferenceChainTestHooks.seedKlassPopulationSample0(LEAK_KLASS_ID, epoch * 10, epoch); ReferenceChainTestHooks.seedKlassPopulationSample0(NOISE_KLASS_ID, epoch * 10, epoch); ReferenceChainTestHooks.seedTidTrendSample0(LEAK_KLASS_ID, leakTid, epoch * 3, epoch); ReferenceChainTestHooks.seedTidTrendSample0(NOISE_KLASS_ID, noiseTid, epoch * 3, epoch); } - // Maintenance continues the seeded ramp instead of restarting at 1. + // Continue the ramp past the seeded range: each maintenance round below + // pre-increments, so the first round pushes epoch SEED_EPOCHS_FOR_HYSTERESIS + 1 + // (same as StaticFieldGrowingCollectionScenario). hysteresisEpoch = SEED_EPOCHS_FOR_HYSTERESIS; // Wait for the BFS thread to fully complete its first pass (static sweep AND frontier // expansion) before any growth round - same load-bearing wait and rationale as @@ -325,23 +298,20 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD } ReferenceChainTestHooks.pollReferenceChainTargets0(); } else { - // No test seams outside -DDEBUG builds (debug/asan/tsan) - give the real allocation sampler and the + // No debug seams outside debug builds - give the real allocation sampler and the // pass loop time to notice the seeded population on their own. Thread.sleep(5000); } - // Each dump re-emits every resolved chain (drainPendingChainEvents()'s - // snapshot-and-keep contract), so a later dump's scan is a superset state - the - // latest scan with a payload chain observed is always the best payload evidence. Gate - // violations are NOT taken from the latest scan alone: keepBest() accumulates them - // across every scan (a violating resolved chain may be invalidated/evicted before a - // later dump, and the gates are 'ever emitted' contracts). + // Each dump re-emits every resolved chain (snapshot-and-keep), so a later dump's scan is + // usually a superset state - the latest scan is preferred, except keepBest() never lets a + // later, worse payload chain displace an earlier in-pool/correlated one; gate violations of + // 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. + // 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. for (int round = 1; round <= totalRounds; round++) { addChunk(); if (debugBuild) { @@ -357,19 +327,16 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD } System.gc(); profiler.dump(scratchDumpPath); - // Scan only every 3rd round: each scan's JMC parse allocates megabytes - // on this same JVM, and that churn races the leak chunks for space in - // LivenessTracker's tracking table (the age-priority tagging then - // selects old JVM-machinery survivors instead of the young chunks - - // observed live as tagged=5 stable but zero interceptions). Less - // scan churn gives the chunks time to accumulate surviving ages and - // win the tagging priority. Rounds are still dumped every time so - // the final dump always holds the freshest chains. + // Scan only every 3rd round: each scan's JMC parse allocates megabytes on this same JVM, + // and that churn races the leak chunks for space in LivenessTracker's tracking table (the + // age-priority tagging then selects old JVM-machinery survivors instead of the young + // chunks - observed live as tagged=5 stable but zero interceptions). Less scan churn gives + // the chunks time to accumulate surviving ages and win the tagging priority. Rounds are + // still dumped every time so the final dump always holds the freshest chains. if (round % 3 == 0) { - // The identity seam is read after the dump: a chain in the dump for a live chunk stays - // cached (snapshot-and-keep), and chains built since only add keys the dump lacks. - best = keepBest(best, scan(scratchDumpPath, debugBuild ? liveOwnerTags() : null)); - if (correlated(best)) { + best = keepBest(best, scan(scratchDumpPath)); + if (best != null && best.payloadChainInPool + && best.liveObjectMatchesForChainTag > 0) { break; // full correlation achieved - no need to grow further } } @@ -377,7 +344,7 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD } for (int attempt = 0; attempt < 5; attempt++) { - if (correlated(best)) { + if (best != null && best.payloadChainInPool && best.liveObjectMatchesForChainTag > 0) { break; } if (debugBuild) { @@ -394,198 +361,88 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD Thread.sleep(1000); System.gc(); profiler.dump(scratchDumpPath); - best = keepBest(best, scan(scratchDumpPath, debugBuild ? liveOwnerTags() : null)); + best = keepBest(best, scan(scratchDumpPath)); } - // Gate violations are 'ever emitted' contracts, independent of whether a payload chain - // was found - report them first so a NOT_FOUND result (accepted by the tight-window - // variant) can never mask a suppression regression. - if (best != null && !best.transientViolations.isEmpty()) { - System.out.println(TRANSIENT_CHAIN_MARKER + best.transientViolations); - return; - } - if (best != null && !best.noiseChains.isEmpty()) { - System.out.println(NOISE_CHAIN_MARKER + best.noiseChains); - return; - } - // Marker order mirrors the evidence ranking in evidenceScore() below: - // out-of-pool (0) < unverified in-pool (1) < owner-verified in-pool (2) - - // report the weakest evidence first, and keep this cascade in lockstep with - // evidenceScore() if the ranking ever changes. - if (best == null || best.payloadChain == null) { - System.out.println(NOT_FOUND_MARKER); - return; - } - if (!best.payloadChainInPool) { - System.out.println(TAG_OUT_OF_POOL_MARKER + best.payloadChain.targetTag); - return; - } - if (!best.payloadChainOwnerVerified) { - System.out.println(NOT_OWNER_VERIFIED_MARKER + best.payloadChain.targetTag); - return; - } - if (best.liveObjectMatchesForChainTag == 0) { - System.out.println(NO_LIVE_OBJECT_MARKER + best.payloadChain.targetTag); - return; - } - if (debugBuild) { - long ownedTag = best.payloadChain.targetTag; - long ownedIncarnation = best.payloadChainIncarnation; - // Drop the verified owner: its tracked entry dies, cleanup_table() releases the leak tag - // (stamp bump), and the next acquire may hand the same tag to another object. The dead - // owner's chain must never be emitted under its old (tag, stamp) again. The removed chunk - // is deliberately not kept in a local. - LEAK_SINK.remove(best.payloadOwnerIndex); - boolean clean = false; - for (int attempt = 0; attempt < STALE_CHECK_ATTEMPTS && !clean; attempt++) { - System.gc(); - Thread.sleep(500); - profiler.dump(scratchDumpPath); - clean = !hasByteArrayChain(scratchDumpPath, ownedTag, ownedIncarnation) - // Without the stamp field the tag alone cannot tell a new owner from the dead one. - || (ownedIncarnation < 0 && liveOwnerTags().containsKey(ownedTag)); - } - if (!clean) { - System.out.println(STALE_OWNER_CHAIN_MARKER + ownedTag); - return; - } - } - System.out.println(FOUND_MARKER + best.payloadChain.targetTag); - } - - private static boolean correlated(ScanResult r) { - return r != null && r.payloadChainInPool && r.payloadChainOwnerVerified - && r.liveObjectMatchesForChainTag > 0; + System.out.println(resultLine(best)); } /** - * Leak tag -> LEAK_SINK index of the chunk whose own cached chain carries it, via the identity - * seam (0 means no chain resolved for that chunk). Stores indices, never chunk references, so - * an owner can still die. + * The one result line run() prints for the kept scan. Gate violations outrank every payload + * outcome, including "no payload chain found": the tight-window caller accepts NOT_FOUND, so a + * violation reported only behind a found payload would be masked there. */ - private static Map liveOwnerTags() { - Map owners = new HashMap<>(); - for (int i = 0; i < LEAK_SINK.size(); i++) { - long tag = - ReferenceChainTestHooks.getReferenceChainResolvedTargetTagForTest0(LEAK_SINK.get(i)); - if (tag != 0) { - owners.put(tag, i); - } + static String resultLine(ScanResult best) { + if (best == null) { + return NOT_FOUND_MARKER; } - return owners; - } - - /** - * Whether the dump holds a byte[] chain with this targetTag and leakTagIncarnation (-1 matches - * a chain without the field). - */ - private static boolean hasByteArrayChain(Path scratchDumpPath, long tag, long incarnation) - throws Exception { - if (!Files.exists(scratchDumpPath)) { - return false; + if (!best.transientViolations.isEmpty()) { + return TRANSIENT_CHAIN_MARKER + best.transientViolations; } - IItemCollection events; - try (InputStream in = Files.newInputStream(scratchDumpPath)) { - events = JfrLoaderToolkit.loadEvents(in); + if (!best.noiseChains.isEmpty()) { + return NOISE_CHAIN_MARKER + best.noiseChains; } - IItemCollection chains = events.apply( - ItemFilters.type("datadog.ReferenceChain")); - for (IItemIterable iterable : chains) { - IType type = iterable.getType(); - IMemberAccessor chainAccessor = ReferenceChainAssertions.findAccessor(type, "chain"); - IMemberAccessor targetTagAccessor = - ReferenceChainAssertions.findAccessor(type, "targetTag"); - IMemberAccessor incarnationAccessor = - ReferenceChainAssertions.findAccessor(type, "leakTagIncarnation"); - if (chainAccessor == null || targetTagAccessor == null) { - continue; - } - for (IItem item : iterable) { - Object chainValue = chainAccessor.getMember(item); - if (!(chainValue instanceof Object[])) { - continue; - } - Object[] rawChain = (Object[]) chainValue; - if (rawChain.length == 0 || !(rawChain[0] instanceof IMCType) - || !"byte[]".equals(((IMCType) rawChain[0]).getFullName())) { - continue; - } - long chainIncarnation = incarnationAccessor != null - ? ReferenceChainAssertions.numberValue(incarnationAccessor.getMember(item)) : -1; - if (ReferenceChainAssertions.numberValue(targetTagAccessor.getMember(item)) == tag - && chainIncarnation == incarnation) { - return true; - } - } + if (best.payloadChain == null) { + return NOT_FOUND_MARKER; } - return false; + if (!best.payloadChainInPool) { + return TAG_OUT_OF_POOL_MARKER + best.payloadChain.targetTag; + } + if (best.liveObjectMatchesForChainTag == 0) { + return NO_LIVE_OBJECT_MARKER + best.payloadChain.targetTag; + } + return FOUND_MARKER + best.payloadChain.targetTag; } /** - * Payload state comes from the latest scan that saw a payload chain (a scan without one - * never displaces one that had it). Gate violations (transient/noise chains) are - * accumulated across ALL scans - they are 'ever emitted' contracts, and a violating - * resolved chain may be invalidated/evicted before a later dump. + * Monotonic in payload quality (same rule as ThreadLocalLeakScenario.keepBest(), and the + * same in-pool preference scan() applies within a single dump): a later scan replaces + * {@code best} only when its payload chain is at least as good - no chain < out-of-pool + * chain < in-pool chain < in-pool chain with a matching live object; at equal quality + * the newer scan wins. The losing scan's gate violations are always merged into the winner + * (both directions), so a transient/noise chain seen in any dump is never hidden by a + * better-quality scan from another dump. */ - private static ScanResult keepBest(ScanResult best, ScanResult scan) { + static ScanResult keepBest(ScanResult best, ScanResult scan) { if (scan == null) { return best; } if (best == null) { return scan; } - // Same evidence ranking scan() applies within one dump: an owner-verified - // in-pool chain (2) outranks an unverified in-pool one (1), which outranks - // any out-of-pool chain (0). A later scan only wins on equal or better - // evidence, so noise churn and tag recycling cannot displace the - // best-evidenced correlation mid-run. - int bestScore = evidenceScore(best); - int scanScore = evidenceScore(scan); - ScanResult result = scanScore > bestScore || (scanScore == bestScore - && scan.payloadChain != null) ? scan : best; - ScanResult other = result == scan ? best : scan; - // Both non-null here: the early returns above already handled a null scan - // (kept best) and a null best (kept scan), so the loser always exists. - mergeDistinct(result.transientViolations, other.transientViolations); - mergeDistinct(result.noiseChains, other.noiseChains); - return result; + ScanResult winner = quality(scan) >= quality(best) ? scan : best; + ScanResult loser = winner == scan ? best : scan; + mergeViolations(winner.transientViolations, loser.transientViolations); + mergeViolations(winner.noiseChains, loser.noiseChains); + return winner; } - private static int evidenceScore(ScanResult scan) { - if (scan.payloadChain == null) { - return -1; + /** 0: no chain, 1: out-of-pool chain, 2: in-pool chain, 3: in-pool and correlated. */ + private static int quality(ScanResult r) { + if (r == null || r.payloadChain == null) { + return 0; } - return evidenceRank(scan.payloadChainInPool, scan.payloadChainOwnerVerified); - } - - // The single definition of the scan-evidence ranking: owner-verified in-pool (2) - // > unverified in-pool (1) > out-of-pool (0). scan()'s within-dump candidate - // selection and keepBest()'s cross-dump merge both derive from this, so a - // ranking change cannot leave one site disagreeing with the others. - private static int evidenceRank(boolean inPool, boolean ownerVerified) { - return inPool ? (ownerVerified ? 2 : 1) : 0; + if (!r.payloadChainInPool) { + return 1; + } + return r.liveObjectMatchesForChainTag > 0 ? 3 : 2; } - private static void mergeDistinct(List into, List from) { - for (String s : from) { - if (!into.contains(s)) { - into.add(s); + private static void mergeViolations(List into, List from) { + for (String v : from) { + if (!into.contains(v)) { + into.add(v); } } } /** - * Scans one dump: the best leaked {@code byte[]} chain (if any), whether its targetTag is - * in the leak-tag pool and owned by a live LEAK_SINK chunk, how many - * {@code datadog.HeapLiveObject} events for a leaked {@code byte[]} carry that same - * {@code (leakTag, leakTagIncarnation)}, plus the two gate regressions (transient-rooted - * depth<=1 chains, any {@link NoisePayload} chain). - * - * @param liveOwners {@link #liveOwnerTags()} read after this dump, or null on release builds - * (no identity seam: any in-pool chain counts as owner-verified) + * Scans one dump: the first leaked {@code byte[]} chain (if any), whether its targetTag is + * in the leak-tag pool, how many {@code datadog.HeapLiveObject} events for a + * leaked {@code byte[]} carry that same tag in {@code leakTag}, plus the two gate + * regressions (transient-rooted depth<=1 chains, any {@link NoisePayload} chain). */ - private static ScanResult scan(Path scratchDumpPath, Map liveOwners) - throws Exception { + private static ScanResult scan(Path scratchDumpPath) throws Exception { if (!Files.exists(scratchDumpPath)) { return null; } @@ -596,7 +453,7 @@ private static ScanResult scan(Path scratchDumpPath, Map liveOwne ScanResult result = new ScanResult(); IItemCollection chains = events.apply( - ItemFilters.type("datadog.ReferenceChain")); + org.openjdk.jmc.common.item.ItemFilters.type("datadog.ReferenceChain")); for (IItemIterable iterable : chains) { IType type = iterable.getType(); IMemberAccessor chainAccessor = ReferenceChainAssertions.findAccessor(type, "chain"); @@ -630,18 +487,11 @@ private static ScanResult scan(Path scratchDumpPath, Map liveOwne if ("byte[]".equals(leaf)) { boolean inPool = targetTag >= LEAK_TAG_BASE && targetTag < LEAK_TAG_BASE + LEAK_TAG_POOL_SIZE; - // Multiple byte[] chains exist in a single dump: every discovered - // instance of the candidate class gets one, and only the tracked-and- - // tagged instances carry a pool-range targetTag - the rest legitimately - // carry their frontier tag (the correlation is additive on top of the - // chain, not a property of every chain). Prefer the first owner-verified - // in-pool chain, then any in-pool one, over an out-of-pool one seen earlier. - boolean ownerVerified = - inPool && (liveOwners == null || liveOwners.containsKey(targetTag)); - int score = evidenceRank(inPool, ownerVerified); - int currentScore = result.payloadChain == null ? -1 - : evidenceRank(result.payloadChainInPool, result.payloadChainOwnerVerified); - if (score > currentScore) { + // Multiple byte[] chains exist in a single dump: every discovered instance of the + // candidate class gets one, and only the tracked-and-tagged instances carry a + // pool-range targetTag. Prefer the first in-pool (correlated) chain over any + // out-of-pool one seen earlier. + if ((inPool && !result.payloadChainInPool) || result.payloadChain == null) { List chain = new ArrayList<>(rawChain.length); for (Object element : rawChain) { chain.add((IMCType) element); @@ -649,11 +499,20 @@ private static ScanResult scan(Path scratchDumpPath, Map liveOwne result.payloadChain = new ReferenceChainAssertions.ChainMatch( chain, targetTag, depth); result.payloadChainInPool = inPool; - result.payloadChainOwnerVerified = ownerVerified; - result.payloadOwnerIndex = - ownerVerified && liveOwners != null ? liveOwners.get(targetTag) : -1; - result.payloadChainIncarnation = incarnationAccessor != null - ? ReferenceChainAssertions.numberValue(incarnationAccessor.getMember(item)) : -1; + // The (tag, incarnation) pair is the chain's identity. Fail loud on a + // missing or -1 stamp: tag-only matching is ambiguous across recycled tags. + if (incarnationAccessor == null) { + throw new IllegalStateException( + "datadog.ReferenceChain events have no leakTagIncarnation accessor"); + } + long chainIncarnation = + ReferenceChainAssertions.numberValue(incarnationAccessor.getMember(item)); + if (chainIncarnation == -1) { + throw new IllegalStateException( + "chain leakTagIncarnation is not a numeric value; " + + "tag-only correlation is ambiguous across recycled tags"); + } + result.payloadChainIncarnation = chainIncarnation; } } } @@ -661,7 +520,7 @@ private static ScanResult scan(Path scratchDumpPath, Map liveOwne if (result.payloadChainInPool) { IItemCollection liveObjects = events.apply( - ItemFilters.type("datadog.HeapLiveObject")); + org.openjdk.jmc.common.item.ItemFilters.type("datadog.HeapLiveObject")); for (IItemIterable iterable : liveObjects) { IType type = iterable.getType(); IMemberAccessor leakTagAccessor = @@ -670,26 +529,24 @@ private static ScanResult scan(Path scratchDumpPath, Map liveOwne ReferenceChainAssertions.findAccessor(type, "objectClass"); IMemberAccessor incarnationAccessor = ReferenceChainAssertions.findAccessor(type, "leakTagIncarnation"); - // Fail loud if either identity field is missing, mirroring - // ThreadLocalLeakScenario's guard: without the tag every live object - // reads -1 and never matches, without the incarnation the match - // degrades to tag-only - both must surface as errors, not as an empty - // correlation with no diagnostic. - if (leakTagAccessor == null) { - throw new IllegalStateException( - "datadog.HeapLiveObject events have no leakTag accessor"); - } + // Fail loud when either identity accessor is missing; a silent fallback + // would degrade to tag-only matching. if (incarnationAccessor == null) { throw new IllegalStateException( "datadog.HeapLiveObject events have no leakTagIncarnation accessor"); } + if (leakTagAccessor == null) { + throw new IllegalStateException( + "datadog.HeapLiveObject events have no leakTag accessor"); + } for (IItem item : iterable) { long leakTag = ReferenceChainAssertions.numberValue(leakTagAccessor.getMember(item)); long incarnation = ReferenceChainAssertions.numberValue(incarnationAccessor.getMember(item)); - // Leak tags are recycled: the tag alone is ambiguous across chunks. - if (leakTag != result.payloadChain.targetTag + // Leak tags are recycled: match on (tag, incarnation), never tag alone. + if (incarnation == -1 + || leakTag != result.payloadChain.targetTag || incarnation != result.payloadChainIncarnation) { continue; } diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenarioKeepBestTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenarioKeepBestTest.java new file mode 100644 index 0000000000..844ac82cd2 --- /dev/null +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakTagCorrelationScenarioKeepBestTest.java @@ -0,0 +1,142 @@ +/* + * Copyright 2026, Datadog, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ + +package com.datadoghq.profiler.referencechains; + +import org.junit.jupiter.api.Test; +import org.openjdk.jmc.common.IMCType; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Plain-JUnit checks (no profiler, no dump) for {@link LeakTagCorrelationScenario#keepBest} and + * {@link LeakTagCorrelationScenario#resultLine}: a gate violation seen in any scanned dump must + * survive into the kept result regardless of which scan's payload wins, and must outrank every + * payload outcome - including "no payload chain found" - in the printed result line. + */ +public class LeakTagCorrelationScenarioKeepBestTest { + + /** + * quality 0: no payload chain; 1: out-of-pool chain; 2: in-pool, no live object; 3: in-pool and + * correlated. Violations prefixed "T:" go to transientViolations, "N:" to noiseChains. + */ + private static LeakTagCorrelationScenario.ScanResult scan( + int quality, long tag, String... violations) { + LeakTagCorrelationScenario.ScanResult r = new LeakTagCorrelationScenario.ScanResult(); + if (quality >= 1) { + r.payloadChain = + new ReferenceChainAssertions.ChainMatch(Collections.emptyList(), tag, 2); + r.payloadChainInPool = quality >= 2; + r.liveObjectMatchesForChainTag = quality >= 3 ? 1 : 0; + } + for (String v : violations) { + if (v.startsWith("T:")) { + r.transientViolations.add(v); + } else if (v.startsWith("N:")) { + r.noiseChains.add(v); + } else { + throw new IllegalArgumentException("violation must start with T: or N: - " + v); + } + } + return r; + } + + private static LeakTagCorrelationScenario.ScanResult fold( + List scans) { + LeakTagCorrelationScenario.ScanResult best = null; + for (LeakTagCorrelationScenario.ScanResult s : scans) { + best = LeakTagCorrelationScenario.keepBest(best, s); + } + return best; + } + + @Test + public void earlierViolationSurvivesBetterLaterScan() { + LeakTagCorrelationScenario.ScanResult best = scan(1, 1, "T:a"); + LeakTagCorrelationScenario.ScanResult later = scan(3, 2); + LeakTagCorrelationScenario.ScanResult kept = LeakTagCorrelationScenario.keepBest(best, later); + assertEquals(2, kept.payloadChain.targetTag, "the better later scan's payload must win"); + assertEquals(Collections.singletonList("T:a"), kept.transientViolations, + "the earlier dump's transient violation must not be hidden by a better later scan"); + } + + @Test + public void laterViolationSurvivesWorseLaterScan() { + LeakTagCorrelationScenario.ScanResult best = scan(3, 1, "N:x"); + LeakTagCorrelationScenario.ScanResult later = scan(1, 2, "T:b"); + LeakTagCorrelationScenario.ScanResult kept = LeakTagCorrelationScenario.keepBest(best, later); + assertEquals(1, kept.payloadChain.targetTag, "the better earlier payload must be kept"); + assertEquals(Collections.singletonList("T:b"), kept.transientViolations); + assertEquals(Collections.singletonList("N:x"), kept.noiseChains); + } + + @Test + public void violationsAccumulateRegardlessOfScanOrder() { + LeakTagCorrelationScenario.ScanResult forward = + fold(Arrays.asList(scan(1, 1, "T:a"), scan(3, 2), scan(2, 3, "N:y"))); + assertTrue(forward.transientViolations.contains("T:a"), forward.transientViolations.toString()); + assertTrue(forward.noiseChains.contains("N:y"), forward.noiseChains.toString()); + + LeakTagCorrelationScenario.ScanResult reverse = + fold(Arrays.asList(scan(2, 3, "N:y"), scan(3, 2), scan(1, 1, "T:a"))); + assertTrue(reverse.transientViolations.contains("T:a"), reverse.transientViolations.toString()); + assertTrue(reverse.noiseChains.contains("N:y"), reverse.noiseChains.toString()); + } + + @Test + public void newerScanWinsPayloadTieButKeepsEarlierViolations() { + LeakTagCorrelationScenario.ScanResult best = scan(2, 10, "T:a"); + LeakTagCorrelationScenario.ScanResult later = scan(2, 11); + LeakTagCorrelationScenario.ScanResult kept = LeakTagCorrelationScenario.keepBest(best, later); + assertEquals(11, kept.payloadChain.targetTag, "at equal quality the newer scan wins"); + assertTrue(kept.transientViolations.contains("T:a"), kept.transientViolations.toString()); + } + + @Test + public void mergedViolationsAreDeduplicated() { + LeakTagCorrelationScenario.ScanResult kept = + LeakTagCorrelationScenario.keepBest(scan(1, 1, "T:a"), scan(3, 2, "T:a")); + assertEquals(1, kept.transientViolations.size(), kept.transientViolations.toString()); + } + + @Test + public void nullInputsReturnTheOtherSide() { + LeakTagCorrelationScenario.ScanResult s = scan(1, 1); + LeakTagCorrelationScenario.ScanResult b = scan(2, 2); + assertSame(s, LeakTagCorrelationScenario.keepBest(null, s)); + assertSame(b, LeakTagCorrelationScenario.keepBest(b, null)); + assertNull(LeakTagCorrelationScenario.keepBest(null, null)); + } + + @Test + public void resultLineRanksGateViolationsAboveEveryPayloadOutcome() { + assertEquals(LeakTagCorrelationScenario.NOT_FOUND_MARKER, + LeakTagCorrelationScenario.resultLine(null)); + assertTrue(LeakTagCorrelationScenario.resultLine(scan(0, 0, "T:a")) + .startsWith(LeakTagCorrelationScenario.TRANSIENT_CHAIN_MARKER), + "a transient violation without any payload chain must not be reported as NOT_FOUND"); + assertTrue(LeakTagCorrelationScenario.resultLine(scan(0, 0, "N:x")) + .startsWith(LeakTagCorrelationScenario.NOISE_CHAIN_MARKER), + "a noise chain without any payload chain must not be reported as NOT_FOUND"); + assertTrue(LeakTagCorrelationScenario.resultLine(scan(3, 5, "T:a", "N:x")) + .startsWith(LeakTagCorrelationScenario.TRANSIENT_CHAIN_MARKER), + "transient outranks noise, and both outrank FOUND"); + assertEquals(LeakTagCorrelationScenario.NOT_FOUND_MARKER, + LeakTagCorrelationScenario.resultLine(scan(0, 0))); + assertTrue(LeakTagCorrelationScenario.resultLine(scan(1, 5)) + .startsWith(LeakTagCorrelationScenario.TAG_OUT_OF_POOL_MARKER)); + assertTrue(LeakTagCorrelationScenario.resultLine(scan(2, 5)) + .startsWith(LeakTagCorrelationScenario.NO_LIVE_OBJECT_MARKER)); + assertEquals(LeakTagCorrelationScenario.FOUND_MARKER + 7, + LeakTagCorrelationScenario.resultLine(scan(3, 7))); + } +} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakingCacheScenario.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakingCacheScenario.java index f23d5f2312..c2404f563c 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakingCacheScenario.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakingCacheScenario.java @@ -8,11 +8,7 @@ import com.datadoghq.profiler.JavaProfiler; import com.datadoghq.profiler.ReferenceChainTestHooks; import org.openjdk.jmc.common.IMCType; -import org.openjdk.jmc.common.item.IItem; import org.openjdk.jmc.common.item.IItemCollection; -import org.openjdk.jmc.common.item.IItemIterable; -import org.openjdk.jmc.common.item.IMemberAccessor; -import org.openjdk.jmc.common.item.IType; import org.openjdk.jmc.common.item.ItemFilters; import org.openjdk.jmc.flightrecorder.JfrLoaderToolkit; @@ -23,42 +19,41 @@ import java.util.Map; /** - * The "leaking Java app" body run inside a separate child JVM by {@code ExternalLauncher}'s + * The actual "leaking Java app" body run inside a separate child JVM by {@code ExternalLauncher}'s * {@code leak-cache} mode, driven by {@code ExternalProcessReferenceChainTest}. Deliberately not a - * JUnit test: it must run alone in its own process so the process-wide {@code ReferenceChainTracker} - * singleton's one-shot root-seeded BFS walk belongs to this scenario only (see - * {@code ExternalProcessReferenceChainTest}'s class comment). The leak shape is an ever-growing, - * never-evicted {@code HashMap}-backed cache. + * JUnit test itself - no shared-JVM {@code ReferenceChainTracker} singleton with any other test; + * the whole point is to run alone in its own process so the one-shot root-seeded BFS walk always + * belongs to this scenario (see {@code ExternalProcessReferenceChainTest}'s comment for why an + * earlier in-process attempt failed). * - *

    Why {@code cache} is populated before the profiler is started: - * {@code threadLoop()} (referenceChains.cpp) sleeps {@code _effective_cadence_ns} - 1 second, - * initially - before its first {@code shouldRunPass()} check, which always returns {@code true} - * on that first check ({@code !_search_started}). So the one-and-only root-seeded walk fires - * roughly one second after {@code startThread()}, or earlier if woken by a GC-finish signal. On a - * cold JVM, allocating only after starting the profiler lost that race essentially every time: - * the walk caught {@code cache} still empty, permanently blocking discovery of everything added - * afterward (a frontier entry once marked {@code EXPANDED} is never revisited for new children - - * see {@code expandFrontier()}). Seeding before {@code profiler.execute()} removes the race. + *

    Why {@code cache} is seeded and populated before the profiler is even started: + * {@code threadLoop()} sleeps one initial cadence *before* its very first {@code shouldRunPass()} + * check, and that first check always returns {@code true} ({@code !_search_started}) - so the + * process's one-and-only root-seeded walk fires roughly one second after {@code startThread()}, or + * earlier if woken by a GC-finish signal (which can race the thread's own startup). An initial + * in-process attempt created {@code cache} only *after* starting the profiler; against a cold JVM + * that lost the race essentially every time - the one-shot walk caught {@code cache} still empty, + * and a frontier entry once marked {@code EXPANDED} is never revisited for new children. Seeding + * before {@code profiler.execute()} removes the race entirely. */ public final class LeakingCacheScenario { private LeakingCacheScenario() {} - // Arbitrary klass id - LivenessTracker treats it as an opaque key (see KlassPopulationEntry) - // and this process runs no other reference-chains scenario, so it cannot collide. + // Arbitrary, scenario-chosen klass id - this process's own LivenessTracker population table + // treats it as an opaque key, and this process never runs any other reference-chains scenario. private static final int CACHED_PAYLOAD_TEST_KLASS_ID = 987301; - // LivenessTracker's hysteresis gate (livenessTracker.h's KLASS_POPULATION_MIN_FILL_FOR_TREND=10 / - // LEAK_TREND_HYSTERESIS_BASE=5) only starts incrementing consecutive_positive once ring_fill - // reaches 10, so exactly 10 seedKlassPopulationSample0() calls cap consecutive_positive at 1 - - // short of what selectLeakCandidates() requires before hasLeakSignal() reports a candidate, which - // generations=true needs to authorize the BFS thread's first real pass. + // See ReferenceChainTrackingTest.SEED_EPOCHS_FOR_HYSTERESIS for the derivation: 10 + // (KLASS_POPULATION_MIN_FILL_FOR_TREND) + 5 (LEAK_TREND_HYSTERESIS_BASE) - 1 = 14 seed calls + // is the minimum that clears selectLeakCandidates()'s consecutive_positive gate; 15 gives + // margin. generations=true needs that gate cleared to authorize the BFS thread's first pass. private static final int SEED_EPOCHS_FOR_HYSTERESIS = 15; /** - * Durable holder for the cache fixture. The discovered-chain gate suppresses - * chains shallower than the first real holder hop that are rooted at a TRANSIENT - * root (stack/JNI local) - a frame-held "leak" is not a leak - so the cache must - * be retained through a static field rather than only {@code run()}'s local. + * Durable holder for the cache fixture. The discovered-chain gate suppresses chains shallower + * than the first real holder hop that are rooted at a TRANSIENT root (stack/JNI local) - a + * frame-held "leak" is by definition not a leak - so the cache must be retained through a + * static field; {@code run()}'s local {@code cache} variable below aliases this static. */ private static final Map CACHE = new HashMap<>(); @@ -69,25 +64,29 @@ private LeakingCacheScenario() {} public static final String NOT_FOUND_MARKER = "[chain-not-found]"; /** - * Printed to stdout (with the offending chain appended) if a match was found but its - * chain did not pass through {@code java.util.HashMap}'s internal storage. + * Printed to stdout (with the offending class name appended) if a match was found but its chain + * did not pass through {@code java.util.HashMap}'s own internal storage - asserted in the + * parent process rather than here, keeping this scenario's stdout contract to "found/not-found". */ public static final String NO_HASHMAP_INTERNALS_MARKER = "[chain-found-no-hashmap-internals] "; /** * Printed to stdout (with an "N/M" re-emitted-dump count appended) if the chain was - * reconstructed but did not re-emit into one or more subsequent JFR dumps - i.e. the - * "snapshot-and-keep" contract of {@code ReferenceChainTracker::drainPendingChainEvents()} - * (referenceChains.cpp) failed to hold across dumps. + * reconstructed but did not re-emit into one or more subsequent, independently written + * JFR dumps - i.e. the "snapshot-and-keep" contract of {@code drainPendingChainEvents()} failed + * to hold across dumps. Asserted by {@code ExternalProcessReferenceChainTest}. */ public static final String CHAIN_NOT_PERSISTED_MARKER = "[chain-not-persisted] "; /** - * Seeds {@code cache}, *then* starts the profiler (see the class comment for why that order is - * load-bearing), then grows the cache, dumping to {@code scratchDumpPath} after every round + * Seeds {@code cache} with an initial batch, *then* starts the profiler (order is load-bearing, + * see the class comment), then runs the same population-growth loop + * {@code ReferenceChainTrackingTest} pioneered - dumping after every round (plus a grace period) * until a {@code datadog.ReferenceChain} event whose {@code chain[0]} is {@link CachedPayload} - * appears, and prints exactly one result line to stdout. The profiler is started here rather - * than in {@code ExternalLauncher} so {@code cache} can be seeded first. + * appears - then prints exactly one result line to stdout. Called from {@code ExternalLauncher} + * (a different package - hence {@code public}), which owns the generic child-process bootstrap; + * starting the profiler happens here, not there, specifically so {@code cache} can be seeded + * first. */ public static void run(JavaProfiler profiler, String startCommand, Path scratchDumpPath) throws Exception { Map cache = CACHE; @@ -119,22 +118,26 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD match = findMatch(scratchDumpPath); if (match == null && debugBuild) { - // Debug-only seeded short-circuit: decouples the assertion from whether probabilistic - // allocation sampling notices CachedPayload's trend within the fixed round budget, by - // clearing hasLeakSignal()'s hysteresis gate outright. "seed-0" stays reachable via - // cache for the whole run, so it is a valid representative on any round. - // Representative BEFORE seeding: setKlassPopulationRepresentativeForTest0 aliases the - // synthetic id to the representative's real klass id, so the seeds below land in the - // real population entry (candidate matching is keyed by real id). - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(CACHED_PAYLOAD_TEST_KLASS_ID, cache.get("seed-0")); + // Same debug-only, seeded-representative short-circuit as ReferenceChainTrackingTest: + // clears hasLeakSignal()'s hysteresis gate outright, decoupling this scenario's assertion + // from whether probabilistic slope detection happens to notice CachedPayload's trend + // within the fixed round/timeout budget. "seed-0" (from seedInitialBatch()) is reachable + // via cache for the scenario's entire lifetime, so it is always a valid representative. + // Representative BEFORE seeding: setting it registers the synthetic->real id alias, so + // the seeds below land in the real population entry (candidate matching is real-id keyed + // since the leak-tag pool redesign). + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + CACHED_PAYLOAD_TEST_KLASS_ID, cache.get("seed-0")); if (!seededTestKlassTrend) { // The qualifying tid is THIS thread's - the creator of "seed-0". The per-round - // allocator threads are one-shot, so none of their tids carries a sustained + // allocator threads are one-shot, so no single one of their tids carries a sustained // per-tid signal. int leakTid = ReferenceChainTestHooks.getTid(); for (int epoch = 1; epoch <= SEED_EPOCHS_FOR_HYSTERESIS; epoch++) { - ReferenceChainTestHooks.seedKlassPopulationSample0(CACHED_PAYLOAD_TEST_KLASS_ID, epoch * 10, epoch); - ReferenceChainTestHooks.seedTidTrendSample0(CACHED_PAYLOAD_TEST_KLASS_ID, leakTid, epoch * 3, epoch); + ReferenceChainTestHooks.seedKlassPopulationSample0( + CACHED_PAYLOAD_TEST_KLASS_ID, epoch * 10, epoch); + ReferenceChainTestHooks.seedTidTrendSample0( + CACHED_PAYLOAD_TEST_KLASS_ID, leakTid, epoch * 3, epoch); } seededTestKlassTrend = true; } @@ -178,10 +181,12 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD return; } - // Across-dumps persistence: Profiler::dump() re-snapshots the resolved-chain cache without - // clearing it (drainPendingChainEvents(), "snapshot-and-keep, not a drain"), so a still-live - // sample's chain must re-emit into every dump. `cache` keeps the sample live; each dump - // targets a brand-new file, so a match there can only come from re-emission. + // Across-dumps persistence: a resolved chain must re-emit into EVERY subsequent dump the + // sample survives into, not only the dump that first observed it. Profiler::dump() + // re-snapshots the resolved-chain cache without clearing it on every call + // ("snapshot-and-keep, not a drain"). `cache` still holds every CachedPayload here, so the + // sample stays live across these extra dumps; each dump targets a brand-new file, so a chain + // found there can only be present because it was re-emitted into that dump. int persistChecks = 3; int persisted = 0; for (int i = 1; i <= persistChecks; i++) { @@ -207,7 +212,11 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD System.out.println(FOUND_MARKER + match.chain.get(0).getFullName()); } - /** Key namespace "seed-" is disjoint from the round loop's "leak-". */ + /** + * Populates {@code cache} with {@code count} entries under a key namespace ("seed-") disjoint + * from the round loop's own ("leak-") - before the profiler (and its BFS thread) even starts, + * so the one-shot root-seeded walk can never catch {@code cache} empty. + */ private static void seedInitialBatch(Map cache, int count) { for (int i = 0; i < count; i++) { String key = "seed-" + i; @@ -215,6 +224,7 @@ private static void seedInitialBatch(Map cache, int count } } + /** Temporary diagnostic: print all native debug counters. */ private static void debugDumpCounters(JavaProfiler profiler) { try { Map counters = profiler.getDebugCounters(); @@ -228,6 +238,7 @@ private static void debugDumpCounters(JavaProfiler profiler) { } } + /** Temporary diagnostic: print every datadog.ReferenceChain event's chain[0] class, if any. */ private static void debugDumpAllChainLeaves(Path scratchDumpPath) { try { if (!Files.exists(scratchDumpPath)) { @@ -247,15 +258,15 @@ private static void debugDumpAllChainLeaves(Path scratchDumpPath) { IItemCollection abandoned = events.apply(ItemFilters.type("datadog.ReferenceChainAbandoned")); long totalAbandoned = abandoned.stream().mapToLong(it -> it.getItemCount()).sum(); System.out.println("[debug] datadog.ReferenceChainAbandoned total events: " + totalAbandoned); - for (IItemIterable iterable : chains) { - IType type = iterable.getType(); - IMemberAccessor chainAccessor = + for (org.openjdk.jmc.common.item.IItemIterable iterable : chains) { + org.openjdk.jmc.common.item.IType type = iterable.getType(); + org.openjdk.jmc.common.item.IMemberAccessor chainAccessor = ReferenceChainAssertions.findAccessor(type, "chain"); if (chainAccessor == null) { System.out.println("[debug] no chain accessor found"); continue; } - for (IItem item : iterable) { + for (org.openjdk.jmc.common.item.IItem item : iterable) { Object chainValue = chainAccessor.getMember(item); if (chainValue instanceof Object[]) { Object[] raw = (Object[]) chainValue; @@ -283,10 +294,11 @@ private static ReferenceChainAssertions.ChainMatch findMatch(Path scratchDumpPat } /** - * Leak fixture. The 32 {@code long} fields exist so far fewer entries reach - * {@code ObjectSampler}'s 256KiB sampling floor - plain fields, not a nested array, because a - * companion array would be a second allocation competing for the size-weighted sampler's - * attention. + * Realistic-leak fixture - identical shape to {@code ReferenceChainTrackingTest.CachedPayload}. + * The 32 {@code long} fields exist purely so a given number of megabytes needs far fewer entries + * to reach {@code ObjectSampler}'s real 256KiB sampling floor - deliberately plain fields, not + * a nested array (a same-instance companion array would be a second, separate heap allocation + * the size-weighted sampler would compete for, taking attention away from CachedPayload). */ static final class CachedPayload { final String key; diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/PersistentAllocator.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/PersistentAllocator.java deleted file mode 100644 index 02f7b08b9d..0000000000 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/PersistentAllocator.java +++ /dev/null @@ -1,196 +0,0 @@ -/* - * Copyright 2026, Datadog, Inc. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -package com.datadoghq.profiler.referencechains; - -import com.datadoghq.profiler.ReferenceChainTestHooks; - -import java.util.concurrent.ArrayBlockingQueue; -import java.util.concurrent.SynchronousQueue; -import java.util.concurrent.TimeUnit; - -/** - * A single long-lived thread that runs caller-supplied allocation batches on demand. - * - *

    Tests use one persistent thread so every instance of the leak klass is allocated by the - * same thread: a fresh short-lived thread per round gets a new tid each time, and the per-tid - * trend qualification ages out before the population trend can establish itself (the flake the - * {@code @Order(1)} test documents). Callers must also seed with this thread's tid, obtained - * from {@link #tid()} - captured on the allocator thread itself, never the caller's. - * - *

    {@link #run(Runnable)} blocks until the batch completes and rethrows any failure from the - * allocator thread, so allocations are finished (and any exception surfaced) before the caller - * proceeds to {@code System.gc()} or a dump. - */ -final class PersistentAllocator implements AutoCloseable { - // Poison task handed to the worker by close(); identity compared, never run. - private static final Runnable CLOSE = () -> { }; - - private static final long HANDOFF_TIMEOUT_SECONDS = 30; - - // One unit of work handed to the worker; carries the caller-side generation - // so the worker can echo it back in the Outcome. The CLOSE poison rides in a - // Job too (generation -1, never a real generation). - private static final class Job { - final Runnable task; - final int generation; - - Job(Runnable task, int generation) { - this.task = task; - this.generation = generation; - } - } - - // The worker's completion entry; the generation identifies which run() - // produced it, so a stale outcome left by an abandoned (timed-out) run is - // discarded by the caller instead of being misattributed to the next task. - private static final class Outcome { - final int generation; - final Object result; // Object marker = success, a Throwable = the batch's failure - - Outcome(int generation, Object result) { - this.generation = generation; - this.result = result; - } - } - - private final SynchronousQueue handoff = new SynchronousQueue<>(); - // Worker-to-caller completion slot: an Outcome with the caller's generation - // = this task's verdict (Object marker = success, a Throwable = the batch's - // failure). It is buffered (one slot) because the protocol is single-in-flight, - // and the worker's offer() must retain the signal even when the batch finishes - // in the window between the handoff rendezvous and the caller entering poll(): - // a SynchronousQueue would drop that bare offer and turn a completed batch - // (or its rethrown failure) into a 30-second poll timeout. - private final ArrayBlockingQueue completion = new ArrayBlockingQueue<>(1); - private final Thread thread; - private volatile int tid; - private volatile boolean closed; - // Monotonic per-run() token; only the caller thread runs tasks through run(), - // so plain int arithmetic is sufficient. - private int nextGeneration; - - PersistentAllocator(String name) { - thread = new Thread(this::loop, name); - // Daemon: a test that dies before close() must not hold the JVM open on handoff.take(). - thread.setDaemon(true); - thread.start(); - // The caller may call tid() immediately after construction (seed-before-wait ordering); - // do not return until the allocator thread has published it. - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(HANDOFF_TIMEOUT_SECONDS); - while (tid == 0 && thread.isAlive()) { - if (System.nanoTime() - deadline > 0) { - throw new IllegalStateException("allocator thread " + name + " never published its tid"); - } - Thread.yield(); - } - if (tid == 0) { - throw new IllegalStateException("allocator thread " + name + " died before publishing its tid"); - } - } - - /** The allocator thread's OS tid, captured by that thread itself at startup. */ - int tid() { - return tid; - } - - /** - * Runs {@code task} on the allocator thread, blocking until it completes. A task that throws - * is rethrown on the caller's thread, so a failed batch fails the test instead of stalling - * the next round on partially-allocated state. - */ - void run(Runnable task) { - if (closed || !thread.isAlive()) { - throw new IllegalStateException("allocator thread " + thread.getName() + " is no longer running"); - } - try { - // A previous run() that timed out may still have its outcome parked in - // the slot (the worker finishes the abandoned task and offers into the - // then-empty queue). Drop what is already there; the generation check - // below then discards any stale outcome that lands after this clear - - // a plain clear alone cannot close that window, because the worker only - // parks at handoff.take() AFTER offering the abandoned task's outcome. - completion.clear(); - int generation = nextGeneration++; - if (!handoff.offer(new Job(task, generation), HANDOFF_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { - throw new IllegalStateException("allocator thread " + thread.getName() + " did not accept a task"); - } - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(HANDOFF_TIMEOUT_SECONDS); - while (true) { - Outcome outcome = completion.poll(deadline - System.nanoTime(), TimeUnit.NANOSECONDS); - if (outcome == null) { - throw new IllegalStateException("allocator thread " + thread.getName() + " never finished a task"); - } - // Discard an abandoned earlier run()'s outcome and keep waiting for this - // generation's verdict. The stale entry is always offered before our own: - // the worker offers it before looping back to handoff.take(), through - // which alone our Job can be accepted. - if (outcome.generation != generation) { - continue; - } - if (outcome.result instanceof RuntimeException) { - throw (RuntimeException) outcome.result; - } - if (outcome.result instanceof Error) { - throw (Error) outcome.result; - } - return; - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IllegalStateException("interrupted while waiting for allocator thread " + thread.getName(), e); - } - } - - /** Stops the allocator thread and waits for it to exit. Idempotent. */ - @Override - public void close() { - if (closed) { - return; - } - closed = true; - try { - if (thread.isAlive() && handoff.offer(new Job(CLOSE, -1), HANDOFF_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { - // Drain the exit acknowledgement so close() returns only after the thread is done. - completion.poll(HANDOFF_TIMEOUT_SECONDS, TimeUnit.SECONDS); - } - thread.join(TimeUnit.SECONDS.toMillis(HANDOFF_TIMEOUT_SECONDS)); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - - private void loop() { - tid = ReferenceChainTestHooks.getTid(); - try { - while (true) { - Job job = handoff.take(); - if (job.task == CLOSE) { - // put (blocking), not offer: after an abandoned run() the slot can still - // hold that run's stale outcome, and a dropped exit acknowledgement would - // leave close()'s poll draining the stale entry instead. close() always - // drains exactly one entry after the handoff, which unblocks this put. - completion.put(new Outcome(job.generation, new Object())); // exit acknowledgement; no further work arrives - return; - } - Object outcome = null; - try { - job.task.run(); - } catch (RuntimeException | Error t) { - outcome = t; - } - // put (blocking), not offer: with one slot, a stale outcome left by an - // abandoned (timed-out) run() can still occupy the queue when this - // task finishes, and a dropped outcome would turn a completed task into - // the caller's 30s poll timeout. The caller's generation loop drains the - // stale entry (discard-by-generation), which unblocks this put; if the - // caller already gave up, the next run()'s clear() or close()'s poll does. - completion.put(new Outcome(job.generation, outcome != null ? outcome : new Object())); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } -} diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java index 735720170f..68b7c58364 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainAssertions.java @@ -52,7 +52,7 @@ public static boolean isDebugInstrumentedBuild() { } /** - * The {@code ddprof_test.config} value ProfilerTestPlugin.kt sets on every test<config> task. + * The {@code ddprof_test.config} value ProfilerTestPlugin.kt sets on every test task. * Launchers forwarding it to a child JVM must fail loud when it is unset (a test run directly * from an IDE, outside the Gradle tasks) instead of concatenating the literal "null", which the * child would silently classify as a release build and take the unseeded, flakier path. @@ -185,9 +185,11 @@ public static JfrChainMatch findMatchForClass(JfrEvents events, Class targetC } /** - * {@code targetClass}'s name in {@code IMCType.getFullName()} format: {@code Class.getName()} - * renders {@code byte[].class} as {@code "[B"}, but JMC renders it as {@code "byte[]"}, so a - * direct comparison never matches an array-typed leaf. Non-array names are unchanged. + * {@code targetClass}'s name in the format {@code IMCType.getFullName()} uses for array types: + * {@code Class.getName()} renders {@code byte[].class} as {@code "[B"} (JVM internal signature + * notation) but JMC's chunk parser renders the same class as {@code "byte[]"} (Java source + * notation), so comparing {@link Class#getName()} directly against {@code getFullName()} can + * never match an array-typed leaf class. Only affects array {@code targetClass} values. */ private static String jmcStyleName(Class targetClass) { int dimensions = 0; @@ -230,12 +232,9 @@ private static String classFullName(Object element) { /** * Looks up a field's accessor by identifier rather than via {@code Attribute.attr(...)}: JMC's - * v1 chunk parser (internal.parser.v1.ValueReaders.ArrayReader#getContentType()) registers - * {@code UnitLookup.UNKNOWN} as the declared content type for every array field regardless of - * what its element reader resolves to, so an {@code F_ARRAY} field like {@code chain} - * (T_CLASS, F_CPOOL|F_ARRAY, jfrMetadata.cpp) cannot be bound via a compile-time-typed - * {@code Attribute}. Mirrors {@code ReferenceChainJfrParserTest}'s identical lookup, which - * already proves this resolves {@code chain}'s array elements to real {@link IMCType}s. + * v1 chunk parser registers {@code UnitLookup.UNKNOWN} as the declared content type for every + * array field regardless of what its element reader resolves to, so an {@code F_ARRAY} field + * like {@code chain} cannot be bound via a compile-time-typed {@code Attribute}. */ public static IMemberAccessor findAccessor(IType type, String identifier) { for (IAccessorKey key : type.getAccessorKeys().keySet()) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainJfrParserTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainJfrParserTest.java index 30e7537c53..b9c0c4110d 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainJfrParserTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainJfrParserTest.java @@ -5,8 +5,8 @@ package com.datadoghq.profiler.referencechains; -import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Assumptions; import org.openjdk.jmc.common.IMCType; import org.openjdk.jmc.common.item.IAccessorKey; import org.openjdk.jmc.common.item.IItem; @@ -27,7 +27,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.Properties; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -35,34 +34,53 @@ import static org.junit.jupiter.api.Assertions.fail; /** - * Verifies JMC resolves {@code datadog.ReferenceChain}'s {@code chain} field - declared in - * jfrMetadata.cpp as {@code field("chain", T_CLASS, ..., F_CPOOL | F_ARRAY)}, an array of - * constant-pool {@code T_CLASS} indices. Neither scalar F_CPOOL fields (e.g. {@code objectClass}) - * nor F_ARRAY-of-struct fields (e.g. {@code jdk.StackTrace#frames}) cover this combination. + * PROF-15341 design doc, Open Question: does JMC's parser resolve the {@code datadog.ReferenceChain} + * event's {@code chain} field - declared in jfrMetadata.cpp as {@code field("chain", T_CLASS, + * ..., F_CPOOL | F_ARRAY)}, an array of scalar constant-pool-index {@code T_CLASS} + * values - the same way it resolves a plain scalar F_CPOOL field ({@code objectClass}) or a + * plain F_ARRAY-of-composite-struct field ({@code jdk.StackTrace#frames})? Neither shape proves + * this combination. * - *

    Loads the .jfr produced by the companion gtest ({@code referenceChainJfrRoundtrip_ut.cpp}) - * directly, with no live profiler, and asserts the seeded class names in leaf-to-root order. + *

    Loads the standalone .jfr file produced by the companion gtest + * ({@code referenceChainJfrRoundtrip_ut.cpp}) directly via {@link JfrLoaderToolkit} - no live + * profiler attach - and asserts the {@code chain} field resolves to the exact class names the + * gtest seeded, in the leaf-to-root order {@code buildChainEvent()} produces. * *

    Why a generic accessor, not {@code Attribute.attr(...)}: JMC's v1 chunk parser - * ({@code ArrayReader#getContentType()}) registers {@code UnitLookup.UNKNOWN} as the content type - * of every array field, so it cannot be bound via a typed {@code Attribute}. Looking it up by - * identifier via {@code IType#getAccessorKeys()} still goes through {@code ArrayReader}'s - * per-element {@code PoolReader} resolution - the thing this test exists to prove. + * registers {@code UnitLookup.UNKNOWN} as the declared content type for every array + * field, regardless of what its element reader resolves to, and a field registered as UNKNOWN + * cannot be bound via a compile-time-typed {@code Attribute}. This test looks the field up by + * identifier via {@code IType#getAccessorKeys()} and calls {@code IType#getAccessor(IAccessorKey)} + * directly - the same lookup JMC's own UI uses for fields it has no compile-time-known attribute + * for. It still calls through the code ({@code ArrayReader.read()}/{@code resolve()} delegating + * per-element to the field's {@code PoolReader}) that resolves each element's constant-pool index + * to its class - the actual thing this test exists to prove. */ public class ReferenceChainJfrParserTest { private static final String EVENT_TYPE = "datadog.ReferenceChain"; - /** Set by the build for both this test and the companion gtest (see build.gradle.kts). */ - private static final String ROUNDTRIP_JFR_ENV = "DDPROF_REFERENCE_CHAIN_ROUNDTRIP_JFR"; + /** + * The path the companion gtest resolves to: {@code DDPROF_REFERENCE_CHAIN_ROUNDTRIP_JFR} when + * set (ddprof-lib's build sets it to a build-dir path for both the gtest and this test task), + * otherwise the OS temp dir (TMPDIR, falling back to /tmp) for standalone runs. The file is + * consume-once: this test deletes it after loading and the gtest unlinks it before regenerating, + * so a stale fixture from an earlier build can never satisfy the existence assumption below. + */ + private static Path roundtripJfrPath() { + String configured = System.getenv("DDPROF_REFERENCE_CHAIN_ROUNDTRIP_JFR"); + if (configured != null && !configured.isEmpty()) { + return Paths.get(configured); + } + String tmp = System.getenv("TMPDIR"); + String dir = (tmp != null && !tmp.isEmpty()) ? tmp : "/tmp"; + return Paths.get(dir, "datadog_reference_chain_roundtrip.jfr"); + } @Test public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() throws IOException, CouldNotLoadRecordingException { - String configured = System.getenv(ROUNDTRIP_JFR_ENV); - Assumptions.assumeTrue(configured != null && !configured.isEmpty(), - ROUNDTRIP_JFR_ENV + " is not set - this test only runs from the Gradle test tasks"); - Path jfrPath = Paths.get(configured); + Path jfrPath = roundtripJfrPath(); // Assumption, not assertion: the producing gtest does not run on CI runners without Google // Test (e.g. the el7 testRelease job) or with -Pskip-gtest. The build deletes the file before // the gtest runs, and when the gtest is skipped, so an existing file is always this build's. @@ -70,29 +88,21 @@ public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() "Expected " + jfrPath + " to exist - run " + ":ddprof-lib:gtestDebug_referenceChainJfrRoundtrip_ut first " + "(referenceChainJfrRoundtrip_ut.cpp produces this file)."); - // The seeded values the companion gtest wrote next to the recording - the - // single source of truth for this test's expectations, so the producer and - // this consumer cannot drift apart silently. - Path expectedPath = Paths.get(configured + ".expected"); - Assumptions.assumeTrue(Files.exists(expectedPath), - "Expected " + expectedPath + " to exist - run " - + ":ddprof-lib:gtestDebug_referenceChainJfrRoundtrip_ut first " - + "(referenceChainJfrRoundtrip_ut.cpp produces this sidecar)."); - Properties expected = new Properties(); - try (InputStream in = Files.newInputStream(expectedPath)) { - expected.load(in); - } IItemCollection events; try (InputStream in = Files.newInputStream(jfrPath)) { events = JfrLoaderToolkit.loadEvents(in); + } finally { + // Consume-once (see roundtripJfrPath()): loadEvents() fully materializes the recording, so + // the shared fixture can go now - even on a parse failure - and a later run only ever sees + // a file freshly written by the gtest. + Files.deleteIfExists(jfrPath); } IItemCollection chainEvents = events.apply(ItemFilters.type(EVENT_TYPE)); assertTrue(chainEvents.hasItems(), "Expected at least one " + EVENT_TYPE + " event"); List resolvedChain = null; long targetTag = -1; - long leakTagIncarnation = -1; int depth = -1; for (IItemIterable iterable : chainEvents) { IType type = iterable.getType(); @@ -100,9 +110,6 @@ public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() IMemberAccessor edgesAccessor = findAccessor(type, "edges"); IMemberAccessor targetTagAccessor = findAccessor(type, "targetTag"); IMemberAccessor depthAccessor = findAccessor(type, "depth"); - IMemberAccessor incarnationAccessor = findAccessor(type, "leakTagIncarnation"); - assertNotNull(incarnationAccessor, "No accessor for 'leakTagIncarnation' field on " - + EVENT_TYPE); assertNotNull(chainAccessor, "No accessor for 'chain' field on " + EVENT_TYPE); assertNotNull(edgesAccessor, "No accessor for 'edges' field on " + EVENT_TYPE + " - the new retention-edge label array (T_STRING|F_ARRAY) did not make it " @@ -126,25 +133,24 @@ public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() chain.add((IMCType) element); } resolvedChain = chain; - // Retention-edge labels, one per hop. The gtest has no JNIEnv, so it seeds kind labels - // instead of decoded ones; this checks the array field parses, not the decode - // (covered natively by referenceChains_ut). + // Retention-edge labels: one per chain hop, aligned with it. The gtest seeds kind labels + // because its JNIEnv is null - the label DECODE is skipped; the Java side validates the + // ARRAY FIELD parses, not the decode itself (referenceChains_ut covers that natively). Object edgesValue = edgesAccessor.getMember(item); assertNotNull(edgesValue, "'edges' field resolved to null"); assertTrue(edgesValue instanceof Object[], "'edges' field resolved to " + edgesValue.getClass() + ", expected an array"); Object[] rawEdges = (Object[]) edgesValue; - assertEquals(expectedInt(expected, "chain.length"), rawEdges.length, - "edges must align with the chain, one label per hop"); + assertEquals(3, rawEdges.length, "edges must align with the chain, one label per hop"); for (Object e : rawEdges) { assertTrue(e instanceof String, "edges[] element resolved to " + (e == null ? "null" : e.getClass()) + ", expected a String"); } - assertEquals(expected.getProperty("edges.0"), rawEdges[0]); - assertEquals(expected.getProperty("edges.1"), rawEdges[1]); - assertEquals(expected.getProperty("edges.2"), rawEdges[2]); + assertEquals("field", rawEdges[0]); + assertEquals("element", rawEdges[1]); + assertEquals("jni_global", rawEdges[2]); if (targetTagAccessor != null) { Object v = targetTagAccessor.getMember(item); if (v instanceof Number) { @@ -153,7 +159,6 @@ public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() targetTag = ((IQuantity) v).longValue(); } } - leakTagIncarnation = longValue(incarnationAccessor.getMember(item)); if (depthAccessor != null) { Object v = depthAccessor.getMember(item); if (v instanceof Number) { @@ -170,55 +175,15 @@ public void chainFieldResolvesToSeededClassNamesInLeafToRootOrder() } assertNotNull(resolvedChain, "Never iterated a " + EVENT_TYPE + " item"); - assertEquals(expectedInt(expected, "chain.length"), resolvedChain.size(), - "Expected leaf/middle/root - as many entries as the gtest seeded"); + assertEquals(3, resolvedChain.size(), "Expected leaf/middle/root - 3 entries"); // Leaf-to-root order, matching ReferenceChainTracker::buildChainEvent()'s // FrontierTable::reconstructChain() contract and the gtest's insert() calls - // (tag=3 leaf -> tag=2 middle -> tag=1 root). The sidecar carries the gtest's - // slashed internal names; JMC renders IMCType full names with dots. - for (int i = 0; i < resolvedChain.size(); i++) { - assertEquals(expected.getProperty("chain." + i).replace('/', '.'), - resolvedChain.get(i).getFullName(), - "chain[" + i + "] must be the gtest's seeded class in leaf-to-root order"); - } - assertEquals(expectedLong(expected, "targetTag"), targetTag, - "targetTag should be the leaf's frontier tag the gtest seeded"); - assertEquals(expectedInt(expected, "depth"), depth, - "depth should be the leaf's own depth the gtest seeded"); - assertEquals(expectedLong(expected, "leakTagIncarnation"), leakTagIncarnation, - "leakTagIncarnation should carry the event's ownership stamp the gtest seeded"); - } - - // Both sidecar readers report the offending key with the value: a bare parse - // exception names neither, and a MISSING key surfaces as a null parse whose - // built-in message ("null") says nothing about which sidecar entry is absent. - private static int expectedInt(Properties expected, String key) { - String value = expected.getProperty(key); - try { - return Integer.parseInt(value); - } catch (RuntimeException e) { - throw new IllegalStateException( - "malformed or missing '" + key + "' in the roundtrip sidecar (value: " + value + ")", e); - } - } - - private static long expectedLong(Properties expected, String key) { - String value = expected.getProperty(key); - try { - return Long.parseLong(value); - } catch (RuntimeException e) { - throw new IllegalStateException( - "malformed or missing '" + key + "' in the roundtrip sidecar (value: " + value + ")", e); - } - } - - private static long longValue(Object v) { - if (v instanceof Number) { - return ((Number) v).longValue(); - } else if (v instanceof IQuantity) { - return ((IQuantity) v).longValue(); - } - return -1; + // (tag=3 leaf -> tag=2 middle -> tag=1 root). + assertEquals("com.test.ChainLeaf", resolvedChain.get(0).getFullName()); + assertEquals("com.test.ChainMiddle", resolvedChain.get(1).getFullName()); + assertEquals("com.test.ChainRoot", resolvedChain.get(2).getFullName()); + assertEquals(3L, targetTag, "targetTag should be the leaf's frontier tag (3)"); + assertEquals(2, depth, "depth should be the leaf's own depth (2 hops from the root)"); } private static IMemberAccessor findAccessor(IType type, String identifier) { diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java index d50f79c647..bf8c073c06 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ReferenceChainTestSeamsTest.java @@ -6,56 +6,55 @@ package com.datadoghq.profiler.referencechains; import com.datadoghq.profiler.AbstractProfilerTest; -import com.datadoghq.profiler.JavaProfiler; -import com.datadoghq.profiler.ReferenceChainTestHooks; import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.ReferenceChainTestHooks; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; /** - * {@code ReferenceChainTrackingTest} exercises {@code LivenessTracker}'s probabilistic slope - * detection and {@code ReferenceChainTracker}'s root-seeded BFS discovery together, which is - * reliable only when both line up within the same retry window. This class decouples them via the - * debug-build-only seams on {@link JavaProfiler} (javaApi.cpp), verifying each in isolation: + * PROF-15341 follow-up: {@code ReferenceChainTrackingTest} drives slope detection and chain + * reconstruction together and only passes when both happen to line up. This class decouples + * them via the debug-build-only test seams on {@code JavaProfiler}, reached via + * {@link ReferenceChainTestHooks} (backed by {@code LivenessTracker}'s existing + * {@code *ForTest} seams and {@code ReferenceChainTracker::tagAsRootForTest()}, see javaApi.cpp): *

      - *
    • {@link #shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope()} - asserts a slope signal - * would fire from directly-seeded population history, with no allocation sampling involved.
      • - *
    • {@link #shouldReconstructChainForDirectlyTaggedRoot()} - asserts the BFS/chain- - * reconstruction/event-emission path fires for a directly-tagged, known live object, with no - * dependency on {@code selectLeakCandidates()} organically picking the right klass.
      • + *
    • {@link #shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope()} - slope signal from + * directly-seeded population history, no allocation sampling involved.
      • + *
    • {@link #shouldReconstructChainForDirectlyTaggedRoot()} - BFS/chain-reconstruction/ + * event-emission for a directly-tagged, known live object, without depending on + * {@code selectLeakCandidates()} organically picking the right klass.
      • *
    * - *

    Only runs under -DDEBUG native builds (debug/asan/tsan) - the backing native methods are - * no-op stubs in a release build (javaApi.cpp's {@code #ifdef DEBUG} guard). + *

    Only runs under the debug native build ({@code testdebug}) - the backing native methods do + * not exist in a release build (see javaApi.cpp's {@code #ifdef DEBUG} guard), same pattern as + * {@code JVMAccessTest}. */ public class ReferenceChainTestSeamsTest extends AbstractProfilerTest { - // Arbitrary klass ids, opaque to the population table except when a representative is set: - // that aliases the representative's real class id onto the synthetic id, so the chain test - // targets a unique nested class - a real entry with existing (noisy) history, e.g. - // java.lang.Object's, would outrank the seeded ramp. + // Arbitrary, test-chosen klass ids. setKlassPopulationRepresentativeForTest() aliases the + // representative's REAL class id onto the seeded synthetic id, so the chain test must target + // a class whose real population entry does not already exist - otherwise the real entry's + // genuine history outranks the seeded ramp. private static final int SLOPE_TEST_KLASS_ID = 987001; private static final int CHAIN_TEST_KLASS_ID = 987002; - /** Unique class for {@link #shouldReconstructChainForDirectlyTaggedRoot()}'s target. */ private static final class ChainTarget {} - /** - * Durable root for {@link #shouldReconstructChainForDirectlyTaggedRoot()}'s target: the - * noise gate (suppressChainEvent) drops depth<2 chains whose root is transient (stack/JNI - * local), so a static holder is required for the chain event to be emitted at all. - */ + // The target must be reachable ONLY through this static holder (nulled in a finally so the + // next test in this no-forkEvery JVM does not inherit the reference). Binding it to a live + // local would make it a STACK_LOCAL root for the whole test body, and the noise gate + // (suppressChainEvent) drops depth<2 chains rooted in a transient stack slot - holding it + // statically (the canonical leak shape) is required for the chain event to be emitted at all. private static Object chainTargetHolder; @Override protected String getProfilerCommand() { - // generations=true: required for selectLeakCandidates() to return anything, real or seeded. - // referencechains=true: constructs the FrontierTable the seams insert into. - // framecap=2000000: the first pass's IterateOverReachableObjects admits every GC root in the - // JVM, and a small framecap fills with them before the tagged root is expanded, abandoning - // with FRONTIER_CAP and no chain event. + // framecap=2000000 (not 1024): the first BFS pass runs IterateOverReachableObjects, which + // admits every GC root in the whole JVM into the frontier; a small framecap fills with that + // ambient count before the tagged root gets to expandFrontier(), hitting FRONTIER_CAP with + // no chain event queued. Same value as ReferenceChainTrackingTest's success-path tests. return "generations=true,referencechains=true:hops=32:budget=500:ttl=60000:framecap=2000000"; } @@ -68,26 +67,21 @@ private static void assumeDebugBuild() { assumeTrue(ReferenceChainAssertions.isDebugInstrumentedBuild()); } - /** - * Seeds twenty epochs of strictly increasing population counts for {@link #SLOPE_TEST_KLASS_ID} - * directly into LivenessTracker's ring buffer - bypassing real allocation sampling entirely - - * then asserts {@code selectLeakCandidates()} ranks it as a leak candidate. This is the "assert - * a slope signal would be generated" seam: it proves the ranking logic itself works without - * depending on the real JVMTI heap sampler ever surfacing this specific klass. - */ + /** Seeds twenty epochs of strictly increasing population counts, then asserts + * {@code selectLeakCandidates()} ranks the klass as a leak candidate - proving the ranking + * logic works without the real JVMTI heap sampler ever surfacing this klass. */ @Test public void shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope() { assumeDebugBuild(); ReferenceChainTestHooks.resetKlassPopulationForTest0(); ReferenceChainTestHooks.setGcGenerationsEnabled0(true); - // KLASS_POPULATION_MIN_FILL_FOR_TREND = 10 only makes a trend eligible; selectLeakCandidates() - // also needs LEAK_TREND_HYSTERESIS_BASE = 5 consecutive qualifying epochs, so at least - // 10 + (5 - 1) = 14 increasing samples are needed; 20 gives headroom. + // selectLeakCandidates() requires consecutive_positive to reach LEAK_TREND_HYSTERESIS_BASE + // = 5 epochs on top of KLASS_POPULATION_MIN_FILL_FOR_TREND = 10, so at least 14 strictly + // increasing samples are needed; 20 gives headroom. for (int epoch = 1; epoch <= 20; epoch++) { ReferenceChainTestHooks.seedKlassPopulationSample0(SLOPE_TEST_KLASS_ID, epoch * 10, epoch); - // selectLeakCandidates() also requires a qualifying allocating thread. No tracked instance - // needs to match here, so a fixed synthetic tid suffices. + // Fixed synthetic tid suffices: this test only asserts the klass-level candidate list. ReferenceChainTestHooks.seedTidTrendSample0(SLOPE_TEST_KLASS_ID, 4242, epoch * 3, epoch); } @@ -103,69 +97,66 @@ public void shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope() { + " to be selected as a leak candidate after a seeded positive-slope population history"); } - /** - * Tags a real, live, caller-chosen object directly as a reference-chain frontier root - * (bypassing ReferenceChainTracker's normal root-seeded discovery walk), wires it in as a - * seeded leak candidate's representative, then drives one BFS pass and one poll cycle - * synchronously. This is the "trigger the refchain on a known live heap sample" seam: it - * proves {@code runPass()}/{@code pollWatchedTargets()}/{@code buildChainEvent()} correctly - * produce a chain event whose leaf is this test's {@link ChainTarget}, decoupled from whether - * LivenessTracker's probabilistic sampler would have picked the same object on its own. Only - * chains with a {@code ChainTarget} leaf count, so a chain another test left cached in this - * shared JVM cannot satisfy the assertion. - */ + /** Tags a live object directly as a frontier root (bypassing the normal root-seeded + * discovery walk), wires it in as a seeded leak candidate's representative, then drives + * pass/poll cycles synchronously - proving {@code runPass()}/{@code pollWatchedTargets()}/ + * {@code buildChainEvent()} produce a chain event for a target this test controls directly. */ @Test public void shouldReconstructChainForDirectlyTaggedRoot() { assumeDebugBuild(); ReferenceChainTestHooks.resetKlassPopulationForTest0(); ReferenceChainTestHooks.setGcGenerationsEnabled0(true); - // Guards against inheriting a full/tiny FrontierTable from an earlier test in this - // no-forkEvery JVM (e.g. ReferenceChainTrackingTest's framecap=1 abandonment test), against - // which tagAsReferenceChainRoot0()'s insert() could fail. + // Guards against inheriting a FrontierTable an earlier test in this same, no-forkEvery + // JVM left permanently full/tiny - e.g. ReferenceChainTrackingTest's + // shouldReportAbandonedSearchOnTinyFrontierCap drives the shared table to framecap=1 and + // leaves it that way; without the reset, insert() below can fail. ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); - // Referenced only through the static holder, never a local: a local would make the target a - // stack-local GC root for the whole test body, and the noise gate suppresses chains rooted in - // a transient stack slot. chainTargetHolder = new ChainTarget(); + try { + // Representative BEFORE seeding: setting it registers the synthetic->real id alias, so + // the seeding below lands re-keyed under the target's real class id. + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + CHAIN_TEST_KLASS_ID, chainTargetHolder); + + // See shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope()'s comment above for why 20 + // (not just KLASS_POPULATION_MIN_FILL_FOR_TREND = 10) is needed to clear the hysteresis gate. + for (int epoch = 1; epoch <= 20; epoch++) { + ReferenceChainTestHooks.seedKlassPopulationSample0(CHAIN_TEST_KLASS_ID, epoch * 10, epoch); + // Belt-and-braces: seed THIS thread's real tid so any path hinging on leak tagging + // of the target as a tracked instance finds a qualifying tid. + ReferenceChainTestHooks.seedTidTrendSample0( + CHAIN_TEST_KLASS_ID, ReferenceChainTestHooks.getTid(), epoch * 3, epoch); + } - // Representative before seeding: setting it registers the synthetic->real id alias, so the - // seeds land under the target's real class id. - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(CHAIN_TEST_KLASS_ID, chainTargetHolder); - - // 20 epochs to clear the hysteresis gate - see the slope test above. - for (int epoch = 1; epoch <= 20; epoch++) { - ReferenceChainTestHooks.seedKlassPopulationSample0(CHAIN_TEST_KLASS_ID, epoch * 10, epoch); - // This thread's real tid, since the target was allocated here. Belt-and-braces: the - // chain's primary path is the directly-tagged root plus the seeded representative. - ReferenceChainTestHooks.seedTidTrendSample0( - CHAIN_TEST_KLASS_ID, ReferenceChainTestHooks.getTid(), epoch * 3, epoch); - } - - // Poll before the direct tag: it admits the seeded klass as a watched candidate, and - // tagAsRootForTest's recordDiscoveredInstance only lands when a candidate slot already - // watches the klass. - ReferenceChainTestHooks.pollReferenceChainTargets0(); - - long tag = ReferenceChainTestHooks.tagAsReferenceChainRoot0(chainTargetHolder); - assertTrue(tag > 0, "Expected tagAsReferenceChainRoot0 to assign a valid frontier tag"); - - // One pass is not enough: admitStaticFieldRoots() sweeps loaded classes in budget-bounded - // chunks, and the target's durable root_kind is only discovered on the pass whose chunk - // covers ChainTarget's holder class. 40 passes give several full sweep laps of headroom - // (~10 passes for ~4000 classes). The engine lock (runPassSerialized/ - // pollWatchedTargetsSerialized) keeps the BFS thread from racing these calls. - int eventCount = 0; - for (int pass = 0; pass < 40 && eventCount == 0; pass++) { - boolean sawPassRun = ReferenceChainTestHooks.runReferenceChainPass0(); - assertTrue(sawPassRun || pass > 0, - "Expected the first runReferenceChainPass0 to run (reference chains enabled)"); + // One poll BEFORE the direct tag: the first poll admits the seeded klass as a watched + // candidate, and tagAsRootForTest's recordDiscoveredInstance only lands when a candidate + // slot already watches the klass - the marker-tag path the original pre-poll tag rode is + // gone in the leak-tag redesign. ReferenceChainTestHooks.pollReferenceChainTargets0(); - eventCount = ReferenceChainTestHooks.drainReferenceChainEventCountForLeaf0(chainTargetHolder); + + long tag = ReferenceChainTestHooks.tagAsReferenceChainRoot0(chainTargetHolder); + assertTrue(tag > 0, "Expected tagAsReferenceChainRoot0 to assign a valid frontier tag"); + + // One pass is NOT enough: admitStaticFieldRoots() sweeps loaded classes in chunks and + // the first pass truncates before reaching this test's class - the durable static-field + // root discovery that upgrades the target's root_kind only happens on the pass whose + // chunk covers ChainTarget's holder class. 40 gives the full sweep lap (~10 passes for + // ~4000 loaded classes) several round trips of headroom; the engine lock keeps the BFS + // thread from racing these calls. + int eventCount = 0; + for (int pass = 0; pass < 40 && eventCount == 0; pass++) { + boolean sawPassRun = ReferenceChainTestHooks.runReferenceChainPass0(); + assertTrue(sawPassRun || pass > 0, + "Expected the first runReferenceChainPass0 to run (reference chains enabled)"); + ReferenceChainTestHooks.pollReferenceChainTargets0(); + eventCount = ReferenceChainTestHooks.drainReferenceChainEventCount0(); + } + assertTrue(eventCount > 0, + "Expected pollWatchedTargets() to have queued at least one chain event for the " + + "directly-tagged, seeded-representative target"); + } finally { + chainTargetHolder = null; // shared-JVM hygiene } - assertTrue(eventCount > 0, - "Expected pollWatchedTargets() to have queued at least one chain event whose leaf is " - + "ChainTarget (the directly-tagged, seeded-representative target)"); - chainTargetHolder = null; // shared-JVM hygiene } } 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 867e02af18..28de9d6789 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 @@ -6,10 +6,10 @@ package com.datadoghq.profiler.referencechains; import com.datadoghq.profiler.AbstractProfilerTest; -import com.datadoghq.profiler.ReferenceChainTestHooks; import com.datadoghq.profiler.JfrEvent; import com.datadoghq.profiler.JfrEvents; import com.datadoghq.profiler.Platform; +import com.datadoghq.profiler.ReferenceChainTestHooks; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Tag; @@ -20,12 +20,12 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import java.lang.reflect.Method; import java.util.Map; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -33,116 +33,169 @@ import static org.junit.jupiter.api.Assertions.assertTrue; /** - * End-to-end coverage for {@code ReferenceChainTracker} (referenceChains.h/.cpp) against a real - * JVM, real GCs and a real JVMTI heap walk. {@code Profiler::dump()} emits - * {@code datadog.ReferenceChainAbandoned} once the search has ended in - * {@code SearchState::ABANDONED}; {@code pollWatchedTargets()} emits {@code datadog.ReferenceChain} - * for ranked {@code selectLeakCandidates()} klasses whose representative a {@code runPass()} walk - * has already tagged ({@code getTag() > 0} - a read, never a {@code SetTag} seed). + * PROF-15341 (+ lifecycle-wiring follow-up): end-to-end coverage for + * {@code ReferenceChainTracker} (ddprof-lib/src/main/cpp/referenceChains.h/.cpp). + * + *

    {@code ReferenceChainTracker::start()} is called from {@code Profiler::start()}, gated on + * {@code args._reference_chains}, followed by {@code startThread()} which spawns its BFS thread; + * {@code Profiler::stop()} calls the matching {@code stopThread()}/{@code stop()} pair. A + * {@code datadog.ReferenceChainAbandoned} event reaches a live {@code Recording}: {@code + * Profiler::dump()} calls {@code buildAbandonedEvent()} and writes its output whenever the search + * ended in {@code SearchState::ABANDONED} - mirroring {@code LivenessTracker::flush()}'s call site + * there. {@link #shouldReportAbandonedSearchOnTinyFrontierCap()} exercises that path end-to-end. * - *

    Why {@code @TestMethodOrder}: {@code ReferenceChainTracker} is a process-wide - * singleton. Leaving {@code SearchState::RUNNING} releases every tag the search assigned - * ({@code releaseSearchTags()}), and its {@code FrontierTable} is sized only by the first - * {@code start()} in this JVM ({@code if (_frontier == nullptr)} guard; no {@code forkEvery}, see - * {@code ProfilerTestPlugin.kt}). {@link #shouldReportAbandonedSearchOnTinyFrontierCap()} drives - * the search to {@code ABANDONED}, while both success-path tests need it {@code RUNNING} to see - * non-zero tags and have no fallback for losing that race, so they run first - * ({@code @Order(1)}/{@code @Order(2)}). Their relative order does not matter: they target - * different klasses ({@link ChainLink} vs {@link CachedPayload}) and neither exhausts the search. + *

    Target-selection bridging: {@code pollWatchedTargets()} (referenceChains.cpp), called + * from {@code threadLoop()} once per scheduling cycle after {@code runPass()}, polls + * {@code LivenessTracker::selectLeakCandidates()} (positive population-slope ranking over a + * rolling per-klass survivor-count window, gated on {@code _gc_generations}) and, for each ranked + * klass whose representative instance an ordinary {@code runPass()} walk has *already* tagged + * ({@code getTag() > 0} - a read, never a {@code SetTag} seed), reconstructs and emits its chain + * via {@code Profiler::writeReferenceChain()}. {@link #shouldReconstructReferrerChainToGcRoot()} + * exercises this end to end against a real JVM, real GCs, and a real JVMTI heap walk - the + * synthetic-frontier reconstruction-correctness proof lives in + * {@code referenceChainJfrRoundtrip_ut.cpp}/{@code ReferenceChainJfrParserTest}. + * + *

    Why {@code @TestMethodOrder}/{@code @Order(1)}: {@code ReferenceChainTracker} is a + * process-wide singleton whose single search never restarts once it leaves + * {@code SearchState::RUNNING} - {@code shouldRunPass()} returns {@code false} forever after that + * point, and the transition releases every tag the search ever assigned. Its {@code FrontierTable} + * is sized once, the first time any test in this JVM calls {@code ReferenceChainTracker::start()} + * (the {@code if (_frontier == nullptr)} guard), and never resized on later start()/stop() cycles + * - so whichever test runs first also fixes that capacity for every test after it (no + * {@code forkEvery} configured, so this whole class runs in one JVM). The abandonment test + * deliberately drives that shared search to {@code ABANDONED}; both success-path tests need it + * still {@code RUNNING} to find their own candidates' tags non-zero, so they are pinned first via + * {@code @Order(1)}/{@code @Order(2)}. Their relative order between themselves does not matter - + * they target different klasses within the same shared search and neither exhausts it - only + * both running before the abandonment test does. */ @TestMethodOrder(MethodOrderer.OrderAnnotation.class) @Tag("slow") public class ReferenceChainTrackingTest extends AbstractProfilerTest { - // Arbitrary klass ids for the debug-only population-seeding seams. Distinct from each other and - // from ReferenceChainTestSeamsTest's ids to avoid collisions in the shared, no-forkEvery JVM. + // Arbitrary, test-chosen klass ids for the debug-only population-seeding seams below - + // LivenessTracker's population table treats them as opaque keys. Distinct per test purely as + // cheap insurance against collision in a shared, no-forkEvery test JVM. private static final int CHAIN_LINK_TEST_KLASS_ID = 987201; private static final int CACHED_PAYLOAD_TEST_KLASS_ID = 987202; - // Static, not local: the discovered-chain gate suppresses shallow chains rooted at a transient - // root (stack/JNI local), and ChainLinks at depth 1 of a locally-held list are exactly that - // shape. (CachedPayload chains go three hops through HashMap internals, which the gate lets - // through regardless of root kind.) Cleared in the test's finally so it does not keep its - // population reachable for later tests in this shared JVM. + // Durable (static-field) holder for the gc-root fixture below. The discovered-chain gate + // suppresses chains shallower than the first real holder hop that are rooted at a TRANSIENT + // root (stack/JNI local) - a frame-held "leak" is by definition not a leak - and the ChainLink + // instances sit at depth 1 of a locally-held list, exactly that shape. A static field gives + // them the real unmaintained-singleton-collection retention shape instead. (The cache fixture + // needs no such change: CachedPayload chains are three hops deep through HashMap's own + // internals, which the gate lets through regardless of root kind.) Cleared in the test's + // finally - a leftover holder would keep its entire population reachable for every later test + // in this shared JVM. private static List gcRootHolder; - // consecutive_positive only starts incrementing once ring_fill reaches - // KLASS_POPULATION_MIN_FILL_FOR_TREND=10 (ringThirdsStats()'s fill < min_fill guard resets it to - // 0 before that), and selectLeakCandidates() needs it at LEAK_TREND_HYSTERESIS_BASE=5 (or 3 when - // heapFloorRising() corroborates). hasLeakSignal() - which shouldRunPass() gates even the first - // pass on under generations=true - consults that. 10 + 5 - 1 = 14 seeded epochs is the minimum; - // +1 for margin. + // The current round's allocation batch, handed to the persistent allocator thread below. + // Volatile: written here, read by the allocator thread every round - the request queue's + // happens-before already covers it, but the visibility of a plain field handed across threads + // is kept explicit rather than relying on queue semantics alone. + private static volatile List currentBatch; + + // LivenessTracker's hysteresis gate (KLASS_POPULATION_MIN_FILL_FOR_TREND=10 / + // LEAK_TREND_HYSTERESIS_BASE=5) only starts incrementing consecutive_positive once ring_fill + // has reached 10 - every call before that resets it back to 0. A single batch of 10 seed + // calls therefore caps consecutive_positive at 1, far short of the >=5 that + // selectLeakCandidates() requires. 10 + 5 - 1 = 14 calls is the minimum for the 5 qualifying + // calls at ring_fill>=10 (epochs 10-14) to land; +1 for margin against off-by-one uncertainty + // in that derivation. private static final int SEED_EPOCHS_FOR_HYSTERESIS = 15; - // Dedicated id for shouldReportAbandonedSearchOnTinyFrontierCap()'s seeding, for the same - // collision-avoidance reason as the ids above. + // Dedicated id for shouldReportAbandonedSearchOnTinyFrontierCap()'s own seeding (see that + // method's own comment) - distinct from CHAIN_LINK_TEST_KLASS_ID/CACHED_PAYLOAD_TEST_KLASS_ID + // for the same cheap-insurance-against-collision reason those two are distinct from each other. private static final int ABANDON_TEST_KLASS_ID = 987203; - // T_REFERENCE_CHAIN in jfrMetadata.h - the jdk.ActiveSetting "id" written by flightRecorder.cpp's - // writeBoolSetting(buf, T_REFERENCE_CHAIN, "enabled", ...). Must stay in sync with that enum. + /** + * The {@code datadog.ReferenceChain} ActiveSetting's {@code id} (flightRecorder.cpp's + * T_REFERENCE_CHAIN). ActiveSetting events from cpu/wall/memory/nativemem also read + * "enabled"="true", so the id is the only unambiguous discriminator. + */ private static final long REFERENCE_CHAIN_EVENT_ID = 129; + // Upper bound on any single main-thread wait for the persistent allocator thread: a + // dead/stuck allocator must fail the test, not hang the shared test fork. + private static final long ALLOCATOR_HANDOFF_TIMEOUT_MS = 30_000; + @Override protected String getProfilerCommand() { String testName = testInfo != null - ? testInfo.getTestMethod().map(Method::getName).orElse("") + ? testInfo.getTestMethod().map(java.lang.reflect.Method::getName).orElse("") : ""; if ("shouldReconstructReferrerChainToGcRoot".equals(testName) || "shouldReconstructReferrerChainThroughUnboundedCacheLeak".equals(testName)) { - // generations=true: pollWatchedTargets() does nothing unless - // LivenessTracker::gcGenerationsEnabled() is true; referencechains=... alone stays - // whole-graph-only. A large framecap avoids FRONTIER_CAP abandonment against this JVM's - // real root-reachable graph, and a long ttl avoids TTL abandonment mid-test - either would - // call releaseSearchTags() and zero every tag this test depends on. - // - // pausetarget=5000: measured FollowReferences/GetObjectsWithTags latency starts ~150ms and - // climbs past 1s as cumulative progress grows (each pass re-scans the discovered set). - // Against the default 5ms ceiling (DEFAULT_REFERENCE_CHAINS_PAUSE_TARGET_MS) updatePacing() - // settles at MIN_EFFECTIVE_BUDGET=50 edges/pass and MAX_EFFECTIVE_CADENCE_NS=4s, too slow - // for this test's wall-clock budget; a merely generous ceiling only postpones that. A - // ceiling above the expected peak per-pass cost keeps _effective_budget at budget=4000. + // generations=true + a small sampling interval: pollWatchedTargets() does nothing without + // LivenessTracker::gcGenerationsEnabled(), and the small interval maximizes the chance the + // ChainLink instances allocated below actually get liveness-tracked. A large framecap avoids + // an early FRONTIER_CAP abandonment against this JVM's real, not-controlled-by-this-test + // root-reachable graph, and a long ttl avoids a TTL abandonment mid-test - either would call + // releaseSearchTags() and permanently zero every tag this test depends on. // - // painbudget=100: the @Order(2) test needs a search restart (canAffordNewSearch()) after - // the @Order(1) search terminates. PainBudget (painBudget.h) gates that on - // pain_spent_ms / refill_rate of cooldown, which at the default 1% refill can run to - // minutes - past this test's bounded retry loop. Nothing else here competes for the budget. + // pausetarget=5000: the pacing controller measures this JVM's real FollowReferences call + // latency at ~150ms early on, climbing past a full second as this test's own allocation + // keeps handing the search more to discover (each pass re-scans the whole already-discovered + // set, so cost grows with cumulative progress, not with this test's allocation rate). Against + // the default 5ms ceiling every observed pass is "over ceiling even at the floor", so the + // controller settles at MIN_EFFECTIVE_BUDGET=50 edges/pass and 4s between passes - a real but + // glacial rate this test's wall-clock budget cannot outwait, and one a merely-generous + // ceiling (e.g. 500ms) only postpones: once cumulative progress pushes the real per-pass cost + // past *that* ceiling too, the same throttling recurs. A ceiling comfortably above the + // highest per-call cost this test's population growth is expected to reach lets the + // controller keep the configured budget=4000 for the whole run. // - // firstpassbudget=200000: root enumeration (runPassManualWalk()'s - // IterateOverReachableObjects) otherwise shares budget=4000, which was observed exhausting - // on other roots' subtrees before ever admitting an edge from this thread's stack (where - // the cache test's HashMap lives). Raising only the root-enumeration budget leaves the - // steady-state expansion pacing (updatePacing()) untouched. + // painbudget=100: the cache-leak test shares this singleton search and needs a restart once + // the first search reaches a terminal state. PainBudget gates that restart on a cooldown of + // pain_spent_ms / refill_rate; the default 1% refill can stretch that to minutes for a + // search this large - far past this test's bounded retry loop. Nothing else in this JVM + // competes for the pain budget, so keep the gate affordable immediately. // - // memory=64:l:1.0 - the explicit 100% live-sample ratio is load-bearing: the default 10% - // (_live_samples_ratio, arguments.h) makes LivenessTracker::track() drop 90% of tracked - // instances, thinning the representative and trend pools (observed as intermittent - // "representative died/evicted" and zero-tag runs). Tracking volume is still capped by the - // 256KiB sampling-interval floor, so the table stays small. + // firstpassbudget=200000: root/stack-ref enumeration shares budget=4000 with expandFrontier() + // unless overridden - and in this JVM's real root-reachable graph that left this method's own + // cache HashMap (a direct stack-local root) at the mercy of whatever order JVMTI enumerates + // roots in: budget=4000 was observed exhausting itself on other roots' subtrees every single + // attempt, never once admitting a single edge from this thread's own stack. Overriding just + // root enumeration's budget gives every enumeration attempt enough headroom to reach this + // thread's stack without slowing the steady-state expansion passes that follow. + // memory=64:l:1.0 - the explicit 100% live-sample ratio is load-bearing: without a ratio + // segment the default is 10% and LivenessTracker::track() drops 90% of tracked instances + // probabilistically, thinning the pool the representative-minting and candidate trend rings + // draw from - observed live as intermittent "representative died/evicted" and zero-tag runs. + // Total tracking volume is still capped by the 256KiB sampling-interval floor, not the + // ratio, so 100% keeps the tracking table tiny. return "memory=64:l:1.0,generations=true," + "referencechains=true:hops=64:budget=4000:ttl=120000:framecap=2000000:pausetarget=5000:painbudget=100:firstpassbudget=200000"; } - // shouldReportAbandonedSearchOnTinyFrontierCap needs the frontier table, not the per-pass - // budget, to run out first: heapReferenceCallback() checks the budget before - // FrontierTable::insert(), so budget<=framecap completes normally instead of abandoning. A - // larger budget lets a second root-referenced object hit the full table, which runPass() - // abandons on. + // shouldReportAbandonedSearchOnTinyFrontierCap needs the frontier table (not the per-pass + // budget) to run out first: heapReferenceCallback() checks the budget before ever calling + // FrontierTable::insert(), so a budget as small as the cap itself only exhausts the budget + // after admitting exactly one object - insert() never gets a chance to fail. A budget + // comfortably larger than framecap=1 lets a *second* root-referenced object reach insert() + // and hit the actually-full table, which is what runPass() treats as grounds to abandon. // - // framecap=1 only sizes the table if this is the first start() in the JVM - // (`if (_frontier == nullptr)` guard); otherwise @Order(1)'s framecap=2000000 wins. ttl=100 is - // the fallback trigger: _search_start_ns is set when the shared search first started, so on an - // inherited oversized table the ttl check fires almost immediately. Either path produces - // datadog.ReferenceChainAbandoned, which is all this method asserts on. + // framecap=1 only controls the table's capacity the *first* time start() constructs it in + // this JVM (the `if (_frontier == nullptr)` guard - the table survives every later start()/ + // stop() cycle at whatever size it was first built with). The @Order(1) test's much larger + // framecap=2000000 wins that race instead - so this command also supplies a small ttl as a + // second, independent abandonment trigger. A fresh, genuinely-tiny table (this method running + // first, e.g. in isolation) still abandons via FRONTIER_CAP; inheriting an already-oversized + // table falls through to the ttl check, which fires almost immediately regardless of table + // size because _search_start_ns is set once, the first time the shared search ever started. + // Either path produces a datadog.ReferenceChainAbandoned event, which is all this method + // asserts on. // - // painbudget=100: restartSearch()/resetSearchStateForTest() both call - // _pain_budget.spend(_search_pain_ms) unconditionally, so the reset below inherits the - // previous long-running search's pain debt; at the default 1% refill, draining it back to - // canStartNow()==true takes minutes, past this method's retry budget. - // - // memory=64:l:1.0 is load-bearing: Profiler::start() only starts the ReferenceChainTracker - // when allocation sampling activated (activated & EM_ALLOC); a bare referencechains=... falls - // back to cpu-only and never runs the search. No generations=true: LivenessTracker::initialize() - // then clears _gc_generations, so hasLeakSignal() returns true without any population trend. + // painbudget=100: this method's own resetReferenceChainSearchForTest0() spends whatever + // _search_pain_ms the earlier long-running search left behind into *this* method's PainBudget + // (restartSearch()/resetSearchStateForTest() both call _pain_budget.spend(_search_pain_ms) + // unconditionally) - at the default 1% refill, draining that debt would take minutes. if ("shouldReportAbandonedSearchOnTinyFrontierCap".equals(testName)) { + // memory=64:l:1.0 is load-bearing: Profiler::start() only starts the ReferenceChainTracker + // when allocation sampling activated (activated & EM_ALLOC); a bare referencechains=... falls + // back to cpu-only and never runs the search. No generations=true: LivenessTracker::initialize() + // then clears _gc_generations, so hasLeakSignal() returns true without any population trend. return "memory=64:l:1.0,referencechains=true:hops=32:budget=500:framecap=1:ttl=100:painbudget=100"; } // shouldExposeReferenceChainsSettingWhenEnabled: a referencechains-only command. Arguments::parse() @@ -162,7 +215,7 @@ protected boolean isPlatformSupported() { /** * Verifies the {@code referencechains=...} flag round-trips through * {@code Arguments} parsing (arguments.cpp's {@code CASE("referencechains")}) into the - * {@code datadog.ReferenceChain} JFR setting (flightRecorder.cpp's + * {@code datadog.ReferenceChain} JFR setting (flightRecorder.cpp:1143's * {@code writeBoolSetting(buf, T_REFERENCE_CHAIN, "enabled", args._reference_chains)}). */ @Test @@ -182,99 +235,176 @@ public void shouldExposeReferenceChainsSettingWhenEnabled() { } /** - * Allocates a growing, statically-held population of {@link ChainLink} instances with a GC and - * dump between rounds until {@code selectLeakCandidates()} trusts the trend, then waits for - * {@code pollWatchedTargets()} to emit the candidate's chain once a {@code runPass()} walk has - * tagged it. + * Allocates a growing population of {@link ChainLink} instances, forcing GCs between rounds so + * LivenessTracker observes a rising per-klass survivor count each time, until + * {@code selectLeakCandidates()} trusts the trend - then waits for {@code pollWatchedTargets()} + * to notice that candidate has already been tagged by an ordinary walk and reconstruct + emit + * its chain, asserting on the resulting {@code datadog.ReferenceChain} event. * - *

    Each round needs an explicit {@link #dump(Path)}: {@code cleanup_table()}'s per-klass - * population accounting only runs (unforced) via {@code flush_table()} from - * {@code LivenessTracker::flush()}, which is called only from {@code Profiler::dump()} - there - * is no timer-driven flush. {@code System.gc()} bumps {@code _gc_epoch} synchronously in the - * {@code GarbageCollectionFinish} callback, so no sleep is needed between the GC and the dump. + *

    Each round pairs with an explicit {@link #dump(Path)}: LivenessTracker's per-klass + * population accounting only runs (unforced) from {@code flush_table()}, which only runs from + * {@code LivenessTracker::flush()}, called *only* from {@code Profiler::dump()} - there is no + * timer-driven flush. {@code System.gc()} bumps the GC epoch synchronously inside the + * {@code GarbageCollectionFinish} callback, so it is already visible by the time {@code gc()} + * returns - no sleep needed between the gc() and the dump() that observes it. */ @Test @Order(1) public void shouldReconstructReferrerChainToGcRoot() throws Exception { - // Seed one element *before* resetting the search: ArrayList's backing array starts as the - // shared empty sentinel, and expandFrontier() freezes a node's children when it is expanded - // and never re-examines it. An unrelated GC can wake the restarted BFS thread before round 1's - // addAll(); expanding the holder while still empty would leave every later ChainLink - // unreachable (seen in CI as tag=0 on every poll). Every later grow() copies index 0, so - // whichever array the walk snapshots is non-empty. + // Per-run unique path, not a fixed name in the CWD: parallel workers or a shared checkout can + // never read each other's recording, and an aborted run leaves nothing a later run inherits. + // Created first - before any static holder, reset, or allocator thread - so an IOException here + // leaks nothing; the placeholder is deleted so dump() creates the file itself. + Path scratchDumpPath = Files.createTempFile("referencechains-population-scratch-", ".jfr"); + Files.deleteIfExists(scratchDumpPath); + // Seed gcRootHolder with one live element *before* resetting the search below: ArrayList's + // backing array starts as the shared empty-array sentinel and is only replaced on its first + // grow(). expandFrontier() freezes whatever children it observes for a node the moment that + // node is expanded - it never re-examines an already-EXPANDED node. If gcRootHolder's node + // gets expanded while elementData still points at the empty sentinel (some unrelated GC can + // wake the freshly restarted BFS thread in the gap between the reset and round 1's addAll()), + // the frozen children set would be permanently empty and no ChainLink added later would ever + // become reachable - matching a real CI failure where pollWatchedTargets() reported tag=0 for + // this klass on every poll. Seeding one element first means the backing array is never empty + // after the search (re)starts: every later grow() copies all prior elements into the new + // array, so index 0 is always present in whichever array the walk snapshots. gcRootHolder = new ArrayList<>(); gcRootHolder.add(new ChainLink("gc-root-seed")); if (ReferenceChainAssertions.isDebugInstrumentedBuild()) { - // @Order(1) only orders this class: another test class in this no-forkEvery JVM may have - // left the singleton search non-RUNNING, after which runPass() never takes its root-seeded - // walk again. The population reset is needed for the same reason: _klass_population - // survives stop()/start(), and a leftover klass already past the hysteresis gate would let - // the first pass start regardless of this method's own seeding, masking whether it is - // sufficient. Debug-only: these native seams do not exist in a release build. + // Being pinned first within this class does not make this the first reference-chain test in + // the whole shared JVM - another test class can run first and leave the singleton search + // already non-RUNNING/already-tagged, in which case runPass() never takes its real + // root-seeded-walk branch again. Force a genuine fresh search so this test's ChainLink + // population is guaranteed reachable by a real walk. Debug-only: the seam does not exist in + // a release build. + // + // resetKlassPopulationForTest0() first: LivenessTracker's _klass_population table survives + // stop()/start() cycles, so an earlier test class could leave a klass already past the + // consecutive_positive hysteresis gate sitting in that table - letting this test's first + // pass start for a reason unrelated to the seeding below, masking whether that seeding is + // actually sufficient on its own. ReferenceChainTestHooks.resetKlassPopulationForTest0(); ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); } - // Every ChainLink is allocated by one persistent thread - see PersistentAllocator. - PersistentAllocator allocator = new PersistentAllocator("togcroot-leak-allocator"); + // One persistent allocator thread, reused by every round - NOT a fresh thread per round. + // SampledObjectAlloc never fires for this JUnit worker thread's own allocations, but + // reliably fires for threads created after Profiler::start() already ran; and per-tid, the + // per-(klass, tid) qualification trend accumulates REAL samples for this one thread's tid + // every round, so qualification is organic and cannot age out mid-test. Per-round spawns + // gave every round a different short-lived tid, so the seeded ramp was the only qualifying + // signal - and once its ring slots aged out the candidate dropped out of + // selectLeakCandidates() with the whole crawl still unfinished. + LinkedBlockingQueue allocRequests = new LinkedBlockingQueue<>(); + Semaphore roundDone = new Semaphore(0); + AtomicLong allocatorTid = new AtomicLong(-1); + AtomicReference allocatorFailure = new AtomicReference<>(); + Thread allocator = new Thread(() -> { + try { + allocatorTid.set(ReferenceChainTestHooks.getTid()); + Integer requested; + while ((requested = allocRequests.take()) != null) { + List batch = currentBatch; + for (int i = 0; i < requested; i++) { + batch.add(new ChainLink("leak-" + requested + "-" + i)); + } + roundDone.release(); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); // test shutdown - see finally below + } catch (Throwable t) { + allocatorFailure.set(t); // surfaced by awaitAllocatorTid()/awaitAllocatorRound() + } + }, "togcroot-leak-allocator"); + allocator.start(); - Path scratchDumpPath = Paths.get("referencechains-population-scratch.jfr"); try { - // KLASS_POPULATION_MIN_FILL_FOR_TREND = 10 needs 10 epochs that actually observe a surviving - // ChainLink sample, and sampling is probabilistic, so the loop grows per-round counts and - // checks every round instead of committing to a fixed count. Staying under - // KLASS_POPULATION_RING_SIZE = 30 keeps every round within the slope window. Capped at 16 - // because the shared fork (-Xmx512m, no forkEvery) hit "Java heap space" at 25. + // The seeding block below needs the allocator's tid. Bounded and inside this try so a + // dead/stuck allocator fails the test (and is still shut down by the finally) instead of + // hanging the shared fork. + awaitAllocatorTid(allocator, allocatorTid, allocatorFailure); + // Up to 16 rounds: KLASS_POPULATION_MIN_FILL_FOR_TREND = 10 needs 10 *epochs that actually + // observe a surviving ChainLink sample*, not just 10 dump() calls - allocation sampling is + // probabilistic, so some early, smaller rounds may not land a single ChainLink sample. + // Growing the per-round count compensates, and the loop keeps going (checking for the event + // every round) rather than committing to a fixed round count. Staying under + // KLASS_POPULATION_RING_SIZE = 30 keeps every round's sample within the trend window. + // Capped at 16 rather than a larger margin: this loop's worst case runs inside the shared + // forked test JVM every other ddprof-test class shares (-Xmx512m, no forkEvery) - a prior + // CI run hit "Java heap space" with the cap at 25. // - // Growth is clamped at round 10: later rounds only add retries against the lock-contention - // race below (the trend is already eligible), and unclamped growth was what exhausted the - // heap. + // Per-round growth is clamped to round 10: rounds past 10 exist only to give + // pollWatchedTargets() more retries against the lock-contention race below, not to keep + // building the population trend (already eligible by round 10) - unclamped rounds 11-16 + // each add strictly more retained garbage for no trend benefit. // - // Sized against ObjectSampler::check()'s 256KiB minimum interval, not "memory=64" (which - // only affects LivenessTracker's table capacity). ChainLink's padding keeps the instance - // count - and so the graph the BFS walk must traverse - small. Other klasses (e.g. byte[]) - // may also be flagged, so matching is by ChainLink specifically. + // Instance count is sized against ObjectSampler::check(), not this test's "memory=64" + // request: the *actual* sampling interval is 256KiB regardless of the small value requested + // (that request only affects LivenessTracker's table-capacity formula). ChainLink's padding + // fields get enough megabytes sampled from far fewer instances than plain ~40-byte + // instances would need - keeping this test's contribution to the root-reachable graph (which + // the BFS walk must also traverse) from ballooning past the walk's wall-clock budget. + // ChainLink competes on equal footing against incidental klasses (e.g. byte[]) this same + // mechanism may also flag - the assertions below look for ChainLink among however many + // events actually appear, rather than assuming it is the only one. ReferenceChainAssertions.JfrChainMatch match = null; boolean seededTestKlassTrend = false; int totalRounds = 16; for (int round = 1; round <= totalRounds && match == null; round++) { int newInstances = Math.min(round, 10) * 600; List newLinks = new ArrayList<>(newInstances); - allocator.run(() -> { - for (int i = 0; i < newInstances; i++) { - newLinks.add(new ChainLink("leak-" + newInstances + "-" + i)); - } - }); + currentBatch = newLinks; // published before the request - see the allocator's loop + if (!allocRequests.offer(newInstances)) { + throw new IllegalStateException( + "allocRequests queue rejected instance count " + newInstances); + } + awaitAllocatorRound(roundDone, allocator, allocatorFailure); gcRootHolder.addAll(newLinks); System.gc(); dump(scratchDumpPath); match = ReferenceChainAssertions.findMatchForClass(verifyEvents(scratchDumpPath, "datadog.ReferenceChain", false), ChainLink.class); if (match == null && ReferenceChainAssertions.isDebugInstrumentedBuild()) { - // Under generations=true, shouldRunPass() gates even the first pass on hasLeakSignal(), - // which this early falls through to selectLeakCandidates()'s hysteresis gate for some - // klass. Seeding SEED_EPOCHS_FOR_HYSTERESIS epochs clears it, authorizing the BFS - // thread's next wake to run the root-seeded walk. The poll below is therefore - // best-effort: it only reconstructs once the representative's getTag() > 0, so the - // retries give the background thread's own post-pass pollWatchedTargets() time to run. + // Debug-only native seams - a no-op in release builds, so release-build coverage of the + // organic path below is unchanged. // - // Seed exactly once: recordKlassPopulationSampleLocked() always appends a ring slot, so - // reseeding would turn the ramp into a sawtooth and destroy the positive slope. + // With generations=true, shouldRunPass() gates even the search's very first pass on + // hasLeakSignal(), which falls through to selectLeakCandidates() requiring + // consecutive_positive >= LEAK_TREND_HYSTERESIS_BASE for *some* klass (see + // SEED_EPOCHS_FOR_HYSTERESIS's comment for why 10 seeded epochs alone cannot clear that + // bar). Seeding SEED_EPOCHS_FOR_HYSTERESIS epochs clears it outright, which is what + // authorizes the background BFS thread's next scheduled wake to run its real pass - + // nothing before that point has tagged gcRootHolder's own ChainLink yet. The explicit + // pollReferenceChainTargets0() right below is therefore best-effort: it only reconstructs + // a chain once the representative's getTag() is actually > 0, which needs that background + // pass to have already run. The retry loop below (sleep-then-dump fallback, repeated + // across rounds) gives the now-unblocked background thread room to do that and have its + // own post-pass poll pick the chain up on its own. // - // Representative before seeding: setting it aliases the synthetic id to the - // representative's real klass id, so the seeds land in the real population entry - // (seeds under a bare synthetic id satisfy only hasLeakSignal()'s candidate count). - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(CHAIN_LINK_TEST_KLASS_ID, gcRootHolder.get(0)); + // Seed exactly once: recordKlassPopulationSampleLocked() always *appends* a fresh ring + // slot rather than overwriting one for a repeated epoch - reseeding on a later round + // would append a second ramp right after the first, turning the ring into a non-monotonic + // sawtooth and destroying the positive-slope signal. Only the poll+dump recheck below + // needs to repeat across rounds. + // Representative BEFORE seeding: setKlassPopulationRepresentativeForTest0 resolves the + // representative's real klass id and aliases the synthetic id to it, so the seeds below + // land in the real population entry (candidate matching is real-id keyed since the + // leak-tag pool redesign). + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + CHAIN_LINK_TEST_KLASS_ID, gcRootHolder.get(0)); if (!seededTestKlassTrend) { - // The allocator thread owns every ChainLink allocation, so its tid is the qualifying - // one; the seeds accelerate the organic per-tid growth it produces every round. - int leakTid = allocator.tid(); - // Seed counts match the organic ramp (+1 per epoch): hasQualifyingGrowth() compares - // third-window means over seeds followed by real folds, and inflated seeds (epoch*10) - // made the slope go negative (-20..-40 observed) once real samples displaced them. - // With matched scale ([1..15, 1, 2, 3, ...]) every window pairing stays positive. + // The persistent allocator thread owns every ChainLink allocation, so ITS tid is the + // qualifying one. Seed counts scale as the organic ramp itself (+1 per epoch), NOT + // some inflated multiple: hasQualifyingGrowth() takes the difference of third-window + // means over the whole seed+real series, and seeds of epoch*10 dwarfing the real + // ~1..15 counts made the slope go NEGATIVE (-20..-40 observed live, the "transition + // cliff") the moment real samples displaced the seeded tail. With matched-scale seeds + // every third-window pairing across the transition still yields a positive slope. + int leakTid = (int) allocatorTid.get(); for (int epoch = 1; epoch <= SEED_EPOCHS_FOR_HYSTERESIS; epoch++) { - ReferenceChainTestHooks.seedKlassPopulationSample0(CHAIN_LINK_TEST_KLASS_ID, epoch, epoch); - ReferenceChainTestHooks.seedTidTrendSample0(CHAIN_LINK_TEST_KLASS_ID, leakTid, epoch, epoch); + ReferenceChainTestHooks.seedKlassPopulationSample0( + CHAIN_LINK_TEST_KLASS_ID, epoch, epoch); + ReferenceChainTestHooks.seedTidTrendSample0( + CHAIN_LINK_TEST_KLASS_ID, leakTid, epoch, epoch); } seededTestKlassTrend = true; } @@ -284,19 +414,21 @@ public void shouldReconstructReferrerChainToGcRoot() throws Exception { } if (match == null) { - // A quiet window with no dump() in flight: Profiler::dump() holds every _locks[] entry - // exclusively during _jfr.dump() (rotateDictsAndRun()'s lockAll()/unlockAll()), and - // writeReferenceChain() needs a slot to record anything, so back-to-back dumps leave - // pollWatchedTargets() no window. No gc() before this dump, so cleanup_table()'s - // epoch-advance stays a no-op and the trend is undisturbed. + // A quiet window with no dump() in flight: Profiler::dump() holds every entry of _locks[] + // exclusively for its own _jfr.dump() call - the same array writeReferenceChain() needs a + // slot from. Back-to-back rounds leave pollWatchedTargets() little to no window to win + // that race; sleeping briefly here, then dumping again with no intervening gc() (so + // cleanup_table()'s epoch-advance pass stays a no-op and the population trend is + // undisturbed), gives it one. Thread.sleep(300); dump(scratchDumpPath); match = ReferenceChainAssertions.findMatchForClass(verifyEvents(scratchDumpPath, "datadog.ReferenceChain", false), ChainLink.class); } } - // Grace period: the trend only became eligible partway through the loop, and the same lock - // contention can delay the write past the last dump(). + // Grace period: the trend only became eligible (ring_fill >= 10) partway through the loop + // above, and the same lock contention can still delay the write past this method's last + // dump() - retry a few more times before concluding the mechanism genuinely did not fire. for (int attempt = 0; match == null && attempt < 5; attempt++) { Thread.sleep(1000); dump(scratchDumpPath); @@ -307,60 +439,86 @@ public void shouldReconstructReferrerChainToGcRoot() throws Exception { "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"); - // chain[0] is the target's own class, not its holder's: reconstructChain() appends each - // entry's referrer_klass leaf-to-root, and referrer_klass is the discovered object's own - // class (see referenceChainJfrRoundtrip_ut.cpp). Hops above chain[0] are JDK collection - // internals this test does not control, so they are not asserted. + // 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 + // always some currently-live ChainLink instance - the only class the leak-candidate trend + // was built from - regardless of which instance the representative happens to be. Everything + // above chain[0] reflects real JDK-internal collection representation rather than anything + // this test controls, so it is deliberately not asserted beyond "at least one hop". assertEquals(ChainLink.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"); assertTrue(!gcRootHolder.isEmpty()); // keeps every allocated ChainLink reachable until here } finally { gcRootHolder = null; // shared-JVM hygiene - see the static holder's own comment - allocator.close(); + currentBatch = null; + // Shut the persistent allocator thread down before leaving: this is a + // shared, no-forkEvery test JVM, and a leftover allocation thread would + // keep allocating (and keep its tid trend alive) into every later test. + allocator.interrupt(); + allocator.join(1000); Files.deleteIfExists(scratchDumpPath); } } /** - * Same target-selection mechanism as {@link #shouldReconstructReferrerChainToGcRoot()}, against - * a common real-world leak shape: an ever-growing, never-evicted {@link java.util.HashMap} cache - * of {@link CachedPayload} values. Also asserts the chain threads through HashMap's internal - * storage, proving the walk handles real JDK collection internals. + * A second, independent end-to-end scenario for the same target-selection mechanism, using a + * shape much closer to a real-world leak than {@link ChainLink}'s hand-rolled list: an + * ever-growing, never-evicted {@link java.util.HashMap}-backed cache holding + * {@link CachedPayload} values. Also asserts the reconstructed chain actually threads back + * through {@code java.util.HashMap}'s own internal storage ({@code HashMap$Node}) - that the + * mechanism correctly walks a real JDK collection's internals, not only purpose-built fixtures. * *

    Why {@code cache} is a local variable, not a {@code static} field: - * {@code expandFrontier()} freezes a node's children when it is expanded and never re-examines - * it. A {@code static} field is a root from class-load time, so an early pass would almost - * certainly expand it while still empty and block discovery of everything added later. + * {@code expandFrontier()} freezes whatever children it observes for a node the moment that node + * is expanded - it never re-examines an already-{@code EXPANDED} entry for newly-added children. + * A {@code static} field becomes a root at class-load time, essentially guaranteeing some early + * pass catches it (and marks it {@code EXPANDED}) while still empty - permanently blocking + * discovery of anything added afterward. A local variable created fresh at the top of this + * method, immediately followed by round-1 allocation, gives root enumeration a real chance to + * catch it only after it already holds live data. */ @Test @Order(2) public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exception { - // Seed one entry before the resets, for the same empty-sentinel/frozen-children reason as - // gcRootHolder's seeding: HashMap's table is only allocated on the first put(). + // Seed cache with one live entry *before* the resets below: HashMap's backing table also + // starts as a shared, lazily-replaced empty sentinel (allocated on the first put()), so the + // same one-shot-BFS-freeze hazard documented in shouldReconstructReferrerChainToGcRoot() + // applies here too. Map cache = new HashMap<>(); cache.put("cache-leak-seed", new CachedPayload("cache-leak-seed")); if (ReferenceChainAssertions.isDebugInstrumentedBuild()) { - // The natural restart after the @Order(1) search ends is gated on canAffordNewSearch() and - // not guaranteed within this method's retry budget, so force a fresh search. The population - // reset is also needed: resetReferenceChainSearchForTest0() leaves LivenessTracker's rings - // alone, and ChainLink's leftover positive slope could outrank CachedPayload. - // Debug-only: these native seams do not exist in a release build. + // Force a genuine fresh search rather than depend on the natural restartSearch() cycle being + // gated on cadence/pacing budget (canAffordNewSearch()) firing again within this method's + // retry budget. + // + // resetKlassPopulationForTest0() too: without it, a klass ChainLink already accumulated a + // positive slope for during the @Order(1) test can still outrank CachedPayload as the leak + // candidate here, and the fresh search ends up reconstructing ChainLink's chain again. ReferenceChainTestHooks.resetKlassPopulationForTest0(); ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); } - // Every CachedPayload is allocated by one persistent thread - see PersistentAllocator. - PersistentAllocator allocator = new PersistentAllocator("cache-leak-allocator"); - Path scratchDumpPath = Paths.get("referencechains-cache-leak-scratch.jfr"); + Path scratchDumpPath = Files.createTempFile("referencechains-cache-leak-scratch-", ".jfr"); + Files.deleteIfExists(scratchDumpPath); + Thread allocator = null; try { - // Same round-growth/retry shape (and caps) as shouldReconstructReferrerChainToGcRoot(). + // 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). ReferenceChainAssertions.JfrChainMatch match = null; boolean seededTestKlassTrend = false; int totalRounds = 16; - // Keys are pre-generated: a String key (and its byte[]) allocated alongside each - // CachedPayload would compete for the size-weighted sampler. Allocating them all up front - // lets their own population trend flatten before CachedPayload's starts growing. + // 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, + // size-weighted-sampler competitor as the byte[] field CachedPayload's comment mentions - + // just a separate object. Generating all keys before the round loop lets their own + // klass_population trend flatten out well before CachedPayload's ring starts growing, so + // 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; @@ -370,36 +528,72 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc keys[i] = "leak-" + i; } - int nextKey = 0; + // One persistent allocator thread for every round - same shape and rationale as the gc-root + // test's allocator. Decisively, every CachedPayload (including the keys[0] representative + // set below) carries THIS thread's profiler tid, which is the tid the per-(klass, tid) + // seeds must name: tagLeakInstances() only tags instances allocated by a qualifying tid. + // Only this thread mutates cache, and only between a request's take() and its + // roundDone.release(); the main thread reads cache only after awaitAllocatorRound() returns. + LinkedBlockingQueue allocRequests = new LinkedBlockingQueue<>(); + Semaphore roundDone = new Semaphore(0); + AtomicLong allocatorTid = new AtomicLong(-1); + AtomicReference allocatorFailure = new AtomicReference<>(); + allocator = new Thread(() -> { + try { + allocatorTid.set(ReferenceChainTestHooks.getTid()); + int next = 0; // allocator-owned key cursor; keys[] was sized for every round up front + Integer requested; + while ((requested = allocRequests.take()) != null) { + for (int i = 0; i < requested; i++) { + String key = keys[next++]; + cache.put(key, new CachedPayload(key)); + } + roundDone.release(); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); // test shutdown - see finally below + } catch (Throwable t) { + allocatorFailure.set(t); // surfaced by awaitAllocatorTid()/awaitAllocatorRound() + } + }, "cacheleak-allocator"); + allocator.start(); + awaitAllocatorTid(allocator, allocatorTid, allocatorFailure); + for (int round = 1; round <= totalRounds && match == null; round++) { - // Same *600 rate as the ChainLink test: the two classes are near-identical in size, and a - // lower rate let CachedPayload's ring stall short of KLASS_POPULATION_MIN_FILL_FOR_TREND. + // 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; - int keyOffset = nextKey; - allocator.run(() -> { - for (int i = 0; i < newEntries; i++) { - String key = keys[keyOffset + i]; - cache.put(key, new CachedPayload(key)); - } - }); - nextKey += newEntries; + if (!allocRequests.offer(newEntries)) { + throw new IllegalStateException("allocRequests queue rejected entry count " + newEntries); + } + awaitAllocatorRound(roundDone, allocator, allocatorFailure); System.gc(); dump(scratchDumpPath); JfrEvents events1 = verifyEvents(scratchDumpPath, "datadog.ReferenceChain", false); match = ReferenceChainAssertions.findMatchForClass(events1, CachedPayload.class); if (match == null && ReferenceChainAssertions.isDebugInstrumentedBuild()) { - // Same seed-once, representative-first, best-effort-poll rationale as - // shouldReconstructReferrerChainToGcRoot(). - ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0(CACHED_PAYLOAD_TEST_KLASS_ID, cache.get(keys[0])); + // Same deterministic short-circuit as the gc-root test: the seed is what authorizes the + // background BFS thread's next real pass, so the explicit poll below is best-effort; + // and seed exactly once, since reseeding would corrupt the ring into a non-monotonic + // sawtooth. Representative BEFORE seeding - same real-id aliasing rationale. + ReferenceChainTestHooks.setKlassPopulationRepresentativeForTest0( + CACHED_PAYLOAD_TEST_KLASS_ID, cache.get(keys[0])); if (!seededTestKlassTrend) { - // The allocator thread owns every CachedPayload allocation, and the seeds use the - // organic ramp's scale - both for the reasons shouldReconstructReferrerChainToGcRoot() - // gives. - int leakTid = allocator.tid(); + // The seeds name the allocator thread's tid: it allocated every CachedPayload, + // including the keys[0] representative - NOT this JUnit worker's tid, which allocated + // none of them. Seeds match the organic ramp (epoch, not epoch*10): inflated seeds + // made the slope go negative (-20..-40 observed) once real samples (about 1..15 per + // epoch) pushed the seeds out of the trend window - see the gc-root test's identical + // seeding for the derivation. + int leakTid = (int) allocatorTid.get(); for (int epoch = 1; epoch <= SEED_EPOCHS_FOR_HYSTERESIS; epoch++) { - ReferenceChainTestHooks.seedKlassPopulationSample0(CACHED_PAYLOAD_TEST_KLASS_ID, epoch, epoch); - ReferenceChainTestHooks.seedTidTrendSample0(CACHED_PAYLOAD_TEST_KLASS_ID, leakTid, epoch, epoch); + ReferenceChainTestHooks.seedKlassPopulationSample0( + CACHED_PAYLOAD_TEST_KLASS_ID, epoch, epoch); + ReferenceChainTestHooks.seedTidTrendSample0( + CACHED_PAYLOAD_TEST_KLASS_ID, leakTid, epoch, epoch); } seededTestKlassTrend = true; } @@ -435,9 +629,9 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc 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"); - // The point of this test over shouldReconstructReferrerChainToGcRoot(): confirm the walk - // actually passed through the cache's own internal storage, not some other, coincidental - // retainer - cache is the only thing keeping any CachedPayload instance reachable. + // The point of this test over the gc-root test: confirm the walk actually passed through + // the cache's own internal storage, not some other, coincidental retainer - cache is the + // only thing keeping any CachedPayload instance reachable. boolean sawHashMapInternals = false; for (String type : match.chain) { if (type.startsWith("java.util.HashMap")) { @@ -450,79 +644,109 @@ public void shouldReconstructReferrerChainThroughUnboundedCacheLeak() throws Exc + "storage (cache is a HashMap) - chain was: " + match.chain); assertTrue(!cache.isEmpty()); // keeps every cached CachedPayload reachable until here } finally { - allocator.close(); + if (allocator != null) { + allocator.interrupt(); + allocator.join(1000); + } Files.deleteIfExists(scratchDumpPath); } } /** - * Drives the shared search to {@code SearchState::ABANDONED} - via {@code framecap=1} when this - * method sizes the {@code FrontierTable}, otherwise via the {@code ttl=100} fallback (see - * {@code getProfilerCommand()}) - and asserts a {@code datadog.ReferenceChainAbandoned} event is - * written on {@code dump()}, without asserting which reason produced it. + * Abandonment-path scenario: an artificially tiny frontier cap ({@code framecap=1} for this + * test method) makes the very first BFS pass hit the frontier cap once a second root-referenced + * object is discovered, so {@code runPass()} abandons the search rather than silently + * truncating it - or, if this method lost the race to size the shared {@code FrontierTable}, + * its {@code ttl=100} fallback abandons the search instead (by the time this method runs, that + * clock always has). Either path exercises the same {@code buildAbandonedEvent()}/ + * {@code Profiler::writeReferenceChainAbandoned()}/{@code recordReferenceChainAbandoned()} + * write triggered by {@code Profiler::dump()} - this method only asserts that a + * {@code datadog.ReferenceChainAbandoned} event exists, not which reason produced it. * - *

    {@code @Order(3)}: abandoning the singleton search would break the two success-path tests. - * The debug-build reset makes the fresh-search precondition explicit: if an earlier test's - * trend never fired, the search may still be {@code RUNNING} with a large frontier. + *

    {@code @Order(3)}: this permanently exhausts the singleton's one-and-only search, so it + * must run after both success-path tests. It also calls {@code resetReferenceChainSearchForTest0()} + * itself: if either earlier test's organic GC/sampling trend never fired in time (real + * flakiness, observed in practice), the shared search can still be {@code RUNNING} with a large, + * non-fresh frontier when this method starts - the fresh-search precondition is something this + * method establishes, not something it presumes. Debug-only: the seam does not exist in a + * release build. * - *

    Why the population seeding: {@code resetReferenceChainSearchForTest0()} alone was - * observed never running a first pass. With this command ({@code memory=64:l:1.0}, no - * {@code generations=true}) {@code LivenessTracker::initialize()} clears generations, so - * {@code hasLeakSignal()} returns true unconditionally; the seeding is defensive in case the - * command re-enables generations, in which case {@code shouldRunPass()} falls through to - * {@code selectLeakCandidates()} with nothing left from the cache test's population reset. No - * representative is set: a qualifying entry counts toward {@code hasLeakSignal()} regardless, - * and {@code resolveCandidateRepresentative()} is null-safe. + *

    Why this method also seeds a klass population trend, despite requesting no + * {@code generations=true} of its own: {@code LivenessTracker::_gc_generations} is only ever + * updated when a {@code start()} command actually requests liveness tracking - a bare + * {@code referencechains=true:...} command never calls {@code LivenessTracker::start()}, so + * {@code gcGenerationsEnabled()} stays stuck at whatever the two {@code generations=true} tests + * above last left it. {@code hasLeakSignal()}, which {@code shouldRunPass()} consults for even + * the search's first pass, therefore falls through to {@code selectLeakCandidates()} exactly as + * the tests above do - and with {@code _klass_population} freshly reset, nothing satisfies it. + * Seeding {@code SEED_EPOCHS_FOR_HYSTERESIS} epochs for a dedicated klass id clears that gate; + * this method never sets a representative or polls for it, since it has no reconstructed chain + * of its own to prove - a qualifying entry counts toward {@code hasLeakSignal()}'s candidate + * count regardless, and {@code resolveCandidateRepresentative()} is null-safe against one that + * was never set. * - *

    Why a {@code System.gc()} + throwaway {@code dump()} precede the seeding: - * {@code cleanup_table()} wipes {@code _klass_population} once when the class map's - * generation changes (which this method's fresh {@code start()} causes), but only after the - * {@code if (!is_epoch_owner && !forced) return;} guard, which an unforced dump-triggered call - * passes only when a new GC epoch is pending. Without a GC first, the wipe would be deferred to - * the next forced cleanup (on the BFS thread's wake after the GCs below) and discard the seed. - * The success-path tests get this for free from their round-1 GC + dump. + *

    Why the seeding is preceded by a {@code System.gc()} and a throwaway {@code dump()}: + * {@code cleanup_table()} compares {@code Profiler::classMap()->generation()} against its own + * cached copy and, on a mismatch - which this method's fresh {@code start()} causes, by clearing + * that map - wipes {@code _klass_population} back to empty, exactly once per mismatch; but that + * comparison sits *after* an early-return guard a {@code dump()}-triggered call + * ({@code forced=false}) only clears when a genuinely new, not-yet-processed GC epoch is + * pending - a dump() with no GC since the last cleanup pass never reaches the mismatch check at + * all. The two tests above get this wipe-and-resync for free from their round-1 + * {@code System.gc()}+{@code dump()} pair; seeding before any GC/dump here would leave the seed + * to be discarded by the next call that does clear the guard - this class's own + * {@code maybeForceCleanup()} tick (once per {@code threadLoop()} wake, {@code forced=true} + * unconditionally clears the guard), which fires on this method's own 3 {@code System.gc()} + * calls below. The explicit pair forces the resync to happen now, before seeding. */ @Test @Order(3) public void shouldReportAbandonedSearchOnTinyFrontierCap() throws Exception { if (ReferenceChainAssertions.isDebugInstrumentedBuild()) { - // GC + throwaway dump before seeding - see javadoc. + // See the javadoc above for why the seeding is needed despite no generations=true of this + // method's own, and why both a gc() and a throwaway dump() must precede it. System.gc(); - Path warmupDumpPath = Paths.get("referencechains-abandoned-test-warmup.jfr"); + Path warmupDumpPath = Files.createTempFile("referencechains-abandoned-test-warmup-", ".jfr"); + Files.deleteIfExists(warmupDumpPath); try { dump(warmupDumpPath); } finally { Files.deleteIfExists(warmupDumpPath); } ReferenceChainTestHooks.resetKlassPopulationForTest0(); - // Gate-only seeding: no tagged instance needs to match, so any tid would do. + // Gate-only usage: this test drives the search to ABANDONED and asserts on the abandon + // marker, no tagged instance needs to match - so the tid is this thread's real one but any + // distinct tid would do. int abandonTid = ReferenceChainTestHooks.getTid(); for (int epoch = 1; epoch <= SEED_EPOCHS_FOR_HYSTERESIS; epoch++) { - ReferenceChainTestHooks.seedKlassPopulationSample0(ABANDON_TEST_KLASS_ID, epoch * 10, epoch); - ReferenceChainTestHooks.seedTidTrendSample0(ABANDON_TEST_KLASS_ID, abandonTid, epoch * 3, epoch); + ReferenceChainTestHooks.seedKlassPopulationSample0( + ABANDON_TEST_KLASS_ID, epoch * 10, epoch); + ReferenceChainTestHooks.seedTidTrendSample0( + ABANDON_TEST_KLASS_ID, abandonTid, epoch * 3, epoch); } ReferenceChainTestHooks.resetReferenceChainSearchForTest0(); } List gcRootHolder = new ArrayList<>(); gcRootHolder.add(new ChainLink("middle", new ChainLink("leaf"))); - // GarbageCollectionFinish (onGCFinish(), referenceChains.cpp) wakes the BFS thread - // early, but that wakeup can race the thread's own startup (VM::attachThread() - // completing before its first OS::sleep() call) and be missed. Don't rely on the - // signal alone: sleep comfortably past ReferenceChainTracker::PASS_CADENCE_NS (1s, - // referenceChains.h) too, so the thread's fixed-cadence fallback trigger - // (shouldRunPass()) guarantees at least one pass runs regardless of that race. + // GarbageCollectionFinish wakes the BFS thread early, but that wakeup can race the thread's + // own startup (VM::attachThread() completing before its first sleep) and be missed. Sleep + // comfortably past PASS_CADENCE_NS (1s) too, so the fixed-cadence fallback guarantees at + // least one pass runs regardless of that race. for (int i = 0; i < 3; i++) { System.gc(); Thread.sleep(100); } Thread.sleep(1500); - Path dumpPath = Paths.get("referencechains-abandoned-test.jfr"); + Path dumpPath = Files.createTempFile("referencechains-abandoned-test-", ".jfr"); + Files.deleteIfExists(dumpPath); try { - // The pass still has to be woken and reach the frontier-cap/ttl cutoff within whatever time - // the shared JVM grants it; a single fixed sleep-then-assert failed when it landed a beat - // early. Retry spaced far enough apart to tolerate a slow wake or a larger inherited heap. + // Even with the seeding above clearing the gate, the first pass still has to actually reach + // the frontier cap/ttl cutoff inside whatever wall-clock time this shared JVM's scheduling + // grants it - a single fixed-length sleep-then-dump-then-assert previously failed whenever + // that one attempt landed a beat early. Retry across a few more dumps before concluding the + // search genuinely never abandoned. JfrEvents abandoned = null; for (int attempt = 0; attempt < 8; attempt++) { dump(dumpPath); @@ -542,9 +766,58 @@ public void shouldReportAbandonedSearchOnTinyFrontierCap() throws Exception { } /** - * Referrer-type fixture for the success-path and abandonment-path tests. The 32 {@code long} - * fields cut the instance count needed per sampled megabyte. Plain fields, not an array: an - * array would be a separate allocation competing for the size-weighted sampler. + * Bounded wait for a persistent allocator thread to publish its profiler tid: fails fast with + * the allocator's own failure cause, or if it exited/stalled without publishing, instead of + * hanging the shared test fork. + */ + private static void awaitAllocatorTid(Thread allocator, AtomicLong allocatorTid, + AtomicReference allocatorFailure) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(ALLOCATOR_HANDOFF_TIMEOUT_MS); + while (allocatorTid.get() == -1) { + Throwable failure = allocatorFailure.get(); + if (failure != null) { + throw new AssertionError( + allocator.getName() + " failed before publishing its tid", failure); + } + if (!allocator.isAlive() && allocatorTid.get() == -1) { + throw new AssertionError(allocator.getName() + " exited without publishing its tid"); + } + if (System.nanoTime() - deadline > 0) { + throw new AssertionError(allocator.getName() + " did not publish its tid within " + + ALLOCATOR_HANDOFF_TIMEOUT_MS + "ms"); + } + Thread.sleep(1); + } + } + + /** Bounded wait for one allocator round; same fail-fast policy as awaitAllocatorTid(). */ + private static void awaitAllocatorRound(Semaphore roundDone, Thread allocator, + AtomicReference allocatorFailure) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(ALLOCATOR_HANDOFF_TIMEOUT_MS); + while (!roundDone.tryAcquire(10, TimeUnit.MILLISECONDS)) { + Throwable failure = allocatorFailure.get(); + if (failure != null) { + throw new AssertionError(allocator.getName() + " failed mid-round", failure); + } + if (!allocator.isAlive()) { + if (roundDone.tryAcquire()) { + return; // released just before exiting + } + throw new AssertionError(allocator.getName() + " exited mid-round"); + } + if (System.nanoTime() - deadline > 0) { + throw new AssertionError(allocator.getName() + " did not finish its round within " + + ALLOCATOR_HANDOFF_TIMEOUT_MS + "ms"); + } + } + } + + /** + * Referrer-type fixture shared by both success-path and abandonment-path tests. The 32 + * {@code long} fields exist purely so a given number of megabytes of ChainLink needs far fewer + * instances - deliberately plain fields, not a nested array: an array field would be a second, + * separate heap allocation, and the size-weighted allocation sampler would take sampling + * attention away from ChainLink itself rather than adding to it. */ private static final class ChainLink { final String name; @@ -563,9 +836,11 @@ private static final class ChainLink { } /** - * Cache-value fixture for {@link #shouldReconstructReferrerChainThroughUnboundedCacheLeak()}. - * Padded with plain {@code long} fields for the same reason as {@link ChainLink}; a - * {@code byte[]} field was observed taking every sample away from {@code CachedPayload}. + * Realistic-leak fixture for the cache test: an unbounded, never-evicted cache - the single + * most common real-world leak shape this mechanism is meant to catch. The 32 {@code long} + * fields are there for the same reason {@link ChainLink}'s are (an earlier version used a + * {@code byte[]} field and never observed a single CachedPayload sample - the sampler was + * catching the array instead). */ private static final class CachedPayload { final String key; diff --git a/utils/compare-refchains-repro.sh b/utils/compare-refchains-repro.sh new file mode 100755 index 0000000000..627ac9ec1a --- /dev/null +++ b/utils/compare-refchains-repro.sh @@ -0,0 +1,205 @@ +#!/usr/bin/env bash +# Copyright 2026, Datadog, Inc +# +# Runs the reference-chains repro app (run-refchains-repro.sh) twice, back to +# back, over an identical bounded window - once with referencechains=true and +# once with referencechains=false - and diffs the two runs' throughput, +# safepoint/GC pause times, and heap growth, to answer: +# - is it slower overall (throughput/round latency)? +# - is it imposing lengthy STW safepoints (safepoint/GC log)? +# - is it increasing memory usage significantly (heap growth, peak RSS)? +# +# Prints a markdown comparison table plus explicit answers to those three +# questions, and saves the same content as report.md in the run's temp +# working directory alongside the raw stdout/safepoint logs. +# +# If either variant exits non-zero or prints no "[metrics]" line, no +# comparison is made: report.md is written as an INVALID report naming the +# failed variant(s) and the script exits 1. +# +# Usage: compare-refchains-repro.sh [duration-seconds] +# +# Env vars: same REFCHAINS_SO/REFCHAINS_JAR/REFCHAINS_ARGS/REFCHAINS_JAVA_HOME/ +# REFCHAINS_GC as run-refchains-repro.sh - passed through unchanged to both +# variants so the two runs stay comparable (REFCHAINS_ENABLED is set by this +# script itself, per run - don't set it yourself). + +set -euo pipefail + +HERE="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" + +DURATION_SECONDS="${1:-120}" +WORKDIR="$(mktemp -d /tmp/refchains_compare.XXXXXX)" + +JDK_DESC="${REFCHAINS_JAVA_HOME:-java on PATH}" +GC_DESC="${REFCHAINS_GC:-JVM ergonomic default}" + +echo "Comparing referencechains=true vs. false over ${DURATION_SECONDS}s each" +echo "JDK: ${JDK_DESC}" +echo "GC: ${GC_DESC}" +echo "Working directory: ${WORKDIR}" + +# Space-prefixed list of "