Draw the activities an app is running under the thread running them - #2936
Merged
Conversation
ActivityThread keeps them in mActivities, an ArrayMap from an activity's token to the ActivityClientRecord it runs it from, so the way from the thread to a screen ran ActivityThread.mActivities → ArrayMap.mArray → Object[][1] → ActivityClientRecord.activity: five objects to say the app is running a screen, three of them a map's bookkeeping. Naming the record's field as the owner, which is what the activity OwnerRule did, therefore left the map, its array and the record between the thread and every activity. So RunningActivityReferenceReader adds one virtual reference per activity, named activities, and the rule claims ownership through it instead. Additive like the ViewGroup one: the map, its Object[] and every record are still reached through mActivities and still nodes of their own, and it's the dominator tree that takes them out of the middle, both ways to an activity now starting at the thread. Named activities rather than mActivities, which would be two references of that name out of one ActivityThread pointing at different things — the map, and each activity in it — with nothing in a path or a referrer list to tell them apart. On large-dump.hprof, which is running two activities: MainActivity's dominator goes from the record to the ActivityThread, the chain from a GC root to it from 6 steps to 3, and the two records go from retaining 2,125,170 B and 14,552 B to 381 B and 361 B. The activities' own retained sizes don't move, and no byte count does either, which is what says no object left the graph. Bounded by mSize rather than by the length of mArray, for the reason ViewChildReferenceReader bounds by mChildrenCount: a heap dump catches threads mid-method, and ArrayMap.put fills a pair before it counts it.
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.
What
Adds a virtual
activitiesreference fromActivityThreadto each activity inmActivities, and moves the activityOwnerRuleonto it.ActivityThreadkeeps its activities inmActivities, anArrayMapfrom an activity's token to theActivityClientRecordit runs it from, so the way from the thread down to a screen really reads:Five objects to say the app is running a screen, three of them a map's bookkeeping. The rule named
ActivityClientRecord.activityas the owner, which is a slot of that map rather than the thing running the activity, so all three stayed between the thread and every activity.RunningActivityReferenceReadernow adds one virtual reference per activity and the rule claims ownership through that instead. Additive, likeViewChildReferenceReader: the map, itsObject[]and every record are still reached throughmActivitiesand still nodes holding their own bytes. It's the dominator tree that takes them out of the middle, both ways to an activity now starting at the thread.It's named
activitiesrather thanmActivitiesbecause a virtual reference borrowing a real field's name would be two references calledmActivitiesout of oneActivityThreadpointing at different things — the map, and each activity in it — with nothing in a path or a referrer list to tell them apart.Measured on
large-dump.hprof, which is running two activitiesMainActivitydominatorActivityClientRecordActivityThreadMainActivityMainActivityretainsPaymentActivityretainsmActivitiesArrayMapretainsThe activities' own retained sizes don't move — 2,124,789 B and 11,995 B either way — because a record retained little beyond the activity in it. What moves is where those bytes are drawn: two screens side by side under the thread instead of two piles of map bookkeeping.
Chains into an activity's internals also get a step shorter and a better first step. The shortest way to the
Bundles underMainActivityused to run through a leakedSquareActivity.foot → ArrayList → Object[], that being fewer steps than the map; it now runsThread → ActivityThread, activities → MainActivity.Byte counts are identical before and after, which is the check that no object left the graph: 30,090,032 B strong, 28,302 B thread local, 1,444 B soft, 261 B weak, 9,353 B finalizer, 631,761 B unreachable, 387,971 objects.
Opening the dump costs at most 2% more — median of five steady state opens 2.02 s with the reader against 1.98 s without, ranges overlapping. That's the fourth sequence concatenation
retainingReferencesOfnow does per object; the reader itself is one class id comparison for everything that isn't the activity thread.Details worth a look in review
mSize, not by the length ofmArray. Same reasonViewChildReferenceReaderbounds bymChildrenCount:ArrayMap.removeAtnulls the pair it gives up so the tail is null anyway, but a heap dump catches threads mid-method andArrayMap.putfills a pair before it counts it. Calling an uncounted pair an activity the app is running would attribute an activity the framework has let go of to the framework, which is exactly the leak you'd be looking for.handleDestroyActivitytakes its record out ofmActivities) lands under whatever leaks it, and one in an uncounted pair lands under the record pointing at it. Both pinned by new tests.ArrayMaponly.mActivitieshas been one since Lollipop, seven releases before LeakCanary'sminSdkof 24, so theHashMapit was before that is deliberately not read: a dump without theArrayMapshape logs a line and reads as a thread running nothing.No changelog entry: this is Shark Explorer, whose
Unreleasedsection is still just "Initial release".notes/dominator-tree.mdgains the section for this reader and the numbers above; itsOwnerRulesection grows a "name the thing that holds it, not the slot it's in" bullet, since nameable turned out to be the floor rather than the bar.Test plan
./gradlew :shark:shark-explorer:shark-explorer-core:check :shark:shark-explorer:shark-explorer-app:check :shark:shark-explorer:shark-explorer-jdwp:check— green, detekt included.OwnerReferencesTest: an activity the framework hasn't destroyed is held by the thread running it (with the reference read back as anINSTANCE_FIELDnamedactivitiesonActivityThread), one in a slot the map doesn't count isn't, and a destroyed one is held by whatever leaks it.large-dump.hprofin the app and read the numbers in the table off it.🤖 Generated with Claude Code