Skip to content

Commit

Permalink
Add Conflate Stale Renderings Runtime
Browse files Browse the repository at this point in the history
Check for empty channels and synchronously return if they are empty.
  • Loading branch information
steve-the-edwards committed Aug 24, 2022
1 parent 0ebc57d commit 14acb3f
Show file tree
Hide file tree
Showing 14 changed files with 215 additions and 32 deletions.
76 changes: 76 additions & 0 deletions .github/workflows/kotlin.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,34 @@ jobs :
with :
report_paths : '**/build/test-results/test/TEST-*.xml'

jvm-conflate-runtime-test :
name : Conflate Stale Renderings Runtime JVM Tests
runs-on : ubuntu-latest
timeout-minutes : 20
steps :
- uses : actions/checkout@v3
- uses : gradle/wrapper-validation-action@v1
- name : set up JDK 11
uses : actions/setup-java@v3
with :
distribution : 'zulu'
java-version : 11

## Actual task
- uses : gradle/gradle-build-action@v2
name : Check with Gradle
with :
arguments : |
jvmTest --stacktrace --continue -Pworkflow.runtime=conflate
cache-read-only : false

# Report as Github Pull Request Check.
- name : Publish Test Report
uses : mikepenz/action-junit-report@v3
if : always() # always run even if the previous step fails
with :
report_paths : '**/build/test-results/test/TEST-*.xml'

ios-tests :
name : iOS Tests
runs-on : macos-latest
Expand Down Expand Up @@ -228,6 +256,54 @@ jobs :
name : instrumentation-test-results-${{ matrix.api-level }}
path : ./**/build/reports/androidTests/connected/**

conflate-renderings-instrumentation-tests :
name : Conflate Stale Renderings Instrumentation tests
runs-on : macos-latest
timeout-minutes : 45
strategy :
# Allow tests to continue on other devices if they fail on one device.
fail-fast : false
matrix :
api-level :
- 29
# Unclear that older versions actually honor command to disable animation.
# Newer versions are reputed to be too slow: https://github.com/ReactiveCircus/android-emulator-runner/issues/222
steps :
- uses : actions/checkout@v3
- name : set up JDK 11
uses : actions/setup-java@v3
with :
distribution : 'zulu'
java-version : 11

## Build before running tests, using cache.
- uses: gradle/gradle-build-action@v2
name : Build instrumented tests
with :
# Unfortunately I don't think we can key this cache based on our project property so
# we clean and rebuild.
arguments : |
clean assembleDebugAndroidTest --stacktrace -Pworkflow.runtime=conflate
cache-read-only: false

## Actual task
- name : Instrumentation Tests
uses : reactivecircus/android-emulator-runner@v2
with :
# @ychescale9 suspects Galaxy Nexus is the fastest one
profile : Galaxy Nexus
api-level : ${{ matrix.api-level }}
arch : x86_64
# Skip the benchmarks as this is running on emulators
script : ./gradlew connectedCheck -x :benchmarks:dungeon-benchmark:connectedCheck -x :benchmarks:performance-poetry:complex-benchmark:connectedCheck -x :benchmarks:performance-poetry:complex-poetry:connectedCheck --stacktrace -Pworkflow.runtime=conflate

- name : Upload results
if : ${{ always() }}
uses : actions/upload-artifact@v3
with :
name : instrumentation-test-results-${{ matrix.api-level }}
path : ./**/build/reports/androidTests/connected/**

upload-to-mobiledev :
name : mobile.dev | Build & Upload
runs-on : ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ android.useAndroidX=true
systemProp.org.gradle.internal.publish.checksums.insecure=true

GROUP=com.squareup.workflow1
VERSION_NAME=1.8.0-beta10-SNAPSHOT
VERSION_NAME=1.8.0-beta10-conflate-SNAPSHOT

POM_DESCRIPTION=Square Workflow

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.squareup.workflow1.config

import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.RuntimeConfig.RenderPerAction
import com.squareup.workflow1.WorkflowExperimentalRuntime

Expand All @@ -17,12 +18,15 @@ public class AndroidRuntimeConfigTools {
* this function, and then pass that to the call to [renderWorkflowIn] as the [RuntimeConfig].
*
* Current options are:
* "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
* to the UI layer.
* "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
* be specified as it is the current default and is assumed by this utility.
*/
@WorkflowExperimentalRuntime
public fun getAppWorkflowRuntimeConfig(): RuntimeConfig {
return when (BuildConfig.WORKFLOW_RUNTIME) {
"conflate" -> ConflateStaleRenderings
else -> RenderPerAction
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.squareup.workflow1.config

import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.RuntimeConfig.RenderPerAction
import com.squareup.workflow1.WorkflowExperimentalRuntime

Expand All @@ -16,16 +17,18 @@ public class JvmTestRuntimeConfigTools {
* [RuntimeConfig].
*
* Current options are:
* "conflate" : [ConflateStaleRenderings] Process all queued actions before passing rendering
* to the UI layer.
* "baseline" : [RenderPerAction] Original Workflow Runtime. Note that this doesn't need to
* be specified as it is the current default and is assumed by this utility.
*/
@OptIn(WorkflowExperimentalRuntime::class)
public fun getTestRuntimeConfig(): RuntimeConfig {
return RenderPerAction
// val runtimeConfig = System.getProperty("workflow.runtime", "baseline")
// return when (runtimeConfig) {
// else -> RenderPerAction
// }
val runtimeConfig = System.getProperty("workflow.runtime", "baseline")
return when (runtimeConfig) {
"conflate" -> ConflateStaleRenderings
else -> RenderPerAction
}
}
}
}
4 changes: 4 additions & 0 deletions workflow-core/api/workflow-core.api
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
public abstract interface class com/squareup/workflow1/ActionProcessingResult {
}

public final class com/squareup/workflow1/ActionsExhausted : com/squareup/workflow1/ActionProcessingResult {
public static final field INSTANCE Lcom/squareup/workflow1/ActionsExhausted;
}

public abstract interface class com/squareup/workflow1/BaseRenderContext {
public abstract fun eventHandler (Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function10;)Lkotlin/jvm/functions/Function9;
public abstract fun eventHandler (Lkotlin/jvm/functions/Function0;Lkotlin/jvm/functions/Function11;)Lkotlin/jvm/functions/Function10;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ public sealed interface ActionProcessingResult

public object PropsUpdated : ActionProcessingResult

public object ActionsExhausted : ActionProcessingResult

/** Wrapper around a potentially-nullable [OutputT] value. */
public class WorkflowOutput<out OutputT>(public val value: OutputT) : ActionProcessingResult {
override fun toString(): String = "WorkflowOutput($value)"
Expand Down
7 changes: 6 additions & 1 deletion workflow-runtime/api/workflow-runtime.api
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ public final class com/squareup/workflow1/RuntimeConfig$Companion {
public final fun getDEFAULT_CONFIG ()Lcom/squareup/workflow1/RuntimeConfig;
}

public final class com/squareup/workflow1/RuntimeConfig$ConflateStaleRenderings : com/squareup/workflow1/RuntimeConfig {
public static final field INSTANCE Lcom/squareup/workflow1/RuntimeConfig$ConflateStaleRenderings;
}

public final class com/squareup/workflow1/RuntimeConfig$RenderPerAction : com/squareup/workflow1/RuntimeConfig {
public static final field INSTANCE Lcom/squareup/workflow1/RuntimeConfig$RenderPerAction;
}
Expand Down Expand Up @@ -272,6 +276,7 @@ public final class com/squareup/workflow1/internal/WorkflowRunner {
public final fun cancelRuntime (Ljava/util/concurrent/CancellationException;)V
public static synthetic fun cancelRuntime$default (Lcom/squareup/workflow1/internal/WorkflowRunner;Ljava/util/concurrent/CancellationException;ILjava/lang/Object;)V
public final fun nextRendering ()Lcom/squareup/workflow1/RenderingAndSnapshot;
public final fun processAction (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
public final fun processAction (ZLkotlin/coroutines/Continuation;)Ljava/lang/Object;
public static synthetic fun processAction$default (Lcom/squareup/workflow1/internal/WorkflowRunner;ZLkotlin/coroutines/Continuation;ILjava/lang/Object;)Ljava/lang/Object;
}

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.squareup.workflow1

import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.internal.WorkflowRunner
import com.squareup.workflow1.internal.chained
import kotlinx.coroutines.CancellationException
Expand Down Expand Up @@ -101,7 +102,7 @@ import kotlinx.coroutines.launch
* A [StateFlow] of [RenderingAndSnapshot]s that will emit any time the root workflow creates a new
* rendering.
*/
@OptIn(ExperimentalCoroutinesApi::class)
@OptIn(ExperimentalCoroutinesApi::class, WorkflowExperimentalRuntime::class)
public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
workflow: Workflow<PropsT, OutputT, RenderingT>,
scope: CoroutineScope,
Expand Down Expand Up @@ -133,21 +134,60 @@ public fun <PropsT, OutputT, RenderingT> renderWorkflowIn(
}
)

suspend fun <PropsT, OutputT, RenderingT> renderAndEmitOutput(
runner: WorkflowRunner<PropsT, OutputT, RenderingT>,
actionResult: ActionProcessingResult?,
onOutput: suspend (OutputT) -> Unit
): RenderingAndSnapshot<RenderingT> {
// After receiving an output from the actions that were processed,
// the next render pass must be done before emitting that output,
// so that the workflow states appear consistent to observers of the outputs and renderings.
val nextRenderAndSnapshot = runner.nextRendering()

when (actionResult) {
is WorkflowOutput<*> -> {
@Suppress("UNCHECKED_CAST")
val output = actionResult as? WorkflowOutput<OutputT>
output?.let { onOutput(it.value) }
}
else -> {} // no -op
}

return nextRenderAndSnapshot
}

scope.launch {
while (isActive) {
// It might look weird to start by consuming the output before getting the rendering below,
lateinit var nextRenderAndSnapshot: RenderingAndSnapshot<RenderingT>
// 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.
val output: WorkflowOutput<OutputT>? = runner.processAction()
var actionResult: ActionProcessingResult? = runner.processAction()

// After resuming from runner.nextOutput() our coroutine could now be cancelled, check so we
// don't surprise anyone with an unexpected rendering pass. Show's over, go home.
// After resuming from runner.processAction() our coroutine could now be cancelled, check so
// we don't surprise anyone with an unexpected rendering pass. Show's over, go home.
if (!isActive) return@launch

// After receiving an output, the next render pass must be done before emitting that output,
// so that the workflow states appear consistent to observers of the outputs and renderings.
renderingsAndSnapshots.value = runner.nextRendering()
output?.let { onOutput(it.value) }
nextRenderAndSnapshot = renderAndEmitOutput(runner, actionResult, onOutput)

if (runtimeConfig == ConflateStaleRenderings) {
while (actionResult != ActionsExhausted) {
// We have more actions we can process, so this rendering is stale.
actionResult = runner.processAction(waitForAnAction = false)

if (!isActive) return@launch

// If no actions processed, then no new rendering needed.
if (actionResult == ActionsExhausted) break

nextRenderAndSnapshot = renderAndEmitOutput(runner, actionResult, onOutput)
}
}

// Pass on to the UI.
renderingsAndSnapshots.value = nextRenderAndSnapshot
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ public sealed interface RuntimeConfig {
*/
public object RenderPerAction : RuntimeConfig

/**
* If we have more actions to process, do so before passing the rendering to the UI layer.
*/
@WorkflowExperimentalRuntime
public object ConflateStaleRenderings : RuntimeConfig

public companion object {
public val DEFAULT_CONFIG: RuntimeConfig = RenderPerAction
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,12 @@ internal class SubtreeManager<PropsT, StateT, OutputT>(
* Uses [selector] to invoke [WorkflowNode.tick] for every running child workflow this instance
* is managing.
*/
fun tickChildren(selector: SelectBuilder<ActionProcessingResult?>) {
fun tickChildren(selector: SelectBuilder<ActionProcessingResult?>): Boolean {
var empty = true
children.forEachActive { child ->
child.workflowNode.tick(selector)
empty = empty && child.workflowNode.tick(selector)
}
return empty
}

fun createChildSnapshots(): Map<WorkflowNodeId, TreeSnapshot> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.CoroutineStart.LAZY
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.cancel
import kotlinx.coroutines.channels.Channel
Expand Down Expand Up @@ -150,16 +151,20 @@ internal class WorkflowNode<PropsT, StateT, OutputT, RenderingT>(
*
* It is an error to call this method after calling [cancel].
*/
fun tick(selector: SelectBuilder<ActionProcessingResult?>) {
@OptIn(ExperimentalCoroutinesApi::class)
fun tick(selector: SelectBuilder<ActionProcessingResult?>): Boolean {
// Listen for any child workflow updates.
subtreeManager.tickChildren(selector)
var empty = subtreeManager.tickChildren(selector)

empty = empty && eventActionsChannel.isEmpty

// Listen for any events.
with(selector) {
eventActionsChannel.onReceive { action ->
return@onReceive applyAction(action)
}
}
return empty
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package com.squareup.workflow1.internal

import com.squareup.workflow1.ActionProcessingResult
import com.squareup.workflow1.ActionsExhausted
import com.squareup.workflow1.PropsUpdated
import com.squareup.workflow1.RenderingAndSnapshot
import com.squareup.workflow1.RuntimeConfig
import com.squareup.workflow1.RuntimeConfig.ConflateStaleRenderings
import com.squareup.workflow1.TreeSnapshot
import com.squareup.workflow1.Workflow
import com.squareup.workflow1.WorkflowExperimentalRuntime
import com.squareup.workflow1.WorkflowInterceptor
import com.squareup.workflow1.WorkflowOutput
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -77,20 +78,16 @@ internal class WorkflowRunner<PropsT, OutputT, RenderingT>(
* coroutine and no others.
*/
@OptIn(WorkflowExperimentalRuntime::class)
suspend fun processAction(): WorkflowOutput<OutputT>? {
suspend fun processAction(waitForAnAction: Boolean = true): ActionProcessingResult? {
// First we block and wait until there is an action to process.
val processingResult: ActionProcessingResult? = select {
return select {
onPropsUpdated()
// Have the workflow tree build the select to wait for an event/output from Worker.
rootNode.tick(this)
}

@Suppress("UNCHECKED_CAST")
return when (processingResult) {
PropsUpdated -> null
else -> {
// Unchecked cast as this is the only other option for the sealed interface.
processingResult as WorkflowOutput<OutputT>?
val empty = rootNode.tick(this)
if (!waitForAnAction && runtimeConfig == ConflateStaleRenderings && empty) {
onTimeout(0) {
ActionsExhausted
}
}
}
}
Expand Down
Loading

0 comments on commit 14acb3f

Please sign in to comment.