`DRAIN_EXCLUSIVE_ACTIONS` implementation #1269

steve-the-edwards · 2025-03-05T18:23:33Z

If there are multiple actions that are prepared to be applied that are in separate (exclusive) areas of the tree, then apply them all before rendering again.

Also ensure that we track if any of the chained actions have changed state. If they have, then pass the new rendering to the UI!

Update tests to test this behavior.

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

zach-klippenstein · 2025-04-17T19:03:48Z

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

+          // We start by yielding, because if we are on an Unconfined dispatcher, we want to give
+          // other signals (like Workers listening to the same result) a chance to get dispatched
+          // and queue their actions.
+          yield()


Passes the sniff test: This is a "tight" loop that doesn't necessarily suspend (I think sendOutput is the only suspending call?), so yielding in the middle seems reasonable even if we weren't using an unconfined dispatcher.

zach-klippenstein · 2025-04-17T19:05:51Z

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

@@ -186,11 +187,42 @@ 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 (runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) {
+        drain@ while (isActive &&


Do we still need this isActive check since yield should throw if the job is cancelled?

No, its technically not needed. I like it for explicitness sake /readability though. is there an impact of this that i'm not considering and we should remove it?

zach-klippenstein · 2025-04-17T19:06:36Z

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

      // Next Render Pass.
      var nextRenderAndSnapshot: RenderingAndSnapshot<RenderingT> = runner.nextRendering()

      if (runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) {
-        conflate@ while (isActive && actionResult is ActionApplied<*> && actionResult.output == null) {
+        conflate@ while (isActive &&


Same question about isActive.

zach-klippenstein · 2025-04-17T19:09:06Z

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.


zach-klippenstein · 2025-04-17T19:09:09Z

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

+  // Our state or that of one of our descendants changed.
+  private var subtreeStateDirty: Boolean = true
+
+  // Our state changed.


zach-klippenstein · 2025-04-17T19:20:38Z

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

-    with(selector) {
-      eventActionsChannel.onReceive { action ->
-        return@onReceive applyAction(action)
+    if (!skipChangedNodes || !selfStateDirty) {


Nit: Can either of these properties change between line 221 and here? If not, consider assignin to a local val to make it more clear that both these ifs use the same condition.

no they can't. and sure I can do that!

zach-klippenstein · 2025-04-17T19:22:27Z

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

+  private var subtreeStateDirty: Boolean = true
+
+  // Our state changed.
+  private var selfStateDirty: Boolean = true


Maybe i missed it in the code, but it looks like we never set this on its own (without also setting subtreeStateDirty)? Do we really need it?

Yes, lines 349-350 we set them using different conditions when the action is being applied.

// Our state changed. selfStateDirty = actionApplied.stateChanged // Our state changed or one of our children's state changed. subtreeStateDirty = aggregateActionApplied.stateChanged

that's key for drain exclusive because subtreeStateDirty is true always on a path all the way to the root no matter what changed in the tree. We use selfStateDirty to determine whether the node is exclusive to the previous action applied because otherwise there would never be any other exclusive nodes that we can process actions on.

steve-the-edwards · 2025-04-23T17:06:33Z

Update to add onUiUpdate() to the WorkflowInterceptor interface
Update to check for conflationHasChangedState across both DEA and CSR

steve-the-edwards requested review from zach-klippenstein and a team as code owners March 5, 2025 18:23

steve-the-edwards force-pushed the sedwards/partial-tree branch from 6384fa7 to 7d3a6fe Compare March 6, 2025 15:18

Base automatically changed from sedwards/partial-tree to main March 6, 2025 19:07

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch 2 times, most recently from d67e58c to 43bdda4 Compare March 6, 2025 19:22

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch 2 times, most recently from 63af06d to ee71c0b Compare April 15, 2025 21:09

steve-the-edwards commented Apr 16, 2025

View reviewed changes

workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt Outdated Show resolved Hide resolved

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from ee71c0b to f4a3d4e Compare April 16, 2025 20:38

steve-the-edwards changed the title ~~WIP: DRAIN_EXCLUSIVE_ACTIONS exploration~~ DRAIN_EXCLUSIVE_ACTIONS implementation Apr 16, 2025

steve-the-edwards changed the base branch from main to sedwards/conflate-timeout-1 April 16, 2025 20:40

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from f4a3d4e to a7dda2e Compare April 16, 2025 20:43

zach-klippenstein approved these changes Apr 17, 2025

View reviewed changes

steve-the-edwards force-pushed the sedwards/conflate-timeout-1 branch from dbd02dc to 934826b Compare April 17, 2025 20:50

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from a7dda2e to a877247 Compare April 17, 2025 21:16

Base automatically changed from sedwards/conflate-timeout-1 to main April 22, 2025 14:32

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch 2 times, most recently from 532d1d2 to abd8f88 Compare April 23, 2025 13:38

steve-the-edwards requested a review from zach-klippenstein April 23, 2025 14:23

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from abd8f88 to 6e2a691 Compare April 23, 2025 20:03

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from 6e2a691 to d6aa364 Compare May 7, 2025 20:21

DRAIN_EXCLUSIVE_ACTIONS implementation

11ffd77

steve-the-edwards force-pushed the sedwards/drain-exclusive-actions branch from d6aa364 to 11ffd77 Compare May 9, 2025 15:47

steve-the-edwards closed this May 9, 2025

steve-the-edwards reopened this May 9, 2025

steve-the-edwards mentioned this pull request May 13, 2025

With the Main.immediate dispatcher process side effects in time to drain multiple actions without yield(). #1311

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`DRAIN_EXCLUSIVE_ACTIONS` implementation #1269

`DRAIN_EXCLUSIVE_ACTIONS` implementation #1269

steve-the-edwards commented Mar 5, 2025 •

edited

Loading

Uh oh!

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

steve-the-edwards Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

steve-the-edwards Apr 17, 2025

Uh oh!

zach-klippenstein Apr 17, 2025

Uh oh!

steve-the-edwards Apr 17, 2025

Uh oh!

steve-the-edwards Apr 17, 2025

Uh oh!

steve-the-edwards commented Apr 23, 2025 •

edited

Loading

Uh oh!

Uh oh!

DRAIN_EXCLUSIVE_ACTIONS implementation #1269

Are you sure you want to change the base?

DRAIN_EXCLUSIVE_ACTIONS implementation #1269

Conversation

steve-the-edwards commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steve-the-edwards commented Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

`DRAIN_EXCLUSIVE_ACTIONS` implementation #1269

`DRAIN_EXCLUSIVE_ACTIONS` implementation #1269

steve-the-edwards commented Mar 5, 2025 •

edited

Loading

steve-the-edwards commented Apr 23, 2025 •

edited

Loading