Skip to content

Commit ebf7f17

Browse files
jbachorikclaude
andcommitted
Remove stop/restart test, enable NMT by default
- Removed testGetLineNumberTableNoLeak (stop/restart not production usage) - Updated test docs to focus on continuous profiling scenario - Enabled NMT automatically with IgnoreUnrecognizedVMOptions fallback - No longer requires -PenableNMT flag 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 5b881a7 commit ebf7f17

2 files changed

Lines changed: 21 additions & 213 deletions

File tree

ddprof-test/build.gradle

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -293,10 +293,10 @@ tasks.withType(Test).configureEach {
293293
"-Djava.library.path=${outputLibDir.absolutePath}"
294294
]
295295

296-
// Enable Native Memory Tracking for leak detection tests if requested
297-
if (project.hasProperty('enableNMT') || Boolean.parseBoolean(System.getenv("ENABLE_NMT"))) {
298-
jvmArgsList.add('-XX:NativeMemoryTracking=detail')
299-
}
296+
// Enable Native Memory Tracking for leak detection tests
297+
// Use IgnoreUnrecognizedVMOptions to silently ignore on JVMs that don't support NMT
298+
jvmArgsList.add('-XX:+IgnoreUnrecognizedVMOptions')
299+
jvmArgsList.add('-XX:NativeMemoryTracking=detail')
300300

301301
jvmArgs jvmArgsList
302302

ddprof-test/src/test/java/com/datadoghq/profiler/memleak/GetLineNumberTableLeakTest.java

Lines changed: 17 additions & 209 deletions
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,28 @@
3535
import java.util.List;
3636

3737
/**
38-
* Test to detect native memory leaks in GetLineNumberTable JVMTI calls.
38+
* Test to validate that method_map cleanup prevents unbounded memory growth in continuous profiling.
3939
*
40-
* <p>This test verifies that repeated flush() operations during profiling sessions
41-
* do not cause memory leaks from line number table allocations.
40+
* <p>This test focuses on the production scenario where the profiler runs continuously for
41+
* extended periods without stop/restart cycles. In production, Recording objects live for the
42+
* entire application lifetime (days/weeks), and without cleanup, _method_map would accumulate
43+
* ALL methods encountered, causing unbounded growth (observed: 3.8M methods × ~300 bytes = 1.2 GB).
4244
*
43-
* <p><b>Test Limitations:</b>
45+
* <p><b>What This Test Validates:</b>
4446
* <ul>
45-
* <li>NMT (Native Memory Tracking) does not create a JVMTI category until allocations are
46-
* significant (typically 1KB+)</li>
47-
* <li>GetLineNumberTable allocations are small (~10-100 bytes per method), so short-running
48-
* tests show 0 KB even with leaks present</li>
49-
* <li>This test serves as <b>regression protection</b> rather than numerical leak validation</li>
50-
* <li>It ensures the code path works without crashes and follows JVMTI spec</li>
51-
* <li>Production leaks would manifest over days/weeks with millions of method encounters</li>
47+
* <li>Age-based cleanup removes methods unused for 3+ consecutive chunks</li>
48+
* <li>method_map size stays bounded (500-1000 methods vs 3000 without cleanup)</li>
49+
* <li>Cleanup runs during switchChunk() triggered by dump() operations</li>
50+
* <li>Memory growth plateaus after initial warmup (not linear growth)</li>
5251
* </ul>
5352
*
54-
* <p>Requires NMT: Run with -XX:NativeMemoryTracking=detail or -PenableNMT
53+
* <p><b>Test Strategy:</b>
54+
* <ul>
55+
* <li>Continuous profiling (NO stop/restart cycles)</li>
56+
* <li>Generate transient methods across multiple chunk boundaries</li>
57+
* <li>Hold strong references to prevent GC (isolate cleanup behavior)</li>
58+
* <li>Verify bounded growth via TEST_LOG output showing method_map size</li>
59+
* </ul>
5560
*/
5661
public class GetLineNumberTableLeakTest extends AbstractProfilerTest {
5762

@@ -61,185 +66,6 @@ protected String getProfilerCommand() {
6166
return "cpu=1ms,alloc=512k";
6267
}
6368

64-
@Test
65-
public void testGetLineNumberTableNoLeak() throws Exception {
66-
// Verify NMT is enabled (test skipped if not available)
67-
Assumptions.assumeTrue(
68-
NativeMemoryTracking.isEnabled(), "Test requires -XX:NativeMemoryTracking=detail");
69-
70-
// Take NMT baseline
71-
NativeMemoryTracking.NMTSnapshot baseline = NativeMemoryTracking.takeSnapshot();
72-
System.out.println(
73-
String.format(
74-
"Baseline NMT: malloc=%d KB, Internal=%d KB",
75-
baseline.mallocKB, baseline.internalReservedKB));
76-
77-
// Phase 1: Warmup - generate many unique classes with ASM to populate _method_map
78-
System.out.println("Phase 1: Warmup - generating many unique classes via ASM...");
79-
final int warmupClassGenerations = 20; // Generate classes in batches
80-
final int totalRestarts = 25; // Need 25 restarts for 5 checkpoint intervals (5 restarts each)
81-
final int checkpointInterval = 5; // Check every 5 restarts
82-
83-
// Track snapshots: baseline + afterWarmup + 5 checkpoint intervals = 7 total
84-
NativeMemoryTracking.NMTSnapshot[] checkpoints = new NativeMemoryTracking.NMTSnapshot[7];
85-
checkpoints[0] = baseline;
86-
int checkpointIndex = 1;
87-
88-
// Cache many generated classes for reuse in steady state
89-
// generateUniqueMethodCalls() returns 5 classes, each with 20 methods
90-
// Total: 20 batches × 5 classes/batch = 100 classes with ~2,000 methods
91-
Class<?>[] cachedClasses = new Class<?>[warmupClassGenerations * 5];
92-
int cachedClassIndex = 0;
93-
94-
for (int i = 0; i < warmupClassGenerations; i++) {
95-
// Generate 5 unique classes per batch via ASM (each with 20 methods with line tables)
96-
Class<?>[] newClasses = generateUniqueMethodCalls();
97-
System.arraycopy(newClasses, 0, cachedClasses, cachedClassIndex, newClasses.length);
98-
cachedClassIndex += newClasses.length;
99-
100-
// Flush profiler to trigger method resolution and GetLineNumberTable calls
101-
dump(tempFile("warmup-" + i));
102-
103-
if ((i + 1) % 20 == 0) {
104-
System.out.println(
105-
String.format("Generated %d classes so far (%d batches)...", cachedClassIndex, i + 1));
106-
}
107-
}
108-
109-
// Take snapshot after warmup
110-
NativeMemoryTracking.NMTSnapshot afterWarmup = NativeMemoryTracking.takeSnapshot();
111-
checkpoints[checkpointIndex++] = afterWarmup;
112-
long warmupInternalGrowthKB = afterWarmup.internalReservedKB - baseline.internalReservedKB;
113-
System.out.println(
114-
String.format(
115-
"After warmup: %d classes generated, Internal=%d KB (+%d KB)",
116-
cachedClassIndex,
117-
afterWarmup.internalReservedKB,
118-
warmupInternalGrowthKB));
119-
120-
// Phase 2: Steady state - repeated restarts to accumulate leak if present
121-
// Stop/start cycles trigger SharedLineNumberTable destructors
122-
// With bug: each restart leaks ~16 KB; across many restarts this should accumulate into a detectable leak
123-
System.out.println(
124-
String.format(
125-
"Phase 2: Performing %d profiler restarts to test for accumulated leaks...",
126-
totalRestarts));
127-
128-
for (int restart = 0; restart < totalRestarts; restart++) {
129-
// Stop profiler to destroy Recording and trigger all SharedLineNumberTable destructors
130-
profiler.stop();
131-
Thread.sleep(10); // Allow cleanup to complete
132-
133-
// Restart profiler (creates new Recording, repopulates _method_map from same classes)
134-
profiler.execute(
135-
"start," + getProfilerCommand() + ",jfr,file=" + tempFile("restart-" + restart));
136-
137-
// Reuse cached classes to trigger GetLineNumberTable again
138-
invokeCachedClasses(cachedClasses, restart);
139-
140-
// Single flush per restart
141-
dump(tempFile("steady-" + restart));
142-
143-
// Take checkpoint snapshots every checkpointInterval restarts
144-
if ((restart + 1) % checkpointInterval == 0) {
145-
NativeMemoryTracking.NMTSnapshot progress = NativeMemoryTracking.takeSnapshot();
146-
checkpoints[checkpointIndex++] = progress;
147-
148-
long internalGrowthFromWarmupKB = progress.internalReservedKB - afterWarmup.internalReservedKB;
149-
long internalGrowthKB = progress.internalReservedKB - baseline.internalReservedKB;
150-
System.out.println(
151-
String.format(
152-
"After %d restarts: Internal=%d KB (+%d KB from warmup, +%d KB total)",
153-
restart + 1,
154-
progress.internalReservedKB,
155-
internalGrowthFromWarmupKB,
156-
internalGrowthKB));
157-
}
158-
}
159-
160-
// Take final snapshot
161-
NativeMemoryTracking.NMTSnapshot afterRestarts = checkpoints[6];
162-
long steadyStateInternalGrowthKB =
163-
afterRestarts.internalReservedKB - afterWarmup.internalReservedKB;
164-
long totalInternalGrowthKB = afterRestarts.internalReservedKB - baseline.internalReservedKB;
165-
166-
// Calculate Internal category growth rates for steady state intervals
167-
// checkpoints[1] = after warmup
168-
// checkpoints[2-6] = after 5, 10, 15, 20, 25 restarts (with checkpointInterval = 5)
169-
long[] steadyStateInternalGrowths = new long[5];
170-
for (int i = 0; i < 5; i++) {
171-
steadyStateInternalGrowths[i] =
172-
checkpoints[i + 2].internalReservedKB - checkpoints[i + 1].internalReservedKB;
173-
}
174-
175-
// Assert that Internal category doesn't show super-linear growth
176-
// With fix: Internal should plateau after warmup (< 10 KB per 5 restarts from minor JVM allocations)
177-
// Without fix: each restart leaks ~1.6 KB → 5 restarts = ~8 KB per interval
178-
long maxIntervalGrowth = 0;
179-
for (int i = 0; i < steadyStateInternalGrowths.length; i++) {
180-
maxIntervalGrowth = Math.max(maxIntervalGrowth, steadyStateInternalGrowths[i]);
181-
182-
System.out.println(
183-
String.format(
184-
"Interval %d (restarts %d-%d): Internal +%d KB",
185-
i + 1,
186-
i * checkpointInterval,
187-
(i + 1) * checkpointInterval,
188-
steadyStateInternalGrowths[i]));
189-
190-
// Assert individual intervals don't show excessive JVMTI leak growth
191-
// Threshold: 10 KB per interval
192-
// With fix: typically < 5 KB (minor allocations)
193-
// Without fix: ~10-20 KB per interval (line table leaks accumulate)
194-
if (steadyStateInternalGrowths[i] > 10) { // 10 KB per interval
195-
fail(
196-
String.format(
197-
"Internal category growth indicates JVMTI leak!\n"
198-
+ "Interval %d (restarts %d-%d): +%d KB\n"
199-
+ "Expected: Internal should plateau; no significant growth across intervals\n"
200-
+ "Actual: continued growth indicates leaked GetLineNumberTable allocations",
201-
i + 1,
202-
i * checkpointInterval,
203-
(i + 1) * checkpointInterval,
204-
steadyStateInternalGrowths[i]));
205-
}
206-
}
207-
208-
// Verify total steady state Internal growth is bounded
209-
// With fix: should be < 20 KB total (minor JVM allocations over 25 restarts)
210-
// Without fix: 25 restarts × ~1.6 KB/restart = ~40 KB JVMTI leak (based on observed leak rate)
211-
NativeMemoryTracking.assertInternalMemoryBounded(
212-
afterWarmup,
213-
afterRestarts,
214-
20 * 1024, // 20 KB - tight enough to catch 40 KB leak
215-
"Internal category grew excessively - JVMTI leak detected!");
216-
217-
// Print summary
218-
System.out.println(
219-
String.format(
220-
"\n=== Test Completed Successfully ===\n"
221-
+ "Phase 1 (Warmup):\n"
222-
+ " Classes generated: %d (via ASM)\n"
223-
+ " Internal growth: +%d KB\n"
224-
+ "Phase 2 (Leak Detection):\n"
225-
+ " Profiler restarts: %d\n"
226-
+ " Internal steady state growth: +%d KB (threshold: < 20 KB)\n"
227-
+ " Max interval growth: %d KB per 5 restarts (threshold: < 10 KB)\n"
228-
+ "Total Internal: %d KB → %d KB (+%d KB)\n"
229-
+ "\n"
230-
+ "Result: No JVMTI memory leak detected\n"
231-
+ "Note: GetLineNumberTable allocations tracked in Internal NMT category\n"
232-
+ "Note: Each restart destroys Recording, triggering SharedLineNumberTable destructors",
233-
cachedClassIndex,
234-
warmupInternalGrowthKB,
235-
totalRestarts,
236-
steadyStateInternalGrowthKB,
237-
maxIntervalGrowth,
238-
baseline.internalReservedKB,
239-
afterRestarts.internalReservedKB,
240-
totalInternalGrowthKB));
241-
}
242-
24369
private int classCounter = 0;
24470

24571
/**
@@ -279,24 +105,6 @@ private Class<?>[] generateUniqueMethodCalls() throws Exception {
279105
return generatedClasses;
280106
}
281107

282-
/**
283-
* Invokes methods from cached classes to generate profiling samples without creating new
284-
* jmethodIDs. This allows testing whether malloc plateaus once _method_map is populated.
285-
*
286-
* @param cachedClasses array of previously generated classes
287-
* @param iteration current iteration number (used to cycle through classes)
288-
*/
289-
private void invokeCachedClasses(Class<?>[] cachedClasses, int iteration) throws Exception {
290-
// Cycle through cached classes to ensure we hit various methods
291-
int startIndex = (iteration * 10) % cachedClasses.length;
292-
for (int i = 0; i < Math.min(10, cachedClasses.length); i++) {
293-
int index = (startIndex + i) % cachedClasses.length;
294-
if (cachedClasses[index] != null) {
295-
invokeClassMethods(cachedClasses[index]);
296-
}
297-
}
298-
}
299-
300108
/**
301109
* Invokes all int-returning no-arg methods in a class to trigger profiling samples.
302110
*

0 commit comments

Comments
 (0)