From da58522280b535de0cb048c3a57a9d0ee9cb8211 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Thu, 8 May 2025 11:28:52 -0400 Subject: [PATCH] CSR pass old rendering, do not render again if can skip --- workflow-runtime/api/workflow-runtime.api | 5 +- .../com/squareup/workflow1/RenderWorkflow.kt | 15 ++-- .../squareup/workflow1/WorkflowInterceptor.kt | 18 +++- .../workflow1/RenderWorkflowInTest.kt | 89 +++++++++++++++++++ 4 files changed, 119 insertions(+), 8 deletions(-) diff --git a/workflow-runtime/api/workflow-runtime.api b/workflow-runtime/api/workflow-runtime.api index b1dd44103..150f63976 100644 --- a/workflow-runtime/api/workflow-runtime.api +++ b/workflow-runtime/api/workflow-runtime.api @@ -88,7 +88,10 @@ public final class com/squareup/workflow1/WorkflowInterceptor$RenderContextInter } public final class com/squareup/workflow1/WorkflowInterceptor$RenderPassSkipped : com/squareup/workflow1/WorkflowInterceptor$RuntimeLoopOutcome { - public static final field INSTANCE Lcom/squareup/workflow1/WorkflowInterceptor$RenderPassSkipped; + public fun ()V + public fun (Z)V + public synthetic fun (ZILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun getEndOfTick ()Z } public final class com/squareup/workflow1/WorkflowInterceptor$RenderPassesComplete : com/squareup/workflow1/WorkflowInterceptor$RuntimeLoopOutcome { diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt index c0470fdac..78a25f097 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt @@ -170,12 +170,10 @@ public fun renderWorkflowIn( */ fun shouldShortCircuitForUnchangedState( actionResult: ActionProcessingResult, - conflationHasChangedState: Boolean = false ): Boolean { return runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES) && actionResult is ActionApplied<*> && - !actionResult.stateChanged && - !conflationHasChangedState + !actionResult.stateChanged } scope.launch { @@ -186,7 +184,7 @@ public fun renderWorkflowIn( var actionResult: ActionProcessingResult = runner.processAction() if (shouldShortCircuitForUnchangedState(actionResult)) { - chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped) + chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped()) sendOutput(actionResult, onOutput) continue@outer } @@ -211,10 +209,15 @@ public fun renderWorkflowIn( // Skip rendering if we had unchanged state, keep draining actions. if (shouldShortCircuitForUnchangedState( actionResult = actionResult, - conflationHasChangedState = conflationHasChangedState ) ) { - chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped) + if (conflationHasChangedState) { + chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped(endOfTick = false)) + // An earlier render changed state, so we need to pass that to the UI then we + // can skip this render. + break@conflate + } + chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped()) sendOutput(actionResult, onOutput) continue@outer } diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/WorkflowInterceptor.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/WorkflowInterceptor.kt index 8fc49d898..1b27b2807 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/WorkflowInterceptor.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/WorkflowInterceptor.kt @@ -157,7 +157,23 @@ public interface WorkflowInterceptor { ): Unit = Unit public sealed interface RuntimeLoopOutcome - public object RenderPassSkipped : RuntimeLoopOutcome + + /** + * @param endOfTick This is true if this skip was the end of the loop iteration (i.e. no update + * was passed to the UI. It is false otherwise. An example of when it can be false is if + * we have an action that causes a state change, but then we can process more while the + * `CONFLATE_STALE_RENDERINGS` optimization is enabled. We may skip rendering based on + * the subsequent action not changing state, but we will need to finish the loop and update + * the UI from the previous action that changed state. + */ + public class RenderPassSkipped( + public val endOfTick: Boolean = true + ) : RuntimeLoopOutcome + + /** + * @param renderingAndSnapshot This is the rendering and snapshot that was passed out of the + * Workflow runtime. + */ public class RenderPassesComplete( public val renderingAndSnapshot: RenderingAndSnapshot ) : RuntimeLoopOutcome diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt index 23eb8e165..504203d52 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt @@ -1887,6 +1887,95 @@ class RenderWorkflowInTest { } } + @Test + fun for_conflate_and_render_only_when_state_changed_we_do_not_render_again_if_only_previous_rendering_changed() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions + .filter { + it.first.contains(CONFLATE_STALE_RENDERINGS) && + it.first.contains(RENDER_ONLY_WHEN_STATE_CHANGES) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> + runTest(dispatcher) { + check(runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) + check(runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES)) + + var childHandlerActionExecuted = false + val trigger = MutableSharedFlow() + val emitted = mutableListOf() + var renderCount = 0 + + val childWorkflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it + setOutput(it) + } + } + renderState + } + ) + val workflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + renderChild(childWorkflow) { childOutput -> + action("childHandler") { + childHandlerActionExecuted = true + state = "$childOutput+update" + } + } + runningWorker( + trigger.asWorker() + ) { + action("") { + // no state change now! + } + } + renderState.also { + renderCount++ + } + } + ) + val props = MutableStateFlow(Unit) + val renderings = renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) { } + + val collectionJob = launch { + // Collect this unconfined so we can get all the renderings faster than actions can + // be processed. + renderings.collect { + emitted += it.rendering + } + } + advanceIfStandard(dispatcher) + + launch { + trigger.emit("changed state") + } + advanceIfStandard(dispatcher) + + collectionJob.cancel() + + // 2 renderings. + assertEquals(2, emitted.size) + assertEquals("changed state+update", emitted.last()) + // Only 2 times rendered, the initial + the update (not 3). + assertEquals(2, renderCount) + assertTrue(childHandlerActionExecuted) + } + } + } + private class ExpectedException : RuntimeException() private fun cartesianProduct(