Merge pull request #2616 from square/py/growth_follow_up

Py/growth follow up
square · Jan 9, 2024 · 3f06db7 · 3f06db7
2 parents e72ad1b + 19e70d5
commit 3f06db7
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 121 deletions.
diff --git a/shark/shark-heap-growth/api/shark-heap-growth.api b/shark/shark-heap-growth/api/shark-heap-growth.api
@@ -1,7 +1,5 @@
 public final class shark/DiffingHeapGrowthDetector {
-	public fun <init> ()V
 	public fun <init> (Lshark/ReferenceReader$Factory;Lshark/GcRootProvider;)V
-	public synthetic fun <init> (Lshark/ReferenceReader$Factory;Lshark/GcRootProvider;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun detectHeapGrowth (Lshark/DiffingHeapGrowthDetector$HeapDumpAfterLoopingScenario;Lshark/InputHeapTraversal;)Lshark/HeapTraversal;
 	public static synthetic fun detectHeapGrowth$default (Lshark/DiffingHeapGrowthDetector;Lshark/DiffingHeapGrowthDetector$HeapDumpAfterLoopingScenario;Lshark/InputHeapTraversal;ILjava/lang/Object;)Lshark/HeapTraversal;
 }
@@ -21,7 +19,6 @@ public final class shark/DiffingHeapGrowthDetector$HeapDumpAfterLoopingScenario
 
 public abstract interface class shark/HeapTraversal : shark/InputHeapTraversal {
 	public static final field Companion Lshark/HeapTraversal$Companion;
-	public abstract fun getGrowing ()Z
 	public abstract fun getShortestPathTree ()Lshark/ShortestPathNode;
 }
 
@@ -31,30 +28,27 @@ public final class shark/HeapTraversal$Companion {
 
 public final class shark/HeapTraversalWithDiff : shark/HeapTraversal {
 	public fun <init> (Lshark/ShortestPathNode;Ljava/util/List;)V
-	public fun getGrowing ()Z
 	public final fun getGrowingNodes ()Ljava/util/List;
 	public fun getShortestPathTree ()Lshark/ShortestPathNode;
 	public fun toString ()Ljava/lang/String;
 }
 
 public final class shark/InitialHeapTraversal : shark/HeapTraversal {
 	public fun <init> (Lshark/ShortestPathNode;)V
-	public fun getGrowing ()Z
 	public fun getShortestPathTree ()Lshark/ShortestPathNode;
 }
 
 public abstract interface class shark/InputHeapTraversal {
 }
 
-public final class shark/LoopingHeapDumper {
-	public fun <init> (ILshark/HeapGraphProvider;I)V
-	public synthetic fun <init> (ILshark/HeapGraphProvider;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
-	public final fun dumpHeapRepeated (Lkotlin/jvm/functions/Function0;)Lkotlin/sequences/Sequence;
+public final class shark/LiveHeapGrowthDetector {
+	public fun <init> (ILshark/HeapGraphProvider;ILshark/LoopingHeapGrowthDetector;)V
+	public final fun detectRepeatedHeapGrowth (Lkotlin/jvm/functions/Function0;)Lshark/HeapTraversalWithDiff;
 }
 
 public final class shark/LoopingHeapGrowthDetector {
 	public fun <init> (Lshark/DiffingHeapGrowthDetector;)V
-	public final fun repeatDiffsWhileGrowing (Lkotlin/sequences/Sequence;)Lshark/HeapTraversalWithDiff;
+	public final fun detectRepeatedHeapGrowth (Lkotlin/sequences/Sequence;)Lshark/HeapTraversalWithDiff;
 }
 
 public final class shark/NoHeapTraversalYet : shark/InputHeapTraversal {

diff --git a/shark/shark-heap-growth/src/main/java/shark/DiffingHeapGrowthDetector.kt b/shark/shark-heap-growth/src/main/java/shark/DiffingHeapGrowthDetector.kt
@@ -8,16 +8,12 @@ import shark.HeapObject.HeapClass
 import shark.HeapObject.HeapInstance
 import shark.HeapObject.HeapObjectArray
 import shark.HeapObject.HeapPrimitiveArray
-import shark.internal.hppc.LongScatterSet
 import shark.ReferenceLocationType.ARRAY_ENTRY
+import shark.internal.hppc.LongScatterSet
 
 class DiffingHeapGrowthDetector(
-  private val referenceReaderFactory: ReferenceReader.Factory<HeapObject> = AndroidReferenceReaderFactory(
-    JdkReferenceMatchers.defaults
-  ),
-  private val gcRootProvider: GcRootProvider = MatchingGcRootProvider(
-    JdkReferenceMatchers.defaults
-  ),
+  private val referenceReaderFactory: ReferenceReader.Factory<HeapObject>,
+  private val gcRootProvider: GcRootProvider,
 ) {
 
   data class HeapDumpAfterLoopingScenario(

diff --git a/shark/shark-heap-growth/src/main/java/shark/HeapTraversal.kt b/shark/shark-heap-growth/src/main/java/shark/HeapTraversal.kt
@@ -15,12 +15,6 @@ sealed interface HeapTraversal : InputHeapTraversal {
    */
   val shortestPathTree: ShortestPathNode
 
-  /**
-   * Whether this traversal yielded a [shortestPathTree] that grew compared to the previous
-   * traversal.
-   */
-  val growing: Boolean
-
   companion object {
 
     /**
@@ -40,9 +34,7 @@ sealed interface HeapTraversal : InputHeapTraversal {
 
 class InitialHeapTraversal constructor(
   override val shortestPathTree: ShortestPathNode
-) : HeapTraversal {
-  override val growing get() = true
-}
+) : HeapTraversal
 
 class HeapTraversalWithDiff(
   override val shortestPathTree: ShortestPathNode,
@@ -52,8 +44,7 @@ class HeapTraversalWithDiff(
    */
   val growingNodes: List<ShortestPathNode>
 ) : HeapTraversal {
-  override val growing get() = growingNodes.isNotEmpty()
   override fun toString(): String {
-    return "HeapTraversalWithDiff(growing=$growing), growingNodes=\n$growingNodes"
+    return "HeapTraversalWithDiff(growingNodes=\n$growingNodes"
   }
 }
diff --git a/.../src/main/java/shark/LoopingHeapDumper.kt → ...main/java/shark/LiveHeapGrowthDetector.kt b/.../src/main/java/shark/LoopingHeapDumper.kt → ...main/java/shark/LiveHeapGrowthDetector.kt
@@ -2,10 +2,11 @@ package shark
 
 import shark.DiffingHeapGrowthDetector.HeapDumpAfterLoopingScenario
 
-class LoopingHeapDumper(
+class LiveHeapGrowthDetector(
   private val maxHeapDumps: Int,
   private val heapGraphProvider: HeapGraphProvider,
-  private val scenarioLoopsPerDump: Int = 1,
+  private val scenarioLoopsPerDump: Int,
+  private val detector: LoopingHeapGrowthDetector
 ) {
 
   init {
@@ -17,8 +18,12 @@ class LoopingHeapDumper(
     }
   }
 
-  // todo name repeat?
-  fun dumpHeapRepeated(
+  fun detectRepeatedHeapGrowth(repeatedScenario: () -> Unit): HeapTraversalWithDiff {
+    val heapDumps = dumpHeapRepeated(repeatedScenario)
+    return detector.detectRepeatedHeapGrowth(heapDumps)
+  }
+
+  private fun dumpHeapRepeated(
     repeatedScenario: () -> Unit,
   ): Sequence<HeapDumpAfterLoopingScenario> {
 

diff --git a/shark/shark-heap-growth/src/main/java/shark/LoopingHeapGrowthDetector.kt b/shark/shark-heap-growth/src/main/java/shark/LoopingHeapGrowthDetector.kt
@@ -5,7 +5,7 @@ import shark.DiffingHeapGrowthDetector.HeapDumpAfterLoopingScenario
 class LoopingHeapGrowthDetector(
   private val heapGrowthDetector: DiffingHeapGrowthDetector
 ) {
-  fun repeatDiffsWhileGrowing(
+  fun detectRepeatedHeapGrowth(
     heapDumps: Sequence<HeapDumpAfterLoopingScenario>
   ): HeapTraversalWithDiff {
     var i = 1
@@ -18,7 +18,7 @@ class LoopingHeapGrowthDetector(
         SharkLog.d {
           "After $iterationCount (+ ${heapDump.scenarioLoopCount}) iterations and heap dump $i: ${diffResult.growingNodes.size} growing nodes"
         }
-        if (!diffResult.growing) {
+        if (diffResult.growingNodes.isEmpty()) {
           return diffResult
         }
       }

diff --git a/shark/shark-heap-growth/src/test/java/shark/HeapGrowthDetectorFakeDumpTest.kt b/shark/shark-heap-growth/src/test/java/shark/HeapGrowthDetectorFakeDumpTest.kt
@@ -18,7 +18,6 @@ class HeapGrowthDetectorFakeDumpTest {
     )
 
     assertThat(heapTraversal).isInstanceOf(InitialHeapTraversal::class.java)
-    assertThat(heapTraversal.growing).isTrue
   }
 
   @Test
@@ -51,7 +50,7 @@ class HeapGrowthDetectorFakeDumpTest {
 
     val traversal = detector.detectHeapGrowth(dumps)
 
-    assertThat(traversal.growing).isFalse
+    assertThat(traversal.growingNodes).isEmpty()
   }
 
   @Test
@@ -69,7 +68,7 @@ class HeapGrowthDetectorFakeDumpTest {
 
     val traversal = detector.detectHeapGrowth(dumps)
 
-    assertThat(traversal.growing).isFalse
+    assertThat(traversal.growingNodes).isEmpty()
   }
 
   @Test
@@ -86,9 +85,6 @@ class HeapGrowthDetectorFakeDumpTest {
 
     val traversal = detector.detectHeapGrowth(dumps)
 
-    assertThat(traversal.growing).isTrue
-
-    traversal as HeapTraversalWithDiff
     assertThat(traversal.growingNodes).hasSize(1)
   }
 
@@ -106,7 +102,7 @@ class HeapGrowthDetectorFakeDumpTest {
 
     val traversal = detector.detectHeapGrowth(dumps)
 
-    assertThat(traversal.growing).isFalse
+    assertThat(traversal.growingNodes).isEmpty()
   }
 
   @Test
@@ -126,7 +122,6 @@ class HeapGrowthDetectorFakeDumpTest {
 
     val traversal = detector.detectHeapGrowth(dumps)
 
-    traversal as HeapTraversalWithDiff
     val growingNode = traversal.growingNodes.first()
 
     assertThat(growingNode.selfObjectCount).isEqualTo(1)
@@ -135,12 +130,12 @@ class HeapGrowthDetectorFakeDumpTest {
     assertThat(growingNode.children).hasSize(1)
   }
 
-  private fun DiffingHeapGrowthDetector.detectHeapGrowth(heapDumps: List<HeapDumpAfterLoopingScenario>): HeapTraversal {
+  private fun DiffingHeapGrowthDetector.detectHeapGrowth(heapDumps: List<HeapDumpAfterLoopingScenario>): HeapTraversalWithDiff {
     return heapDumps.fold<HeapDumpAfterLoopingScenario, InputHeapTraversal>(
       initial = NoHeapTraversalYet
     ) { previous, dump ->
       detectHeapGrowth(dump, previous)
-    } as HeapTraversal
+    } as HeapTraversalWithDiff
   }
 
   private fun HprofWriterHelper.classWithStringsInStaticField(vararg strings: String) {