perf(heap-dump): add SoftReference caches for BFS/string-scan/leak-re… - #395
Open
ziyilin wants to merge 1 commit into
Open
perf(heap-dump): add SoftReference caches for BFS/string-scan/leak-re…#395ziyilin wants to merge 1 commit into
ziyilin wants to merge 1 commit into
Conversation
Three HeapDump operations re-executed expensive MAT computations on every call within the same session, because the @Cacheable annotation on queryByCommand never produced a hit: 1. getMergePathToGCRoots - Helper.buildHeapObjectArgument returns a new anonymous IHeapObjectArgument per call, and Cache.CacheKey compares arguments with Arrays.equals, which falls back to reference identity, so every call missed and triggered a full-heap BFS. 2. getLeakReport - only the raw MAT IResult was cached; the LeakReport object and its embedded shortest paths (each triggering further BFS) were rebuilt from scratch every time. 3. getStrings - called the 2-arg queryByCommand, bypassing @Cacheable entirely, so every call rescanned all strings in the heap. Caching: explicit SoftReference caches in AnalysisContext, reclaimable under memory pressure. - mergePathTreeCache, keyed by MergePathTreeCacheKey: the objectIds are cloned (immune to later mutation by the caller) and sorted, so the same object set hits regardless of request order. The merge_shortest_paths result does not depend on that order; it only affects sibling enumeration in the resulting tree, which no caller relies on. - leakReportCache holds the built LeakReport, avoiding both the BFS and the object reconstruction. - stringsCache is keyed by the MAT pattern actually used, so paging through one search scans the heap once while the retained set stays proportional to what was asked for. Entries whose result has been reclaimed are pruned on insert. Locking: the BFS-backed caches use dedicated monitors (mergePathTreeLock, leakReportLock) instead of the context monitor. AnalysisContext is public and shared, so running a minutes-long BFS while holding its monitor would block unrelated code that locks on the context (e.g. the classLoaderExplorerData path). stringsCache takes no lock: the query is idempotent, so a cold-start race at worst repeats the scan, whereas a lock held across a full-heap scan would stall every other search. findStrings: the pattern is now passed to MAT as a compiled Pattern argument instead of being spliced into the command string ("find_strings java.lang.String -pattern <p>"). CommandLine.tokenize splits on whitespace, so a spliced pattern containing a space would be parsed as extra arguments and rejected with a SnapshotException. Matching deliberately stays inside MAT. Serving specific patterns from a single cached full scan by filtering in Java on the result's first column was considered and rejected: that column is the display name ("java.lang.String @ 0x..." followed by the value truncated at 256 chars), so filtering on it would match class names and addresses and would miss content beyond the truncation point. MAT's FindStringsQuery matches IObject#getClassSpecificName - the value, resolved up to 1024 chars - which is the intended semantics, and the new unit tests guard it: the display-name prefix must not match every string, a sentinel planted past the truncation point must still be found, and paging must be served from one result set.
ziyilin
force-pushed
the
heapdump_analysis_cache
branch
from
July 30, 2026 15:36
f5d3017 to
4815125
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
…port results
Problem: Three HeapDump operations redundantly re-execute expensive MAT computations on every call within the same session:
getMergePathToGCRoots - @Cacheable on queryByCommand always misses because Helper.buildHeapObjectArgument returns a new anonymous IHeapObjectArgument instance each time; Cache.CacheKey uses Arrays.equals which falls back to reference identity → never matches. Each miss triggers a full-heap BFS (O(N) graph traversal + disk I/O).
getLeakReport - only the raw MAT IResult was cached (SoftReference), but the LeakReport Java object and its embedded Shortest Paths (which internally trigger additional BFS calls) were rebuilt from scratch on every invocation.
getStrings - called the 2-arg queryByCommand(context, command) which bypasses the @Cacheable 3-arg version entirely, causing a full-heap string scan on every call with any pattern.
Solution: Add explicit SoftReference-based caches in AnalysisContext, bypassing the broken @Cacheable mechanism:
All caches use double-checked locking with synchronized(context) for thread safety, and SoftReference to allow GC under memory pressure.