From ab2a993b3b1fad9a504cb918f9ee05e39f2d7731 Mon Sep 17 00:00:00 2001 From: Stephen Edwards Date: Fri, 9 Dec 2022 16:06:23 -0800 Subject: [PATCH] Cache WorkflowIdentifier --- gradle.properties | 2 +- .../hellocomposeworkflow/ComposeWorkflow.kt | 12 +++++++- workflow-core/api/workflow-core.api | 11 +++++-- .../com/squareup/workflow1/IdCacheable.kt | 14 +++++++++ .../squareup/workflow1/ImpostorWorkflow.kt | 2 +- .../squareup/workflow1/StatefulWorkflow.kt | 15 +++++++++- .../squareup/workflow1/StatelessWorkflow.kt | 15 +++++++++- .../kotlin/com/squareup/workflow1/Workflow.kt | 2 +- .../squareup/workflow1/WorkflowIdentifier.kt | 29 ++++++++++++++----- .../workflow1/WorkflowInterceptorTest.kt | 3 +- .../workflow1/testing/RealRenderTesterTest.kt | 11 +++---- 11 files changed, 95 insertions(+), 21 deletions(-) create mode 100644 workflow-core/src/commonMain/kotlin/com/squareup/workflow1/IdCacheable.kt diff --git a/gradle.properties b/gradle.properties index 001476f86..4c7e9ee4f 100644 --- a/gradle.properties +++ b/gradle.properties @@ -8,7 +8,7 @@ android.useAndroidX=true systemProp.org.gradle.internal.publish.checksums.insecure=true GROUP=com.squareup.workflow1 -VERSION_NAME=1.8.0-beta13-SNAPSHOT +VERSION_NAME=1.8.0-beta13-cache2-SNAPSHOT POM_DESCRIPTION=Square Workflow diff --git a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/ComposeWorkflow.kt b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/ComposeWorkflow.kt index da39ae62f..f556ad84a 100644 --- a/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/ComposeWorkflow.kt +++ b/samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/ComposeWorkflow.kt @@ -1,13 +1,17 @@ package com.squareup.sample.compose.hellocomposeworkflow import androidx.compose.runtime.Composable +import com.squareup.workflow1.IdCacheable import com.squareup.workflow1.RenderContext import com.squareup.workflow1.Sink import com.squareup.workflow1.StatefulWorkflow import com.squareup.workflow1.Workflow +import com.squareup.workflow1.WorkflowIdentifier +import com.squareup.workflow1.computeIdentifier import com.squareup.workflow1.ui.ViewEnvironment import com.squareup.workflow1.ui.WorkflowUiExperimentalApi import com.squareup.workflow1.ui.compose.ComposeScreen +import kotlin.LazyThreadSafetyMode.NONE /** * A stateless [Workflow] that [renders][RenderingContent] itself as a [Composable] function. @@ -29,7 +33,7 @@ import com.squareup.workflow1.ui.compose.ComposeScreen */ @WorkflowUiExperimentalApi abstract class ComposeWorkflow : - Workflow { + Workflow, IdCacheable { /** * Renders [props] by emitting Compose UI. This function will be called to update the UI whenever @@ -48,6 +52,12 @@ abstract class ComposeWorkflow : override fun asStatefulWorkflow(): StatefulWorkflow = ComposeWorkflowImpl(this) + + override val cachedIdentifier: WorkflowIdentifier by lazy( + mode = NONE + ) { + computeIdentifier() + } } /** diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api index 5a896de3e..e609da187 100644 --- a/workflow-core/api/workflow-core.api +++ b/workflow-core/api/workflow-core.api @@ -48,6 +48,10 @@ public final class com/squareup/workflow1/BaseRenderContext$DefaultImpls { public static synthetic fun renderChild$default (Lcom/squareup/workflow1/BaseRenderContext;Lcom/squareup/workflow1/Workflow;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object; } +public abstract interface class com/squareup/workflow1/IdCacheable { + public abstract fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier; +} + public abstract interface class com/squareup/workflow1/ImpostorWorkflow { public abstract fun describeRealIdentifier ()Ljava/lang/String; public abstract fun getRealIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier; @@ -115,9 +119,10 @@ public final class com/squareup/workflow1/Snapshots { public static final fun writeUtf8WithLength (Lokio/BufferedSink;Ljava/lang/String;)Lokio/BufferedSink; } -public abstract class com/squareup/workflow1/StatefulWorkflow : com/squareup/workflow1/Workflow { +public abstract class com/squareup/workflow1/StatefulWorkflow : com/squareup/workflow1/IdCacheable, com/squareup/workflow1/Workflow { public fun ()V public final fun asStatefulWorkflow ()Lcom/squareup/workflow1/StatefulWorkflow; + public fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier; public abstract fun initialState (Ljava/lang/Object;Lcom/squareup/workflow1/Snapshot;)Ljava/lang/Object; public fun onPropsChanged (Ljava/lang/Object;Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object; public abstract fun render (Ljava/lang/Object;Ljava/lang/Object;Lcom/squareup/workflow1/StatefulWorkflow$RenderContext;)Ljava/lang/Object; @@ -141,9 +146,10 @@ public final class com/squareup/workflow1/StatefulWorkflow$RenderContext : com/s public fun runningSideEffect (Ljava/lang/String;Lkotlin/jvm/functions/Function2;)V } -public abstract class com/squareup/workflow1/StatelessWorkflow : com/squareup/workflow1/Workflow { +public abstract class com/squareup/workflow1/StatelessWorkflow : com/squareup/workflow1/IdCacheable, com/squareup/workflow1/Workflow { public fun ()V public final fun asStatefulWorkflow ()Lcom/squareup/workflow1/StatefulWorkflow; + public fun getCachedIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier; public abstract fun render (Ljava/lang/Object;Lcom/squareup/workflow1/StatelessWorkflow$RenderContext;)Ljava/lang/Object; } @@ -283,6 +289,7 @@ public final class com/squareup/workflow1/Workflows { public static synthetic fun action$default (Lcom/squareup/workflow1/StatelessWorkflow;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/WorkflowAction; public static synthetic fun action$default (Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow1/WorkflowAction; public static final fun applyTo (Lcom/squareup/workflow1/WorkflowAction;Ljava/lang/Object;Ljava/lang/Object;)Lkotlin/Pair; + public static final fun computeIdentifier (Lcom/squareup/workflow1/Workflow;)Lcom/squareup/workflow1/WorkflowIdentifier; public static final fun contraMap (Lcom/squareup/workflow1/Sink;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/Sink; public static final fun getIdentifier (Lcom/squareup/workflow1/Workflow;)Lcom/squareup/workflow1/WorkflowIdentifier; public static final fun mapRendering (Lcom/squareup/workflow1/Workflow;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/Workflow; diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/IdCacheable.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/IdCacheable.kt new file mode 100644 index 000000000..186d6e7ad --- /dev/null +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/IdCacheable.kt @@ -0,0 +1,14 @@ +package com.squareup.workflow1 + +/** + * If your Workflow caches its [WorkflowIdentifier] (to avoid frequent lookups) then implement + * this interface. Note that [StatefulWorkflow] and [StatelessWorkflow] already implement this, + * so you only need to do so if you do not extend one of those classes. + * + * If your Workflow is an [ImpostorWorkflow] use the lazy delegate pattern that [StatefulWorkflow] + * and [StatelessWorkflow] do in order to initialize everything in the proper order. + */ +public interface IdCacheable { + + public val cachedIdentifier: WorkflowIdentifier +} diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/ImpostorWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/ImpostorWorkflow.kt index 4e75c8f7d..c11abaf4d 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/ImpostorWorkflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/ImpostorWorkflow.kt @@ -18,7 +18,7 @@ import kotlin.jvm.JvmName public interface ImpostorWorkflow { /** * The [WorkflowIdentifier] of another workflow to be combined with the identifier of this - * workflow, as obtained by [Workflow.identifier]. + * workflow, as obtained by [Workflow.computeIdentifier]. * * For workflows that implement operators, this should be the identifier of the upstream * [Workflow] that this workflow wraps. diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt index 031235ff4..70318909f 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt @@ -6,6 +6,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.StatefulWorkflow.RenderContext import com.squareup.workflow1.WorkflowAction.Companion.toString +import kotlin.LazyThreadSafetyMode.NONE import kotlin.jvm.JvmMultifileClass import kotlin.jvm.JvmName @@ -69,7 +70,7 @@ public abstract class StatefulWorkflow< StateT, out OutputT, out RenderingT - > : Workflow { + > : Workflow, IdCacheable { public inner class RenderContext internal constructor( baseContext: BaseRenderContext @@ -129,6 +130,18 @@ public abstract class StatefulWorkflow< context: RenderContext ): RenderingT + /** + * Use a lazy delegate so that any [ImpostorWorkflow.realIdentifier] will have been computed + * before this is initialized and cached. + * + * We use [LazyThreadSafetyMode.NONE] because access to these identifiers is thread-confined. + */ + override val cachedIdentifier: WorkflowIdentifier by lazy( + mode = NONE + ) { + computeIdentifier() + } + /** * Called whenever the state changes to generate a new [Snapshot] of the state. * diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt index f6545524e..8acb50215 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt @@ -3,6 +3,7 @@ package com.squareup.workflow1 +import kotlin.LazyThreadSafetyMode.NONE import kotlin.jvm.JvmMultifileClass import kotlin.jvm.JvmName @@ -24,7 +25,7 @@ import kotlin.jvm.JvmName * @see StatefulWorkflow */ public abstract class StatelessWorkflow : - Workflow { + Workflow, IdCacheable { @Suppress("UNCHECKED_CAST") public inner class RenderContext internal constructor( @@ -57,6 +58,18 @@ public abstract class StatelessWorkflow context: RenderContext ): RenderingT + /** + * Use a lazy delegate so that any [ImpostorWorkflow.realIdentifier] will have been computed + * before this is initialized and cached. + * + * We use [LazyThreadSafetyMode.NONE] because access to these identifiers is thread-confined. + */ + override val cachedIdentifier: WorkflowIdentifier by lazy( + mode = NONE + ) { + computeIdentifier() + } + /** * Satisfies the [Workflow] interface by wrapping `this` in a [StatefulWorkflow] with `Unit` * state. diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Workflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Workflow.kt index ce2aaf685..5ba821c12 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Workflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Workflow.kt @@ -120,7 +120,7 @@ Workflow.mapRendering( transform: (FromRenderingT) -> ToRenderingT ): Workflow = object : StatelessWorkflow(), ImpostorWorkflow { - override val realIdentifier: WorkflowIdentifier get() = this@mapRendering.identifier + override val realIdentifier: WorkflowIdentifier = this@mapRendering.identifier override fun render( renderProps: PropsT, diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifier.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifier.kt index a15583453..4e8ca03ed 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifier.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifier.kt @@ -145,17 +145,32 @@ public class WorkflowIdentifier internal constructor( /** * The [WorkflowIdentifier] that identifies this [Workflow]. + * + * This can carry some cost if the Workflow class does not implement [IdCacheable]. Note that + * [StatelessWorkflow] and [StatefulWorkflow] do implement the caching. */ -public val Workflow<*, *, *>.identifier: WorkflowIdentifier +public inline val Workflow<*, *, *>.identifier: WorkflowIdentifier get() { - val maybeImpostor = this as? ImpostorWorkflow - return WorkflowIdentifier( - type = Snapshottable(this::class), - proxiedIdentifier = maybeImpostor?.realIdentifier, - description = maybeImpostor?.let { it::describeRealIdentifier } - ) + return when (this) { + is IdCacheable -> cachedIdentifier + else -> computeIdentifier() + } } +/** + * Compute the [WorkflowIdentifier] for this Workflow. Any [IdCacheable] Workflow should call this + * and then store the value in the cachedIdentifier property so as to prevent + * the extra work needed to create the [WorkflowIdentifier] and look up the class name each time. + */ +public fun Workflow<*, *, *>.computeIdentifier(): WorkflowIdentifier { + val maybeImpostor = this as? ImpostorWorkflow + return WorkflowIdentifier( + type = Snapshottable(this::class), + proxiedIdentifier = maybeImpostor?.realIdentifier, + description = maybeImpostor?.let { it::describeRealIdentifier } + ) +} + /** * Creates a [WorkflowIdentifier] that is not capable of being snapshotted and will cause any * [ImpostorWorkflow] workflow identified by it to also not be snapshotted. diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowInterceptorTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowInterceptorTest.kt index 0ab204617..ae85599f8 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowInterceptorTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/WorkflowInterceptorTest.kt @@ -212,7 +212,8 @@ internal class WorkflowInterceptorTest { private val Workflow<*, *, *>.session: WorkflowSession get() = object : WorkflowSession { - override val identifier: WorkflowIdentifier = this@session.identifier + override val identifier: WorkflowIdentifier + get() = this@session.identifier override val renderKey: String = "" override val sessionId: Long = 0 override val parent: WorkflowSession? = null diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt index 43798bfb6..8dea7653f 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt @@ -14,6 +14,7 @@ import com.squareup.workflow1.WorkflowIdentifier import com.squareup.workflow1.WorkflowOutput import com.squareup.workflow1.action import com.squareup.workflow1.asWorker +import com.squareup.workflow1.computeIdentifier import com.squareup.workflow1.contraMap import com.squareup.workflow1.identifier import com.squareup.workflow1.renderChild @@ -1473,32 +1474,32 @@ internal class RealRenderTesterTest { @Test fun `realTypeMatchesExpectation() matches mockito mock of expected interface`() { val expected = TestWorkflowInterface::class.workflowIdentifier - val actual = mock().identifier + val actual = mock().computeIdentifier() assertTrue(actual.realTypeMatchesExpectation(expected)) } @Test fun `realTypeMatchesExpectation() matches mockito mock of expected abstract class`() { val expected = ExpectedWorkflowClass::class.workflowIdentifier - val actual = mock().identifier + val actual = mock().computeIdentifier() assertTrue(actual.realTypeMatchesExpectation(expected)) } @Test fun `realTypeMatchesExpectation() doesn't match mockito mock of unexpected interface`() { val expected = TestWorkflowInterface::class.workflowIdentifier - val actual = mock>().identifier + val actual = mock>().computeIdentifier() assertFalse(actual.realTypeMatchesExpectation(expected)) } @Test fun `realTypeMatchesExpectation() doesn't match mockito mock of unexpected abstract class`() { val expected = ExpectedWorkflowClass::class.workflowIdentifier - val actual = mock().identifier + val actual = mock().computeIdentifier() assertFalse(actual.realTypeMatchesExpectation(expected)) } @Test fun `realTypeMatchesExpectation() handles mockk mocks`() { val expected = TestWorkflowInterface::class.workflowIdentifier - val actual = mockk().identifier + val actual = mockk().computeIdentifier() assertTrue(actual.realTypeMatchesExpectation(expected)) }