Gather the explorer's dominator rules into one rule set

shark/shark-explorer/notes/dominator-tree.md

@@ -21,13 +21,13 @@ per open heap dump, built once, holding every object of the dump.
 **Which reference matchers go in matters, and it isn't "none".** The matchers do two unrelated jobs,
 and only one of them belongs here:
 
-- **Reference strength** is `ReferenceStrengthReader.WEAKENING_REFERENCE_MATCHERS`, one
-  `IgnoredReferenceMatcher` per field of `WEAKENING_FIELDS_BY_CLASS_NAME` and nothing else, read off the
-  same map that gives those fields their strength: the `referent` and `zombie` of the five reference
-  classes, plus the cache and thread local fields. Follow one of those and a weak reference looks like it
-  retains its referent, and retained size stops meaning anything. **Required.** A field pattern matches on
-  any class of an object's hierarchy, so a `KeyedWeakReference` is covered by the `WeakReference` entry and
-  needs none of its own.
+- **Reference strength** is `ExplorerRules.weakeningReferenceMatchers`, one `IgnoredReferenceMatcher` per
+  field of its weakening field rules and nothing else, derived from the same list that gives those fields
+  their strength: the `referent` of the five reference classes, the `zombie` of Android's
+  `FinalizerReference`, and the cache and thread local fields. Follow one of those and a weak reference
+  looks like it retains its referent, and retained size stops meaning anything. **Required.** A field
+  pattern matches on any class of an object's hierarchy, so a `KeyedWeakReference` is covered by the
+  `WeakReference` entry and needs none of its own.
 - **Deliberately not `JdkReferenceMatchers.REFERENCES`**, which is that list plus the `prev`, `next` and
   `element` links of the lists a runtime keeps its `Finalizer`s, `FinalizerReference`s and `Cleaner`s on,
   ignored there so that a leak trace can't run through the queue of objects waiting to be finalized.
@@ -185,8 +185,8 @@ them apart, so it hands the bytes to the root — which is what `ReachabilityStr
 ## A cache is not an owner: the `CACHE` strength
 
 The fix for the above is to stop treating a cache's reference as retaining. `CACHE` ranks between
-`STRONG` and `SOFT`, and `ReferenceStrengthReader.CACHE_FIELDS_BY_CLASS_NAME` is the curated list of
-fields it applies to — one entry today, `coil3.memory.RealStrongMemoryCache$InternalValue.image`.
+`STRONG` and `SOFT`, and `ExplorerRules`' cache entries are the curated list of fields it applies to — one
+today, `coil3.memory.RealStrongMemoryCache$InternalValue.image`.
 
 The mechanics fall out of the weak reference machinery that was already there, because the rule the
 explorer wants is exactly the rule for a weak reference: **the target's strength decides**. A weakening
@@ -221,14 +221,14 @@ itself and retained 4.2 KB and 3.9 KB instead of their windows. The damaging ref
 `PhoneFallbackEventHandler.mView`, `RealWorkflowLifecycleOwner.view`, `ComposeToastServiceImpl.rootView`,
 view bindings and `StandardRowSpec$StandardViewHolder.itemView`.
 
-`OwnerReferences` applies a curated list of `OwnerRule`s: a class whose instances something owns, plus the
-references that own them — named fields, or the virtual references a class's instances hand out. Three
-today — `android.view.View` owned by the `ViewGroup` that reads it as a child (see below) and by
-`Activity.mDecor` or `Dialog.mDecor`, and `android.app.Activity` owned by
-`ActivityThread$ActivityClientRecord.activity`. After: **every one of the 277 child views is under its
-parent**, the dialog's `DecorView` is dominated by the `PartialModalDialog`, and `MainActivity` retains
-18 MB under the record the framework runs it from, which is the second largest rectangle under the GC
-roots.
+`OwnerReferences` applies a curated list of `OwnerRule`s from `ExplorerRules`: a class whose instances
+something owns, plus the references that own them — named fields, or the virtual references a class's
+instances hand out. Three today — `android.view.View` owned by the `ViewGroup` that reads it as a child
+(see below) and by `Activity.mDecor` or `Dialog.mDecor`, and `android.app.Activity` owned by the
+`ActivityThread` that reads it out of `mActivities` (see below). After: **every one of the 277 child views
+is under its parent**, the dialog's `DecorView` is dominated by the `PartialModalDialog`, and
+`MainActivity` retains 18 MB under the thread that runs it, which is the second largest rectangle under
+the GC roots.
 
 **A rule is parked, not dropped, and that's the whole design.** The walk in
 `HeapReachability.walkFromGcRoots` keeps a second queue per strength and only takes from it once the main
@@ -241,7 +241,7 @@ rule the 82 MB dump comes out at exactly the same numbers as before — 80 MB st
 28 B local, 2.6 KB finalizer, 186 KB unreachable.
 
 **Parking is also why a rule needs no check on the state of the object**, which is the thing that looks
-missing when you read `RULES`. "The parent owns an *attached* view" needs no attachment test, because a
+missing when you read the rules. "The parent owns an *attached* view" needs no attachment test, because a
 detached hierarchy isn't held by whatever holds the window: if the parent isn't reachable, no owner
 reference reaches the child and the fallback handles it. "Unless the activity is destroyed" needs no
 `mDestroyed` test either — `ActivityThread.handleDestroyActivity` sets `mDecor = null` and takes the
@@ -299,6 +299,58 @@ Byte counts are untouched by all of it, which is the check that no object moved
 strong, 2.05 MB thread local, 28 B local, 2.6 KB finalizer, 190 KB unreachable, 1,019,837 objects, before
 and after.
 
+### An `ActivityThread` points at the activities it runs: the second virtual reference
+
+`ActivityThreadReferenceReader` gives an `ActivityThread` one reference per running activity, named
+`mActivities` and marked virtual, which is what the activity `OwnerRule` claims ownership through. Same
+shape as the `ViewGroup` one above, and for the same reason: the field is an
+`ArrayMap<IBinder, ActivityClientRecord>`, so every activity of every dump is five objects down from the
+thread that runs it, and the record is the only thing a field rule could name.
+
+The rule this replaced named `ActivityThread$ActivityClientRecord.activity`, which worked and read badly.
+A record is an implementation detail of how the framework runs an activity, so a tree built on it draws
+every screen of an app under a different unnamed record instead of side by side under the one thread that
+runs them all. Measured on `leak_asynctask_o.hprof`: the live `MainActivity` retains 51,634 B either way,
+its dominator moves from `ActivityThread$ActivityClientRecord` to `ActivityThread`, and the chain down to
+it goes from `mActivities → ArrayMap, mArray → Object[], 1 → ActivityClientRecord, activity → MainActivity`
+to `mActivities → MainActivity`. The leaked `MainActivity` in that dump is untouched — 211,038 B under the
+`MainActivity$2` that leaks it — which is the fallback doing its job on a destroyed activity.
+
+**Read the `ArrayMap` the way the map reads itself**: keys at the even slots, values at the odd ones, over
+the first `mSize` pairs. Bounded by the count for the same reason `mChildrenCount` bounds the children, and
+it matters more here: an `ArrayMap` doesn't null the slots it gives up, it leaves them for the next put, so
+past the count is exactly where the record of a destroyed activity is still written down. Attributing that
+activity to the framework would hide the leak. `an activity in a slot the map doesn't count is not one the
+process is running` pins it.
+
+## One rule set, and why the explorer can't just reuse shark's matchers
+
+Every curated list is in `ExplorerRules`: the fields that weaken and how, and the `OwnerRule`s. Each used
+to be declared next to the class applying it, which is fine until you want to read them all and ask whether
+they're still true — and the `java.lang.ref` fields were for a while declared twice over, the strengths here
+and the ignoring of the same fields inherited from LeakCanary. That is worth recording even though the
+second copy is gone, because of *why* it was there.
+
+**That duplication was not a design, it was the absence of a place to put a strength.** Shark's matchers
+are a boolean: `IgnoredReferenceMatcher` carries no payload, `FieldInstanceReferenceReader` drops a matched
+reference with `if (referenceMatcher !is IgnoredReferenceMatcher)`, and `Reference.LazyDetails` carries only
+a `matchedLibraryLeak`. So there is nowhere for "and it's weak" to live, and no way to ask shark which
+matcher dropped a reference. Two ways out: add a strength-carrying matcher to shark — `ReferenceMatcher` is
+`sealed`, so it has to be added *in* shark, an ABI change plus a reader that emits what it currently drops,
+on LeakCanary's hot path — or invert the direction and derive the ignore list from the strengths. The second
+is what the matchers section above describes, and it's why the explorer keeps its own copy of those field
+names rather than importing one.
+
+One redundancy went with the gathering: `zombie` on the four reference classes that don't declare it, which
+only Android's `FinalizerReference` has. That takes the matcher list from 12 to 8, and A/B'd on
+`leak_asynctask_o.hprof` and on the 39 MB `large-dump.hprof` the two lists come out at the same tree —
+every per-strength byte and object count identical, totals identical, and the same number of children
+under the root weighing the same.
+
+**GC root strength is still code**, in `GcRoot.reachabilityStrength()`, and is deliberately not part of the
+rule set: it's a mapping from a `sealed` hierarchy, so making it data means mirroring that hierarchy in an
+enum of our own — more duplication of exactly the kind this section is about.
+
 ## What holds an object: one chain, with the dominators on it marked
 
 "What holds this" is answered as a single chain from a GC root down to the object, `rootPathTo`, drawn

shark/shark-explorer/shark-explorer-core/src/main/java/shark/explorer/ActivityThreadReferenceReader.kt

@@ -0,0 +1,108 @@
+package shark.explorer
+
+import shark.HeapGraph
+import shark.HeapObject
+import shark.HeapObject.HeapInstance
+import shark.Reference
+import shark.Reference.LazyDetails
+import shark.ReferenceLocationType.INSTANCE_FIELD
+import shark.ValueHolder
+
+/**
+ * The activities the process is running, as references from the `ActivityThread` straight to each one, the
+ * way you'd describe what the field holds — `mActivities` — rather than the way it's stored.
+ *
+ * It's stored as an `ArrayMap<IBinder, ActivityClientRecord>`, so every activity of every dump is five
+ * objects down from the thread that runs it: `ActivityThread.mActivities → ArrayMap → Object[] → [1] →
+ * ActivityClientRecord.activity → MainActivity`. Two things follow from reading it that way. A path spends
+ * four steps saying what one says, and it's the record that gets to own the activity — see [OwnerReferences]
+ * — which draws every screen of an app under a different unnamed record instead of side by side under the
+ * one thread running them.
+ *
+ * So this adds one virtual reference per running activity, named after the field they're in. **Additive** —
+ * the map, its array and each record are still reached through `mActivities` and are still nodes holding
+ * their own bytes, because the explorer needs every object of a heap dump to be a node exactly once (see
+ * [ReferenceStrengthReader]). What takes them out of the middle of the tree is the dominator tree rather
+ * than this: both ways to an activity now start at the `ActivityThread`, so it dominates it and the record
+ * is left retaining what it holds besides the activity.
+ */
+internal class ActivityThreadReferenceReader(private val graph: HeapGraph) {
+
+  /**
+   * The class id of `ActivityThread`, resolved once. There is one of these per process, so this turns "is
+   * this the activity thread" into a single comparison — where [HeapGraph.findClassByName] scans every
+   * string of the heap dump and calling it per object costs minutes.
+   */
+  private val activityThreadClassId: Long by lazy {
+    graph.findClassByName(ACTIVITY_THREAD_CLASS_NAME)?.objectId ?: ValueHolder.NULL_REFERENCE
+  }
+
+  /** The running activities when [source] is the `ActivityThread`, and nothing for anything else. */
+  fun runningActivityReferencesOf(source: HeapObject): Sequence<Reference> {
+    if (source !is HeapInstance || source.instanceClassId != activityThreadClassId) {
+      return emptySequence()
+    }
+    val activities = source[ACTIVITY_THREAD_CLASS_NAME, ACTIVITIES_FIELD_NAME]?.valueAsInstance
+      ?: return emptySequence()
+    val entries = activities[ARRAY_MAP_CLASS_NAME, ENTRIES_FIELD_NAME]?.valueAsObjectArray
+      ?: return emptySequence()
+    val elementIds = entries.readRecord().elementIds
+    // An ArrayMap keeps its keys at the even slots and its values at the odd ones, over the first mSize
+    // pairs of an array it grows in chunks. Bounded by the count for the same reason ViewGroup's children
+    // are: a pair the map doesn't count is not an entry, and an ArrayMap doesn't null the slots it gives up
+    // — it leaves them for the next put — so past the count is where a removed activity is still written
+    // down. Which is exactly the leak you'd be looking for, and calling it a running activity hides it.
+    val entryCount = activities[ARRAY_MAP_CLASS_NAME, SIZE_FIELD_NAME]?.value?.asInt
+      ?.coerceIn(0, elementIds.size / 2)
+      ?: return emptySequence()
+    val activityThreadClassObjectId = source.instanceClassId
+    return (0 until entryCount).asSequence()
+      .mapNotNull { entryIndex ->
+        val recordId = elementIds[entryIndex * 2 + 1]
+        if (recordId == ValueHolder.NULL_REFERENCE) {
+          null
+        } else {
+          graph.findObjectByIdOrNull(recordId)?.asInstance
+        }
+      }
+      .mapNotNull { record ->
+        record[ACTIVITY_CLIENT_RECORD_CLASS_NAME, ACTIVITY_FIELD_NAME]?.value?.asNonNullObjectId
+      }
+      .map { activityObjectId ->
+        Reference(
+          valueObjectId = activityObjectId,
+          isLowPriority = false,
+          lazyDetailsResolver = {
+            LazyDetails(
+              name = ACTIVITIES_FIELD_NAME,
+              // The field really is declared here, and it really does hold this activity — through four
+              // objects this reference stands in for. So a path reads ActivityThread.mActivities rather
+              // than naming the record the map happens to keep it in.
+              locationClassObjectId = activityThreadClassObjectId,
+              locationType = INSTANCE_FIELD,
+              isVirtual = true,
+              matchedLibraryLeak = null
+            )
+          }
+        )
+      }
+  }
+
+  companion object {
+    /** Also what an [OwnerRule] names to say that the thread owns the activities read here. */
+    const val ACTIVITY_THREAD_CLASS_NAME = "android.app.ActivityThread"
+
+    private const val ACTIVITY_CLIENT_RECORD_CLASS_NAME =
+      "android.app.ActivityThread\$ActivityClientRecord"
+
+    private const val ARRAY_MAP_CLASS_NAME = "android.util.ArrayMap"
+
+    private const val ACTIVITIES_FIELD_NAME = "mActivities"
+
+    private const val ACTIVITY_FIELD_NAME = "activity"
+
+    private const val ENTRIES_FIELD_NAME = "mArray"
+
+    private const val SIZE_FIELD_NAME = "mSize"
+  }
+}