WIP: DRAIN_EXCLUSIVE_ACTIONS implementatioNS implementatioNS implementatioNS implementatioNS implementatioNS implementatioNS implementatioNS implementatioNS implementation

Author: steve-the-edwards

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt

@@ -170,7 +170,7 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
   }
 
   scope.launch {
-    while (isActive) {
+    outer@ while (isActive) {
       // It might look weird to start by processing an action before getting the rendering below,
       // but remember the first render pass already occurred above, before this coroutine was even
       // launched.
@@ -185,19 +185,45 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
       // we don't surprise anyone with an unexpected rendering pass. Show's over, go home.
       if (!isActive) return@launch
 
+      var drainingActionResult = actionResult
+      // If DRAINING_EXCLUSIVE_ACTIONS
+      drain@ while (isActive &&
+        drainingActionResult is ActionApplied<*> &&
+        drainingActionResult.output == null) {
+        // We may have more mutually exclusive actions we can apply before a render pass.
+        drainingActionResult = runner.processAction(waitForAnAction = false, skipChangedNodes = true)
+
+        // If no mutually exclusive actions processed, then go ahead and do the render pass.
+        if (drainingActionResult == ActionsExhausted) break@drain
+
+        // Now save last result if its not ActionsExhausted
+        actionResult = drainingActionResult
+
+        // If no state changed, send any output and start outer loop again.
+        if (shouldShortCircuitForUnchangedState(drainingActionResult)) {
+          sendOutput(drainingActionResult, onOutput)
+          continue@outer
+        }
+      }
+
       // Next Render Pass.
       var nextRenderAndSnapshot: RenderingAndSnapshot<RenderingT> = runner.nextRendering()
 
       if (runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) {
-        while (isActive && actionResult is ActionApplied<*> && actionResult.output == null) {
+        conflate@ while (isActive &&
+          actionResult is ActionApplied<*> &&
+          actionResult.output == null) {
           // We may have more actions we can process, this rendering could be stale.
           actionResult = runner.processAction(waitForAnAction = false)
 
           // If no actions processed, then no new rendering needed. Pass on to UI.
-          if (actionResult == ActionsExhausted) break
+          if (actionResult == ActionsExhausted) break@conflate
 
           // Skip rendering if we had unchanged state, keep draining actions.
-          if (shouldShortCircuitForUnchangedState(actionResult)) continue
+          if (shouldShortCircuitForUnchangedState(actionResult)) {
+            sendOutput(drainingActionResult, onOutput)
+            continue@outer
+          }
 
           // Make sure the runtime has not been cancelled from runner.processAction()
           if (!isActive) return@launch

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt

@@ -150,11 +150,14 @@ internal class SubtreeManager<PropsT, StateT, OutputT>(
    *
    * @return [Boolean] whether or not the children action queues are empty.
    */
-  fun onNextChildAction(selector: SelectBuilder<ActionProcessingResult>): Boolean {
+  fun onNextChildAction(
+    selector: SelectBuilder<ActionProcessingResult>,
+    skipChangedNodes: Boolean = false
+  ): Boolean {
     var empty = true
     children.forEachActive { child ->
       // Do this separately so the compiler doesn't avoid it if empty is already false.
-      val childEmpty = child.workflowNode.onNextAction(selector)
+      val childEmpty = child.workflowNode.onNextAction(selector, skipChangedNodes)
       empty = childEmpty && empty
     }
     return empty

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt

@@ -96,7 +96,12 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
   private val eventActionsChannel =
     Channel<WorkflowAction<PropsT, StateT, OutputT>>(capacity = UNLIMITED)
   private var state: StateT
-  private var subtreeStateDidChange: Boolean = true
+
+  // Our state or that of one of our descendants changed.
+  private var subtreeStateDirty: Boolean = true
+
+  // Our state changed.
+  private var selfStateDirty: Boolean = true
 
   private val baseRenderContext = RealRenderContext(
     renderer = subtreeManager,
@@ -209,16 +214,27 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
    *    time of suspending.
    */
   @OptIn(ExperimentalCoroutinesApi::class, DelicateCoroutinesApi::class)
-  fun onNextAction(selector: SelectBuilder<ActionProcessingResult>): Boolean {
-    // Listen for any child workflow updates.
-    var empty = subtreeManager.onNextChildAction(selector)
+  fun onNextAction(
+    selector: SelectBuilder<ActionProcessingResult>,
+    skipChangedNodes: Boolean = false
+  ): Boolean {
+    var empty = if (!skipChangedNodes || !selfStateDirty) {
+      // Listen for any child workflow events.
+      subtreeManager.onNextChildAction(selector, skipChangedNodes)
+    } else {
+      // Our state changed and we are skipping changed nodes, so our actions are empty from
+      // this node down.
+      true
+    }
 
     empty = empty && (eventActionsChannel.isEmpty || eventActionsChannel.isClosedForReceive)
 
-    // Listen for any events.
-    with(selector) {
-      eventActionsChannel.onReceive { action ->
-        return@onReceive applyAction(action)
+    if (!skipChangedNodes || !selfStateDirty) {
+      // Listen for any events.
+      with(selector) {
+        eventActionsChannel.onReceive { action ->
+          return@onReceive applyAction(action)
+        }
       }
     }
     return empty
@@ -264,7 +280,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
 
     if (!runtimeConfig.contains(PARTIAL_TREE_RENDERING) ||
       !lastRendering.isInitialized ||
-      subtreeStateDidChange
+      subtreeStateDirty
     ) {
       // If we haven't already updated the cached instance, better do it now!
       maybeUpdateCachedWorkflowInstance(workflow)
@@ -284,7 +300,8 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
       }
       // After we have rendered this subtree, we need another action in order for us to be
       // considered dirty again.
-      subtreeStateDidChange = false
+      subtreeStateDirty = false
+      selfStateDirty = false
     }
 
     return lastRendering.getOrThrow()
@@ -293,7 +310,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
   /**
    * Update props if they have changed. If that happens, then check to see if we need
    * to update the cached workflow instance, then call [StatefulWorkflow.onPropsChanged] and
-   * update the state from that. We consider any change to props as [subtreeStateDidChange] because
+   * update the state from that. We consider any change to props as dirty because
    * the props themselves are used in [StatefulWorkflow.render] (they are the 'external' part of
    * the state) so we must re-render.
    */
@@ -305,7 +322,8 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
       maybeUpdateCachedWorkflowInstance(workflow)
       val newState = interceptedWorkflowInstance.onPropsChanged(lastProps, newProps, state)
       state = newState
-      subtreeStateDidChange = true
+      subtreeStateDirty = true
+      selfStateDirty = true
     }
     lastProps = newProps
   }
@@ -327,8 +345,10 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
       // Changing state is sticky, we pass it up if it ever changed.
       stateChanged = actionApplied.stateChanged || (childResult?.stateChanged ?: false)
     )
+    // Our state changed.
+    selfStateDirty = actionApplied.stateChanged
     // Our state changed or one of our children's state changed.
-    subtreeStateDidChange = aggregateActionApplied.stateChanged
+    subtreeStateDirty = aggregateActionApplied.stateChanged
     return if (actionApplied.output != null ||
       runtimeConfig.contains(PARTIAL_TREE_RENDERING)
     ) {
@@ -341,7 +361,7 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
       //
       // However, the root and the path down to the changed nodes must always
       // re-render now, so this is the implementation detail of how we get
-      // subtreeStateDidChange = true on that entire path to the root.
+      // subtreeStateDirty = true on that entire path to the root.
       emitAppliedActionToParent(aggregateActionApplied)
     } else {
       aggregateActionApplied

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt

@@ -5,7 +5,6 @@ import com.squareup.workflow1.ActionsExhausted
 import com.squareup.workflow1.PropsUpdated
 import com.squareup.workflow1.RenderingAndSnapshot
 import com.squareup.workflow1.RuntimeConfig
-import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS
 import com.squareup.workflow1.TreeSnapshot
 import com.squareup.workflow1.Workflow
 import com.squareup.workflow1.WorkflowExperimentalRuntime
@@ -83,14 +82,16 @@ internal class WorkflowRunner<PropsT, OutputT, RenderingT>(
    * and resume (breaking ties with order of declaration). Guarantees only continuing on the winning
    * coroutine and no others.
    */
-  @OptIn(WorkflowExperimentalRuntime::class)
-  suspend fun processAction(waitForAnAction: Boolean = true): ActionProcessingResult {
+  suspend fun processAction(
+    waitForAnAction: Boolean = true,
+    skipChangedNodes: Boolean = false
+  ): ActionProcessingResult {
     // If waitForAction is true we block and wait until there is an action to process.
     return select {
       onPropsUpdated()
       // Have the workflow tree build the select to wait for an event/output from Worker.
-      val empty = rootNode.onNextAction(this)
-      if (!waitForAnAction && runtimeConfig.contains(CONFLATE_STALE_RENDERINGS) && empty) {
+      val empty = rootNode.onNextAction(this, skipChangedNodes)
+      if (!waitForAnAction && empty) {
         // With CONFLATE_STALE_RENDERINGS if there are no queued actions and we are not
         // waiting for one, then return ActionsExhausted and pass the rendering on.
         onTimeout(timeMillis = 0) {

workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt

@@ -128,7 +128,6 @@ class WorkflowsLifecycleTests {
    * This test ensconces the currently failing behavior of side effects. We are not currently
    * fixing this but rather working around it with [SessionWorkflow].
    */
-  @Ignore
   @Test fun sideEffectsStartAndStoppedWhenHandledSynchronously() {
     runtimeTestRunner.runParametrizedTest(
       paramSource = runtimeOptions,
@@ -205,6 +204,7 @@ class WorkflowsLifecycleTests {
    *
    * This tests show the working behavior when using a [SessionWorkflow] to track the lifetime.
    */
+  @Ignore
   @Test fun childSessionWorkflowStartAndStoppedWhenHandledSynchronously() {
     runtimeTestRunner.runParametrizedTest(
       paramSource = runtimeOptions,