Skip to content

Pass Rendering if any Conflations changed state #1296

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.

Sign up for GitHub

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
Apr 23, 2025
Merged

Pass Rendering if any Conflations changed state #1296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,12 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
* If [runtimeConfig] contains [RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES] and
* we have not changed state, then return true to short circuit the render loop.
*/
fun shouldShortCircuitForUnchangedState(actionResult: ActionProcessingResult): Boolean {
fun shouldShortCircuitForUnchangedState(
actionResult: ActionProcessingResult,
conflationHasChangedState: Boolean = false
): Boolean {
return runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES) &&
actionResult is ActionApplied<*> && !actionResult.stateChanged
actionResult is ActionApplied<*> && !actionResult.stateChanged && !conflationHasChangedState
}

scope.launch {
Expand All @@ -190,7 +193,9 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
var nextRenderAndSnapshot: RenderingAndSnapshot<RenderingT> = runner.nextRendering()

if (runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) {
var conflationHasChangedState = false
conflate@ while (isActive && actionResult is ActionApplied<*> && actionResult.output == null) {
conflationHasChangedState = conflationHasChangedState || actionResult.stateChanged
// 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.
Expand All @@ -202,7 +207,11 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
if (actionResult == ActionsExhausted) break@conflate

// Skip rendering if we had unchanged state, keep draining actions.
if (shouldShortCircuitForUnchangedState(actionResult)) {
if (shouldShortCircuitForUnchangedState(
actionResult = actionResult,
conflationHasChangedState = conflationHasChangedState
)
) {
sendOutput(actionResult, onOutput)
continue@outer
}
Expand Down
88 changes: 85 additions & 3 deletions workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1393,7 +1393,6 @@ class RenderWorkflowInTest {
trigger.asWorker()
) {
action("") {
state = it
setOutput(it)
}
}
Expand All @@ -1403,10 +1402,9 @@ class RenderWorkflowInTest {
val workflow = Workflow.stateful<String, String, String>(
initialState = "unchanging state",
render = { renderState ->
renderChild(childWorkflow) { childOutput ->
renderChild(childWorkflow) { _ ->
action("childHandler") {
childHandlerActionExecuted++
state = childOutput
}
}
runningWorker(
Expand Down Expand Up @@ -1632,6 +1630,90 @@ class RenderWorkflowInTest {
}
}

@Test
fun for_conflate_and_render_only_when_state_changed_we_do_not_conflate_stacked_actions_into_one_rendering_if_previous_rendering_changed() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't get goose to summarize this paragraph for you? 😜

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>()

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
}
)
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())
assertTrue(childHandlerActionExecuted)
}
}
}

private class ExpectedException : RuntimeException()

private fun <T1, T2> cartesianProduct(
Expand Down
Loading