Skip to content

Commit fede524

Browse files
jbachorikclaude
andcommitted
Decouple LeakingCacheScenario from real allocation-sampling timing
Same debug-only seeded-representative short-circuit already used by ReferenceChainTrackingTest, ported to the external-process scenario: propagates ddprof_test.config to the child JVM so it can bypass LivenessTracker's probabilistic slope detection instead of depending on it to notice CachedPayload within the fixed round/timeout budget. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent accd257 commit fede524

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/ExternalProcessReferenceChainTest.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.nio.file.Files;
1313
import java.nio.file.Path;
1414
import java.util.Collections;
15+
import java.util.List;
1516
import java.util.concurrent.atomic.AtomicReference;
1617

1718
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -83,8 +84,16 @@ void shouldReconstructReferrerChainInSeparateProcess() throws Exception {
8384
// commands) contract with a 3rd argument every other mode would have to ignore).
8485
String packedCommand = startCommand + "|||" + scratchDumpPath.toAbsolutePath();
8586

87+
// Propagates this JVM's own ddprof_test.config (set by ProfilerTestPlugin.kt on the
88+
// ddprof-test Test task itself, not inherited by a ProcessBuilder-launched child on its
89+
// own) so LeakingCacheScenario can use the same debug-only seeded-representative fallback
90+
// ReferenceChainTrackingTest already relies on instead of depending purely on real
91+
// allocation-sampling timing.
92+
List<String> jvmArgs = Collections.singletonList(
93+
"-Dddprof_test.config=" + System.getProperty("ddprof_test.config"));
94+
8695
AtomicReference<String> resultLine = new AtomicReference<>();
87-
LaunchResult result = launch("leak-cache", Collections.emptyList(), packedCommand,
96+
LaunchResult result = launch("leak-cache", jvmArgs, packedCommand,
8897
Collections.emptyMap(),
8998
// Generous: a whole separate JVM's startup/classloading cost, on top of the same
9099
// up-to-25-round population-growth loop plus grace period that takes ~20s in-process

ddprof-test/src/test/java/com/datadoghq/profiler/referencechains/LeakingCacheScenario.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,13 @@
5252
public final class LeakingCacheScenario {
5353
private LeakingCacheScenario() {}
5454

55+
// Arbitrary, scenario-chosen klass id - this process's own LivenessTracker population table
56+
// treats it as an opaque key (see KlassPopulationEntry's own comment) and this process never
57+
// runs any other reference-chains scenario, so no collision risk with e.g.
58+
// ReferenceChainTrackingTest's/ReferenceChainTestSeamsTest's own klass ids in their own,
59+
// separate JVMs.
60+
private static final int CACHED_PAYLOAD_TEST_KLASS_ID = 987301;
61+
5562
/** Printed to stdout, followed by the matched leaf class's name, on success. */
5663
public static final String FOUND_MARKER = "[chain-found] ";
5764

@@ -89,6 +96,8 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD
8996
}
9097
System.out.println("[debug] profiler.execute() returned");
9198

99+
boolean debugBuild = "debug".equals(System.getProperty("ddprof_test.config"));
100+
boolean seededTestKlassTrend = false;
92101
ReferenceChainAssertions.ChainMatch match = null;
93102
int totalRounds = 25;
94103
for (int round = 1; round <= totalRounds && match == null; round++) {
@@ -106,6 +115,26 @@ public static void run(JavaProfiler profiler, String startCommand, Path scratchD
106115
profiler.dump(scratchDumpPath);
107116
match = findMatch(scratchDumpPath);
108117

118+
if (match == null && debugBuild) {
119+
// Same debug-only, seeded-representative short-circuit as ReferenceChainTrackingTest's
120+
// own shouldReconstructReferrerChainThroughUnboundedCacheLeak() (see that method's own
121+
// comment): decouples this scenario's assertion from whether the real, probabilistic
122+
// allocation-sampling-driven slope detection happens to notice CachedPayload's own
123+
// population trend within this scenario's fixed round/timeout budget. "seed-0" (from
124+
// seedInitialBatch()) is reachable via cache for the scenario's entire lifetime, so it
125+
// is a safe, always-valid representative regardless of which round this fires on.
126+
if (!seededTestKlassTrend) {
127+
for (int epoch = 1; epoch <= 10; epoch++) {
128+
JavaProfiler.seedKlassPopulationSample0(CACHED_PAYLOAD_TEST_KLASS_ID, epoch * 10, epoch);
129+
}
130+
seededTestKlassTrend = true;
131+
}
132+
JavaProfiler.setKlassPopulationRepresentativeForTest0(CACHED_PAYLOAD_TEST_KLASS_ID, cache.get("seed-0"));
133+
JavaProfiler.pollReferenceChainTargets0();
134+
profiler.dump(scratchDumpPath);
135+
match = findMatch(scratchDumpPath);
136+
}
137+
109138
if (match == null) {
110139
Thread.sleep(300);
111140
profiler.dump(scratchDumpPath);

0 commit comments

Comments
 (0)