Skip to content

CSR pass old rendering, do not render again if can skip #1306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion workflow-runtime/api/workflow-runtime.api
Original file line number Diff line number Diff line change
Expand Up @@ -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 <init> ()V
public fun <init> (Z)V
public synthetic fun <init> (ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun getEndOfTick ()Z
}

public final class com/squareup/workflow1/WorkflowInterceptor$RenderPassesComplete : com/squareup/workflow1/WorkflowInterceptor$RuntimeLoopOutcome {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,10 @@ public fun <PropsT, OutputT, RenderingT> 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 {
Expand All @@ -186,7 +184,7 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
var actionResult: ActionProcessingResult = runner.processAction()

if (shouldShortCircuitForUnchangedState(actionResult)) {
chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped)
chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped())
sendOutput(actionResult, onOutput)
continue@outer
}
Expand All @@ -211,10 +209,15 @@ public fun <PropsT, OutputT, RenderingT> 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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<R>(
public val renderingAndSnapshot: RenderingAndSnapshot<R>
) : RuntimeLoopOutcome
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>()
val emitted = mutableListOf<String>()
var renderCount = 0

val childWorkflow = Workflow.stateful<String, String, String>(
initialState = "unchanging state",
render = { renderState ->
runningWorker(
trigger.asWorker()
) {
action("") {
state = it
setOutput(it)
}
}
renderState
}
)
val workflow = Workflow.stateful<String, String, String>(
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 <T1, T2> cartesianProduct(
Expand Down
Loading