Shrink the per-object entries of the heap dump index - #2964
Merged
Conversation
Every instance and object array in a heap dump belongs to a class that has a class dump record, so an entry doesn't need the 4 or 8 bytes of a class id: the index of that class among the class dump records is 2 bytes for the few thousand classes a heap dump has. The first pass now collects those ids so the second can binary search one, and the class index is sorted by class id, so reading its key back at that index is all it takes to resolve one. On the Android heap dumps in our test resources the four object indexes go from 12.0-14.1 bytes per object to 10.9-12.6, and on a JVM heap dump from 19.2 to 14.8. The frozen numbers move by exactly what that predicts: 2 bytes times the 65814 instances and 5468 object arrays of leak_asynctask_o.hprof is 142564 bytes, and what indexing it retains drops by 142561. Indexing binary searches a class id where it used to write it out, and reading an object resolves the index back to one. Neither is measurable end to end: over five runs alternating between the two implementations, the median analysis of leak_asynctask_o.hprof is 159 ms before and 161 ms after, and of leak_asynctask_m.hprof 168 ms before and 165 ms after. A heap dump with an object whose class has no class dump record now fails to open rather than failing later: nothing can be read from such an object, since its fields are laid out by a class the heap dump doesn't contain. None of the Android or JVM heap dumps in our test resources hold one, including the two that carry unloaded classes.
Every entry of the four object indexes started with the whole object id
that keys it: 4 bytes on an Android heap dump, 8 on a JVM one. Ids are
addresses within one heap, so what they need is set by how far apart
they are, not by how large they are. An entry now stores its id as an
offset from the smallest id in the heap dump, over as many bytes as the
distance up to the largest one needs, which the first pass computes by
reading the id of every object dump record instead of skipping over it.
The base is the smallest id, so no offset wraps and encoding is
monotonic: stored keys sort in the same order as the ids they encode.
Sorting a builder and binary searching a map therefore work on the
stored bytes, and only keyAt() decodes.
Measured per dump, over the four object indexes:
leak_asynctask_o span 3.35 GB 4 bytes 1414042 (0%)
leak_asynctask_m span 2.33 GB 4 bytes 1579311 (0%)
leak_asynctask_pre_m span 3.97 MB 3 bytes 531914 -> 486529
compose_leak span 3.86 GB 4 bytes 3221769 (0%)
gcroot_unknown_object span 1.71 GB 4 bytes 5172045 (0%)
unloaded_classes-stripped span 4.07 GB 4 bytes 4111318 (0%)
gc_root_in_non_primary_heap span 3.39 GB 4 bytes 1451409 (0%)
safe_iterable_map span 1.67 GB 4 bytes 4208959 (0%)
hashmap_api_25 span 3.47 GB 4 bytes 1692090 (0%)
a JVM heap dump span 536 MB 4 bytes 14.77 -> 10.77
bytes per object
So this buys nothing on most Android heap dumps, and that is not a
surprise: ART writes the low 32 bits of a 64 bit address, and the image
space and the app heap land far enough apart that the span still needs
4 bytes. It buys 4 bytes per object on every JVM heap dump, 27% of the
index, which is where the index gets big enough to be the binding
constraint — gigabytes on the hundreds-of-millions-of-objects dumps that
shark-cli and Shark Explorer are pointed at.
Per-page bases rather than one global base would shrink Android keys to
2 bytes, since a page of a sorted index covers a narrow range, but the
base of a page is only known after sorting, so the entries would have to
be written wide and compacted afterwards. That trades peak memory for
retained memory, which is the wrong way round on a device, and the
retained heap test wouldn't even see the regression.
HprofRetainedHeapPerfTest: only the leak shares expectation moves, from
0.86 MB to 0.74 MB. The dumps it builds hold 60022 objects with ids
spanning 60010, which needs 2 bytes rather than the 4 an id takes, and
the measured drop is 120044 bytes, i.e. exactly 2 bytes times the 60022
entries of the one index that is still open at that point. The
leak_asynctask_o and leak_asynctask_m expectations don't move, because
those dumps stay on 4 byte keys. HprofIOPerfTest doesn't move either:
reading an id out of the already buffered source instead of skipping it
reads the same bytes.
Reading a key of any width through a byte at a time loop cost 15% to 17%
of end to end analysis time, consistently across five alternating runs.
Binary searching an index calls it on every probe, so the two widths a
real heap dump lands on are read as a whole int or long instead. With
that, five more alternating runs put the median analysis of
leak_asynctask_o at 159 ms before and 161 ms after, and of
leak_asynctask_m at 166 ms before and 167 ms after; indexing goes from
19 ms to 19 ms and from 25 ms to 26 ms.
Everything here is internal, so there is no ABI change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
HprofInMemoryIndexis what sets LeakCanary's on-device memory cost, so this looks at the twobiggest fields in a per-object entry: the class id every instance and object array carried, and the
object id that keys every entry. One commit each, each with its own measurement.
The starting point, measured rather than read off the declarations — the brief for this work
estimated ~22 bytes per object, which is a JVM-shaped number:
HotSpotDiagnosticMXBean.dumpHeap)1. Store the index of an object's class, not its class id
Every instance and object array held the id of its class. Every one of those objects belongs to a
class that has a class dump record, and the class index is sorted by class id, so an entry can hold
the index of its class among those records instead — 2 bytes for the few thousand classes of a
typical heap dump — and resolve it back with
classIndex.keyAt(index). No side table: the array ofclass dump ids the first pass collects and the sorted class index are the same sequence, which a
requireinbuildIndexchecks entry for entry.On
leak_asynctask_othe predicted saving is 2 bytes × (65,814 instances + 5,468 object arrays) =142,564 bytes and the measured retained drop is 142,561.
2. Key the index by an offset, not by a whole object id
Ids are addresses within one heap, so what they need is set by how far apart they are, not by how
large they are. An entry now stores its id as an offset from the smallest id in the dump, over as
many bytes as the distance to the largest one needs. The base being the smallest id makes encoding
monotonic, so sorting and binary searching still work on the stored bytes and only
keyAt()decodes.This one buys nothing on most Android heap dumps, and it's worth saying plainly since on-device
memory is the point of the exercise. ART writes the low 32 bits of a 64 bit address, and the image
space and the app heap land far enough apart that the span still needs 4 bytes. It buys 4 bytes per
object on every JVM heap dump — 27% of the index — which is where the index gets big enough to be
the binding constraint: gigabytes on the hundreds-of-millions-of-objects dumps
shark-cliand SharkExplorer are pointed at.
Per-page bases rather than one global base would take Android keys down to 2 bytes, since a page of
a sorted index covers a narrow range. Not here: the base of a page is only known after sorting, so
entries would have to be written wide and compacted afterwards, trading peak memory for retained
memory. That's the wrong way round on a device, and
HprofRetainedHeapPerfTestwouldn't even see theregression.
The frozen perf numbers
HprofRetainedHeapPerfTest, all within the existing ±5% margin:The analysis-step numbers move by the same ~142 KB the index of
leak_asynctask_olost in commit 1,and don't move in commit 2 because that dump stays on 4 byte keys. The leak shares number is the one
that moves twice: those dumps are synthetic, with 8 classes and 60,022 objects whose ids span 60,010,
so commit 1 takes 3 bytes off each instance and object array (1 byte for the class index against the
4 an id took) and commit 2 takes 2 more off every key. Both drops are 120 KB, i.e. exactly the entry
count of the one index still open at that point times the bytes per entry saved.
HprofIOPerfTestdoesn't move at all. The first pass now reads the id of each object dump recordwhere it used to skip over it, but that's a read from an already buffered okio source, so the bytes
read from the file are the same.
Time
Both commits were A/B'd end to end, five runs alternating between the two implementations, comparing
medians of per-run medians (a single JVM run is bimodal here, so one run of each side is worth
nothing — the first attempt showed a 26% "regression" that reran at parity).
Commit 2 did have a real regression on the way: reading a key of any width through a byte at a time
loop cost 15% to 17% of end to end analysis, consistently across all five runs, because binary
searching an index calls it on every probe. Reading the two widths a real heap dump lands on as a
whole int or long removes it. Indexing is unchanged to +1 ms (19 → 19 ms and 25 → 26 ms).
Notes
internal, so there's no ABI change andcheckKotlinAbiis unaffected.no class dump record now fails to open, naming that class id. Nothing could be read from such an
object anyway — its fields are laid out by a class the heap dump doesn't contain — and none of the
10 heap dumps measured here hold one, including the two carrying unloaded classes. There's a test
for it, and a 🔀 changelog entry.
./gradlew buildand./gradlew detektpass on Java 17.🤖 Generated with Claude Code