Fix checkpoint interval in GetLineNumberTableLeakTest comment #328
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix 1.2GB memory leak in SharedLineNumberTable destructor by adding proper null checks and error handling for JVMTI Deallocate calls. Root cause: SharedLineNumberTable destructor was not properly deallocating JVMTI-allocated line number tables. Customer production NMT data showed 1.2GB leak from 3.8M GetLineNumberTable allocations. Changes: - Add null pointer checks for _ptr and jvmti before deallocation - Add error handling for Deallocate failures - Add defensive cleanup in fillJavaMethodInfo for buggy JVMTI impls Test: GetLineNumberTableLeakTest validates memory plateaus during steady state with profiler restarts (+1KB vs +20MB leak without fix). See doc/nmt-jvmti-memory-leak-investigation.md for full analysis. 🤖 Generated with Claude Code Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Revert GetClassMethods fix - required for AGCT profiling Revert previous commit that skipped GetClassMethods for JDK 9+. GetClassMethods must be called to preallocate jmethodIDs for AsyncGetCallTrace (AGCT), which operates in signal handlers where lock acquisition is forbidden. Root cause of 9.2GB memory growth: - AGCT requires jmethodIDs to be preallocated before profiling - GetClassMethods triggers JVM-internal jmethodID allocation - These persist until class unload (necessary for AGCT correctness) - High class churn = high jmethodID memory usage - This is NOT a fixable leak - it's inherent to AGCT architecture Failed fix broke profiling: - CpuDumpSmokeTest and WallclockDumpSmokeTest failed - Stack traces incomplete (missing method information) - AGCT couldn't identify methods without preallocated jmethodIDs Solution: Accept as cost of signal-safe profiling. Customers can: 1. Reduce class churn (cache generated classes, minimize proxies) 2. Monitor NMT Internal category growth 3. Investigate alternative stack walking (JEP 435, --cstack vm) See: https://mostlynerdless.de/blog/2023/07/17/jmethodids-in-profiling-a-tale-of-nightmares/ 🤖 Generated with Claude Code Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Clarify NMT memory calculations in investigation doc The NMT #count is number of classes, not individual jmethodIDs. Each class has 10-20 methods typically, so: - 9.1M classes × 15 methods = ~136M jmethodIDs - 9.2GB / 136M = ~68 bytes per jmethodID (reasonable) This clarifies the misleading '1KB per jmethodID' statement.
Test now checks Internal NMT category (where JVMTI allocations appear) with profiler restarts and tighter thresholds to catch line table leaks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Add try-catch blocks around Long.parseLong() calls to properly handle potential NumberFormatException when parsing NMT output. This addresses review comments about uncaught exceptions. The exceptions are now wrapped in IOException with descriptive messages indicating which NMT category failed to parse. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
…neNumberTableLeakTest.java Co-authored-by: Copilot <[email protected]>
…neNumberTableLeakTest.java Co-authored-by: Copilot <[email protected]>
…neNumberTableLeakTest.java Co-authored-by: Copilot <[email protected]>
…neNumberTableLeakTest.java Co-authored-by: Copilot <[email protected]>
…neNumberTableLeakTest.java Co-authored-by: Copilot <[email protected]>
Co-authored-by: jbachorik <[email protected]>
Copilot
AI
changed the title
[WIP] WIP Address feedback on JVMTI memory leak fix
Fix checkpoint interval in GetLineNumberTableLeakTest comment
Jan 10, 2026
Benchmarks [x86_64 wall]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics. |
Benchmarks [x86_64 cpu]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics. |
Benchmarks [x86_64 memleak]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics. |
Benchmarks [aarch64 wall]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 24 unstable metrics. |
Benchmarks [aarch64 cpu,wall]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics. |
Benchmarks [x86_64 memleak,alloc]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics. |
Benchmarks [x86_64 cpu,wall,alloc,memleak]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 23 unstable metrics. |
Benchmarks [aarch64 cpu,wall,alloc,memleak]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 17 metrics, 21 unstable metrics. |
Benchmarks [aarch64 cpu]Parameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 22 unstable metrics.
|
Benchmarks [aarch64 memleak,alloc]Parameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 14 metrics, 23 unstable metrics.
|
Benchmarks [aarch64 alloc]Parameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 15 metrics, 22 unstable metrics.
|
Benchmarks [aarch64 memleak]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics. |
Benchmarks [x86_64 alloc]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics. |
Benchmarks [x86_64 cpu,wall]Parameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 16 metrics, 22 unstable metrics. |
jbachorik
force-pushed
the
jb/linenumber_leak
branch
3 times, most recently
from
73aadd8 to
cd0c279
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Comment on lines 174-175 incorrectly stated "200 restarts" when the test uses
checkpointInterval = 5.Changes:
< 5 KB per 5 restarts(was< 2 MB per 200 restarts)10-20 KB per 5 restart interval(was~3.2 MB per 200 restarts)💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.