Cleanup Side Effects; Able to Freeze

Author: steve-the-edwards

benchmarks/performance-poetry/complex-poetry/src/androidTest/java/com/squareup/benchmarks/performance/complex/poetry/ComposeInheritanceTest.kt

@@ -8,6 +8,7 @@ import androidx.test.platform.app.InstrumentationRegistry
 import androidx.test.uiautomator.UiDevice
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity.Companion.EXTRA_PERF_CONFIG_INITIALIZING
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity.Companion.EXTRA_PERF_CONFIG_REPEAT
+import com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity.Companion.EXTRA_RUNTIME_COMPOSE_RUNTIME
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoetryActivity.Companion.EXTRA_RUNTIME_FRAME_TIMEOUT
 import com.squareup.benchmarks.performance.complex.poetry.cyborgs.landscapeOrientation
 import com.squareup.benchmarks.performance.complex.poetry.cyborgs.openRavenAndNavigate
@@ -16,6 +17,7 @@ import com.squareup.benchmarks.performance.complex.poetry.cyborgs.waitForPoetry
 import com.squareup.benchmarks.performance.complex.poetry.instrumentation.RenderPassCountingInterceptor
 import org.junit.Assert.fail
 import org.junit.Before
+import org.junit.Ignore
 import org.junit.Test
 import org.junit.runner.RunWith
 
@@ -30,7 +32,8 @@ class RenderPassTest {
     val useInitializingState: Boolean,
     val useHighFrequencyRange: Boolean,
     val baselineExpectation: RenderExpectation,
-    val frameTimeoutExpectation: RenderExpectation
+    val frameTimeoutExpectation: RenderExpectation,
+    val frameTimeoutComposeExpectation: RenderExpectation
   )
 
   data class RenderExpectation(
@@ -56,15 +59,15 @@ class RenderPassTest {
   }
 
   @Test fun renderPassCounterBaselineComplexWithInitializingState() {
-    runRenderPassCounter(COMPLEX_INITIALIZING, useFrameTimeout = false)
+    runRenderPassCounter(COMPLEX_INITIALIZING)
   }
 
   @Test fun renderPassCounterBaselineComplexNoInitializingState() {
-    runRenderPassCounter(COMPLEX_NO_INITIALIZING, useFrameTimeout = false)
+    runRenderPassCounter(COMPLEX_NO_INITIALIZING)
   }
 
   @Test fun renderPassCounterBaselineComplexNoInitializingStateHighFrequencyEvents() {
-    runRenderPassCounter(COMPLEX_NO_INITIALIZING_HIGH_FREQUENCY, useFrameTimeout = false)
+    runRenderPassCounter(COMPLEX_NO_INITIALIZING_HIGH_FREQUENCY)
   }
 
   @Test fun renderPassCounterFrameTimeoutComplexWithInitializingState() {
@@ -79,19 +82,43 @@ class RenderPassTest {
     runRenderPassCounter(COMPLEX_NO_INITIALIZING_HIGH_FREQUENCY, useFrameTimeout = true)
   }
 
+  @Ignore(
+    "Not sure why but this gets stuck on initializing. Compose doesn't get the next" +
+      " frame when this is started by the test, but it does when running directly."
+  )
+  @Test fun renderPassCounterFrameTimeoutComposeComplexWithInitializingState() {
+    runRenderPassCounter(COMPLEX_INITIALIZING, useFrameTimeout = true, useCompose = true)
+  }
+
+  @Test fun renderPassCounterFrameTimeoutComposeComplexNoInitializingState() {
+    runRenderPassCounter(COMPLEX_NO_INITIALIZING, useFrameTimeout = true, useCompose = true)
+  }
+
+  @Test fun renderPassCounterFrameTimeoutComposeComplexNoInitializingStateHighFrequencyEvents() {
+    runRenderPassCounter(
+      COMPLEX_NO_INITIALIZING_HIGH_FREQUENCY,
+      useFrameTimeout = true,
+      useCompose = true
+    )
+  }
+
   private fun runRenderPassCounter(
     scenario: Scenario,
-    useFrameTimeout: Boolean
+    useFrameTimeout: Boolean = false,
+    useCompose: Boolean = false
   ) {
+    if (useCompose) require(useFrameTimeout) { "Only use Compose Frame Timeout." }
+
     val intent = Intent(context, PerformancePoetryActivity::class.java).apply {
       addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
       addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
       putExtra(
         EXTRA_PERF_CONFIG_INITIALIZING,
         scenario.useInitializingState
       )
+      putExtra(EXTRA_RUNTIME_FRAME_TIMEOUT, useFrameTimeout)
       if (useFrameTimeout) {
-        putExtra(EXTRA_RUNTIME_FRAME_TIMEOUT, useFrameTimeout)
+        putExtra(EXTRA_RUNTIME_COMPOSE_RUNTIME, useCompose)
       }
       if (scenario.useHighFrequencyRange) {
         putExtra(EXTRA_PERF_CONFIG_REPEAT, PerformancePoetryActivity.HIGH_FREQUENCY_REPEAT_COUNT)
@@ -111,11 +138,19 @@ class RenderPassTest {
     device.openRavenAndNavigate()
 
     val expectation =
-      if (useFrameTimeout) scenario.frameTimeoutExpectation else
+      if (useFrameTimeout) {
+        if (useCompose) {
+          scenario.frameTimeoutComposeExpectation
+        } else {
+          scenario.frameTimeoutExpectation
+        }
+      } else {
         scenario.baselineExpectation
+      }
 
     val title = if (useFrameTimeout) {
-      "Runtime: FrameTimeout; "
+      val usingCompose = if (useCompose) "(Using Compose Optimizations)" else "(No Compose)"
+      "Runtime: FrameTimeout $usingCompose;"
     } else {
       "Runtime: RenderPerAction; "
     } + scenario.title
@@ -227,6 +262,11 @@ class RenderPassTest {
         totalPasses = 41..42,
         freshRenderedNodes = 85..85,
         staleRenderedNodes = 436..436
+      ),
+      frameTimeoutComposeExpectation = RenderExpectation(
+        totalPasses = 41..42,
+        freshRenderedNodes = 85..85,
+        staleRenderedNodes = 436..436
       )
     )
 
@@ -243,6 +283,11 @@ class RenderPassTest {
         totalPasses = 40..41,
         freshRenderedNodes = 83..83,
         staleRenderedNodes = 431..431
+      ),
+      frameTimeoutComposeExpectation = RenderExpectation(
+        totalPasses = 40..41,
+        freshRenderedNodes = 113..113,
+        staleRenderedNodes = 82..82
       )
     )
 
@@ -270,9 +315,14 @@ class RenderPassTest {
         staleRenderedNodes = 2350..2350
       ),
       frameTimeoutExpectation = RenderExpectation(
-        totalPasses = 64..68,
+        totalPasses = 60..64,
         freshRenderedNodes = 106..108,
         staleRenderedNodes = 679..698
+      ),
+      frameTimeoutComposeExpectation = RenderExpectation(
+        totalPasses = 59..64,
+        freshRenderedNodes = 259..259,
+        staleRenderedNodes = 207..207
       )
     )
 

benchmarks/performance-poetry/complex-poetry/src/androidTest/java/com/squareup/benchmarks/performance/complex/poetry/RenderPassTest.kt

@@ -1,9 +1,6 @@
 package com.squareup.benchmarks.performance.complex.poetry
 
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.MutableState
-import androidx.compose.runtime.mutableStateOf
-import androidx.compose.runtime.remember
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.ClearSelection
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.HandleStanzaListOutput
 import com.squareup.benchmarks.performance.complex.poetry.PerformancePoemWorkflow.Action.SelectNext
@@ -20,7 +17,6 @@ import com.squareup.benchmarks.performance.complex.poetry.views.BlankScreen
 import com.squareup.sample.container.overviewdetail.OverviewDetailScreen
 import com.squareup.sample.poetry.PoemWorkflow
 import com.squareup.sample.poetry.PoemWorkflow.ClosePoem
-import com.squareup.sample.poetry.StanzaListScreen
 import com.squareup.sample.poetry.StanzaListWorkflow
 import com.squareup.sample.poetry.StanzaListWorkflow.NO_SELECTED_STANZA
 import com.squareup.sample.poetry.StanzaScreen
@@ -326,7 +322,7 @@ class PerformancePoemWorkflow(
           .copy(selection = stanzaIndex)
 
         if (stanzaIndex != NO_SELECTED_STANZA) {
-          val previousStanzas = renderProps.stanzas.subList(0, stanzaIndex)
+          val stackedStanzas = renderProps.stanzas.subList(0, stanzaIndex + 1)
             .mapIndexed { index, _ ->
               context.ChildRendering(
                 StanzaWorkflow,
@@ -337,36 +333,20 @@ class PerformancePoemWorkflow(
                 ),
                 key = "$index",
               ) {
-                noAction()
+                when (it) {
+                  CloseStanzas -> ClearSelection(simulatedPerfConfig)
+                  ShowPreviousStanza -> SelectPrevious(simulatedPerfConfig)
+                  ShowNextStanza -> SelectNext(simulatedPerfConfig)
+                }
               }
-            }
-          val visibleStanza = context.ChildRendering(
-            StanzaWorkflow,
-            Props(
-              poem = renderProps,
-              index = stanzaIndex,
-              eventHandlerTag = ActionHandlingTracingInterceptor::keyForTrace
-            ),
-            key = "$stanzaIndex",
-          ) {
-            when (it) {
-              CloseStanzas -> ClearSelection(simulatedPerfConfig)
-              ShowPreviousStanza -> SelectPrevious(simulatedPerfConfig)
-              ShowNextStanza -> SelectNext(simulatedPerfConfig)
-            }
-          }
-
-          val stackedStanzas = visibleStanza.let {
-            (previousStanzas + it).toBackStackScreen<Screen>()
-          }
+            }.toBackStackScreen<Screen>()
 
           return OverviewDetailScreen(
             overviewRendering = BackStackScreen(stanzaListOverview),
             detailRendering = stackedStanzas
           )
         }
 
-
         return OverviewDetailScreen(
           overviewRendering = BackStackScreen(stanzaListOverview),
           selectDefault = {

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoemWorkflow.kt

@@ -25,6 +25,7 @@ import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
 import com.squareup.workflow1.ui.container.BackStackScreen
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableStateFlow
+import java.lang.IllegalStateException
 
 /**
  * Version of [PoemsBrowserWorkflow] that takes in a [SimulatedPerfConfig] to control the
@@ -161,6 +162,22 @@ class PerformancePoemsBrowserWorkflow(
     renderState: State,
     context: RenderContext,
   ): OverviewDetailScreen {
+
+    // Again, then entire `Initializing` state is a smell, which is most obvious from the
+    // use of `Worker.from { Unit }`. A Worker doing no work and only shuttling the state
+    // along is usually the sign you have an extraneous state that can be collapsed!
+    // Don't try this at home.
+    if (renderState is Initializing) {
+      context.runningWorker(TraceableWorker.from("BrowserInitializing") { Unit }, "init") {
+      isLoading.value = true
+      action {
+        isLoading.value = false
+        state = NoSelection
+      }
+    }
+      return OverviewDetailScreen(overviewRendering = BackStackScreen(BlankScreen))
+    }
+
     val poemListProps = Props(
       poems = renderProps,
       eventHandlerTag = ActionHandlingTracingInterceptor::keyForTrace
@@ -174,26 +191,12 @@ class PerformancePoemsBrowserWorkflow(
       choosePoem(selected)
     }
     when (renderState) {
-      // Again, then entire `Initializing` state is a smell, which is most obvious from the
-      // use of `Worker.from { Unit }`. A Worker doing no work and only shuttling the state
-      // along is usually the sign you have an extraneous state that can be collapsed!
-      // Don't try this at home.
-      is Initializing -> {
-        context.runningWorker(TraceableWorker.from("BrowserInitializing") { Unit }, "init") {
-        isLoading.value = true
-        action {
-          isLoading.value = false
-          state = NoSelection
-        }
-      }
-        return OverviewDetailScreen(overviewRendering = BackStackScreen(BlankScreen))
-      }
       is NoSelection -> {
         return OverviewDetailScreen(
-            overviewRendering = BackStackScreen(
-              poemListRendering.copy(selection = NO_POEM_SELECTED)
-            )
+          overviewRendering = BackStackScreen(
+            poemListRendering.copy(selection = NO_POEM_SELECTED)
           )
+        )
       }
       is ComplexCall -> {
         context.runningWorker(
@@ -243,8 +246,10 @@ class PerformancePoemsBrowserWorkflow(
           key = "",
         ) { clearSelection }
       }
+      else -> {
+        throw IllegalStateException("$renderState state is impossible.")
+      }
     }
-
   }
 
   override fun snapshotState(state: State): Snapshot? = null

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoemsBrowserWorkflow.kt

@@ -84,8 +84,9 @@ class PerformancePoetryActivity : AppCompatActivity() {
     }
 
     val isFrameTimeout = intent.getBooleanExtra(EXTRA_RUNTIME_FRAME_TIMEOUT, false)
+    val useCompose = intent.getBooleanExtra(EXTRA_RUNTIME_COMPOSE_RUNTIME, false)
     val runtimeConfig = if (isFrameTimeout) {
-      FrameTimeout(useComposeInRuntime = true)
+      FrameTimeout(useComposeInRuntime = useCompose)
     } else {
       RenderPerAction
     }
@@ -247,6 +248,8 @@ class PerformancePoetryActivity : AppCompatActivity() {
     const val EXTRA_PERF_CONFIG_DELAY = "complex.poetry.performance.config.delay.length"
     const val EXTRA_RUNTIME_FRAME_TIMEOUT =
       "complex.poetry.performance.config.runtime.frametimeout"
+    const val EXTRA_RUNTIME_COMPOSE_RUNTIME =
+      "complex.poetry.performance.config.runtime.compose"
 
     const val SELECT_ON_TIMEOUT_LOG_NAME =
       "kotlinx.coroutines.selects.SelectBuilderImpl\$onTimeout\$\$inlined\$Runnable"

benchmarks/performance-poetry/complex-poetry/src/main/java/com/squareup/benchmarks/performance/complex/poetry/PerformancePoetryActivity.kt

@@ -48,6 +48,7 @@ fun UiDevice.resetToRootPoetryList() {
  */
 fun UiDevice.openRavenAndNavigate() {
   waitForIdle()
+  waitForIdle(5_000)
   waitForAndClick(RavenPoemSelector)
   waitForLoadingInterstitial()
   waitForAndClick(By.textStartsWith("Deep into that darkness peering"))