Merge pull request #911 from square/sedwards/cache-12

Cache WorkflowIdentifier

.github/workflows/kotlin.yml

@@ -149,14 +149,6 @@ jobs :
             iosX64Test --stacktrace
           cache-read-only: false
 
-      ## iOS Specific Tests w/ strict memory model (for KMP ios actuals in core and runtime).
-      - uses: gradle/gradle-build-action@v2
-        name : Check with Gradle
-        with :
-          arguments : |
-            iosX64Test -Pkotlin.native.binary.memoryModel=strict --stacktrace
-          cache-read-only: false
-
       # Report as Github Pull Request Check.
       - name : Publish Test Report
         uses : mikepenz/action-junit-report@v3

samples/compose-samples/src/main/java/com/squareup/sample/compose/hellocomposeworkflow/ComposeWorkflow.kt

@@ -1,10 +1,12 @@
 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.ui.ViewEnvironment
 import com.squareup.workflow1.ui.WorkflowUiExperimentalApi
 import com.squareup.workflow1.ui.compose.ComposeScreen
@@ -29,7 +31,7 @@ import com.squareup.workflow1.ui.compose.ComposeScreen
  */
 @WorkflowUiExperimentalApi
 abstract class ComposeWorkflow<in PropsT, out OutputT : Any> :
-  Workflow<PropsT, OutputT, ComposeScreen> {
+  Workflow<PropsT, OutputT, ComposeScreen>, IdCacheable {
 
   /**
    * Renders [props] by emitting Compose UI. This function will be called to update the UI whenever
@@ -48,6 +50,8 @@ abstract class ComposeWorkflow<in PropsT, out OutputT : Any> :
 
   override fun asStatefulWorkflow(): StatefulWorkflow<PropsT, *, OutputT, ComposeScreen> =
     ComposeWorkflowImpl(this)
+
+  override var cachedIdentifier: WorkflowIdentifier? = null
 }
 
 /**

workflow-core/api/workflow-core.api

@@ -48,6 +48,11 @@ 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 fun setCachedIdentifier (Lcom/squareup/workflow1/WorkflowIdentifier;)V
+}
+
 public abstract interface class com/squareup/workflow1/ImpostorWorkflow {
 	public abstract fun describeRealIdentifier ()Ljava/lang/String;
 	public abstract fun getRealIdentifier ()Lcom/squareup/workflow1/WorkflowIdentifier;
@@ -115,12 +120,14 @@ 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 <init> ()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;
+	public fun setCachedIdentifier (Lcom/squareup/workflow1/WorkflowIdentifier;)V
 	public abstract fun snapshotState (Ljava/lang/Object;)Lcom/squareup/workflow1/Snapshot;
 }
 
@@ -141,10 +148,12 @@ 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 <init> ()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;
+	public fun setCachedIdentifier (Lcom/squareup/workflow1/WorkflowIdentifier;)V
 }
 
 public final class com/squareup/workflow1/StatelessWorkflow$RenderContext : com/squareup/workflow1/BaseRenderContext {
@@ -284,6 +293,7 @@ public final class com/squareup/workflow1/Workflows {
 	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 contraMap (Lcom/squareup/workflow1/Sink;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow1/Sink;
+	public static final fun getComputedIdentifier (Lcom/squareup/workflow1/Workflow;)Lcom/squareup/workflow1/WorkflowIdentifier;
 	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;
 	public static final fun renderChild (Lcom/squareup/workflow1/BaseRenderContext;Lcom/squareup/workflow1/Workflow;Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;

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.
+ *
+ * Your Workflow can just assign null to this value as the [identifier] extension will use it
+ * for caching.
+ */
+public interface IdCacheable {
+
+  public var cachedIdentifier: WorkflowIdentifier?
+}

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.computedIdentifier].
    *
    * For workflows that implement operators, this should be the identifier of the upstream
    * [Workflow] that this workflow wraps.

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<PropsT, OutputT, RenderingT> {
+  > : Workflow<PropsT, OutputT, RenderingT>, IdCacheable {
 
   public inner class RenderContext internal constructor(
     baseContext: BaseRenderContext<PropsT, StateT, OutputT>
@@ -129,6 +130,14 @@ 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 var cachedIdentifier: WorkflowIdentifier? = null
+
   /**
    * Called whenever the state changes to generate a new [Snapshot] of the state.
    *

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<in PropsT, out OutputT, out RenderingT> :
-  Workflow<PropsT, OutputT, RenderingT> {
+  Workflow<PropsT, OutputT, RenderingT>, IdCacheable {
 
   @Suppress("UNCHECKED_CAST")
   public inner class RenderContext internal constructor(
@@ -57,6 +58,14 @@ public abstract class StatelessWorkflow<in PropsT, out OutputT, out RenderingT>
     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 var cachedIdentifier: WorkflowIdentifier? = null
+
   /**
    * Satisfies the [Workflow] interface by wrapping `this` in a [StatefulWorkflow] with `Unit`
    * state.

workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifier.kt

@@ -113,6 +113,13 @@ public class WorkflowIdentifier internal constructor(
     return result
   }
 
+  /**
+   * Used to detect when this [WorkflowIdentifier] is a deeply stubbed mock. Stubs are provided
+   * until a primitive, in this case the typeName. This lets us determine if we are a mock
+   * object, or a real one. Mea culpa.
+   */
+  internal val deepNameCheck = type.typeName
+
   public companion object {
     private const val NO_PROXY_IDENTIFIER_TAG = 0.toByte()
     private const val PROXY_IDENTIFIER_TAG = 1.toByte()
@@ -145,8 +152,40 @@ 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
+  get() {
+    return when (this) {
+      is IdCacheable -> {
+        // The following lines look more complex than they need to be. If we have not yet cached
+        // the identifier, we do. But we return the [computedIdentifier] value directly as in the
+        // case of tests which use mocks we want to ensure that we return what is on line 180 -
+        // "WorkflowIdentifier(Snapshottable(this::class))" as that depends solely on types.
+        // We do the 'senseless' comparison of .type.typeName here to detect the case where we
+        // we have mocks with deep stubs but the name itself (a String) is null.
+        // The reason this is so complicated is this caching has been added afterword via the
+        // [IdCacheable] interface so that the [Workflow] interface itself remains unchanged.
+        @Suppress("SENSELESS_COMPARISON")
+        if (cachedIdentifier == null || cachedIdentifier!!.deepNameCheck == null) {
+          return computedIdentifier.also {
+            cachedIdentifier = it
+          }
+        }
+        cachedIdentifier!!
+      }
+      else -> computedIdentifier
+    }
+  }
+
+/**
+ * The computed [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 val Workflow<*, *, *>.computedIdentifier: WorkflowIdentifier
   get() {
     val maybeImpostor = this as? ImpostorWorkflow
     return WorkflowIdentifier(