Rewriting internals for better performance.

instacart · Jun 30, 2023 · 05b8423 · 05b8423
1 parent 0876c79
commit 05b8423
Show file tree

Hide file tree

Showing 12 changed files with 142 additions and 160 deletions.
diff --git a/formula/src/main/java/com/instacart/formula/FormulaRuntime.kt b/formula/src/main/java/com/instacart/formula/FormulaRuntime.kt
@@ -1,5 +1,6 @@
 package com.instacart.formula
 
+import com.instacart.formula.internal.Event
 import com.instacart.formula.internal.FormulaManager
 import com.instacart.formula.internal.FormulaManagerImpl
 import com.instacart.formula.internal.ManagerDelegate
@@ -29,6 +30,7 @@ class FormulaRuntime<Input : Any, Output : Any>(
 
     private var input: Input? = null
     private var key: Any? = null
+    private var isEvaluating: Boolean = false
     private var isExecuting: Boolean = false
 
     fun isKeyValid(input: Input): Boolean {
@@ -72,6 +74,10 @@ class FormulaRuntime<Input : Any, Output : Any>(
         run(shouldEvaluate = evaluate)
     }
 
+    override fun onPostponedTransition(event: Event) {
+        event.dispatch()
+    }
+
     private fun forceRun() = run(shouldEvaluate = true)
 
     /**
@@ -80,6 +86,8 @@ class FormulaRuntime<Input : Any, Output : Any>(
      * @param shouldEvaluate Determines if evaluation needs to be run.
      */
     private fun run(shouldEvaluate: Boolean) {
+        if (isEvaluating) return
+
         try {
             val freshRun = !isExecuting
             if (freshRun) {
@@ -89,14 +97,18 @@ class FormulaRuntime<Input : Any, Output : Any>(
             val manager = checkNotNull(manager)
             val currentInput = checkNotNull(input)
 
+            isEvaluating = true
             if (shouldEvaluate && !manager.terminated) {
                 evaluationPhase(manager, currentInput)
             }
+            isEvaluating = false
 
-            executionRequested = true
+            if (shouldEvaluate || effectQueue.isNotEmpty()) {
+                executionRequested = true
+            }
             if (isExecuting) return
 
-            executionPhase(manager)
+            effectPhase(manager)
 
             if (freshRun) {
                 inspector?.onRunFinished()
@@ -107,6 +119,8 @@ class FormulaRuntime<Input : Any, Output : Any>(
                 onOutput(checkNotNull(lastOutput))
             }
         } catch (e: Throwable) {
+            isEvaluating = false
+
             manager?.markAsTerminated()
             onError(e)
             manager?.performTerminationSideEffects()
@@ -138,26 +152,12 @@ class FormulaRuntime<Input : Any, Output : Any>(
     /**
      * Executes operations containing side-effects such as starting/terminating streams.
      */
-    private fun executionPhase(manager: FormulaManagerImpl<Input, *, Output>) {
+    private fun effectPhase(manager: FormulaManagerImpl<Input, *, Output>) {
         isExecuting = true
         while (executionRequested) {
             executionRequested = false
 
             val transitionId = manager.transitionID
-            if (!manager.terminated) {
-                if (manager.terminateDetachedChildren()) {
-                    continue
-                }
-
-                if (manager.terminateOldUpdates()) {
-                    continue
-                }
-
-                if (manager.startNewUpdates()) {
-                    continue
-                }
-            }
-
             // We execute pending side-effects even after termination
             if (executeEffects(manager, transitionId)) {
                 continue
@@ -174,6 +174,7 @@ class FormulaRuntime<Input : Any, Output : Any>(
             val effects = effectQueue.pollFirst()
             if (effects != null) {
                 effects.execute()
+                inspector?.onEffectExecuted()
 
                 if (manager.hasTransitioned(transitionId)) {
                     return true

diff --git a/formula/src/main/java/com/instacart/formula/Inspector.kt b/formula/src/main/java/com/instacart/formula/Inspector.kt
@@ -48,6 +48,7 @@ interface Inspector {
     /**
      * Called when an action is started.
      *
+     *
      * @param formulaType Formula type used to filter for specific events.
      * @param action Action that was started.
      */
@@ -61,6 +62,11 @@ interface Inspector {
      */
     fun onActionFinished(formulaType: KClass<*>, action: DeferredAction<*>) = Unit
 
+    /**
+     * Called when transition [Effects] are executed
+     */
+    fun onEffectExecuted() = Unit
+
     /**
      * Called when a transition happens
      *

diff --git a/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt b/formula/src/main/java/com/instacart/formula/internal/ActionManager.kt
@@ -52,7 +52,7 @@ internal class ActionManager(
                 running?.remove(action)
                 finishAction(action)
 
-                if (manager.hasTransitioned(transitionID)) {
+                if (manager.hasTransitioned(transitionID) || manager.hasPendingTransitions()) {
                     return true
                 }
             }
@@ -79,7 +79,7 @@ internal class ActionManager(
                 getOrInitRunningActions().add(action)
                 action.start()
 
-                if (manager.hasTransitioned(transitionID)) {
+                if (manager.hasTransitioned(transitionID) || manager.hasPendingTransitions()) {
                     return true
                 }
             }

diff --git a/formula/src/main/java/com/instacart/formula/internal/ChildrenManager.kt b/formula/src/main/java/com/instacart/formula/internal/ChildrenManager.kt
@@ -13,46 +13,19 @@ internal class ChildrenManager(
     private var children: SingleRequestMap<Any, FormulaManager<*, *>>? = null
     private var pendingRemoval: MutableList<FormulaManager<*, *>>? = null
 
-    private val actionsToStart = PendingFormulaManagerList(delegate) { manager ->
-        manager.startNewUpdates()
-    }
-    private val actionsToRemove = PendingFormulaManagerList(delegate) { manager ->
-        manager.terminateOldUpdates()
-    }
-
-    private val hasDetachedChildren = PendingFormulaManagerList(delegate) { manager ->
-        manager.terminateDetachedChildren()
-    }
-
     fun evaluationFinished() {
         children?.clearUnrequested {
             pendingRemoval = pendingRemoval ?: mutableListOf()
             it.markAsTerminated()
             pendingRemoval?.add(it)
         }
-
-        actionsToStart.evaluationFinished()
-        actionsToRemove.evaluationFinished()
-        hasDetachedChildren.evaluationFinished()
     }
 
-    fun terminateDetachedChildren(transitionID: Long): Boolean {
+    fun terminateChildren(transitionID: Long): Boolean {
         val local = pendingRemoval
         pendingRemoval = null
         local?.forEach { it.performTerminationSideEffects() }
-        if (delegate.hasTransitioned(transitionID)) {
-            return true
-        }
-
-        return hasDetachedChildren.iterate(children, transitionID)
-    }
-
-    fun terminateOldUpdates(transitionID: Long): Boolean {
-        return actionsToRemove.iterate(children, transitionID)
-    }
-
-    fun startNewUpdates(transitionID: Long): Boolean {
-        return actionsToStart.iterate(children, transitionID)
+        return delegate.hasTransitioned(transitionID) || delegate.hasPendingTransitions()
     }
 
     fun markAsTerminated() {

diff --git a/formula/src/main/java/com/instacart/formula/internal/Event.kt b/formula/src/main/java/com/instacart/formula/internal/Event.kt
@@ -0,0 +1,5 @@
+package com.instacart.formula.internal
+
+fun interface Event {
+    fun dispatch()
+}
diff --git a/formula/src/main/java/com/instacart/formula/internal/FormulaManager.kt b/formula/src/main/java/com/instacart/formula/internal/FormulaManager.kt
@@ -15,23 +15,6 @@ interface FormulaManager<Input, Output> {
      */
     fun evaluate(input: Input): Evaluation<Output>
 
-    /**
-     * Called after [evaluate] to terminate children that were removed.
-     *
-     * @return True if transition happened while performing this.
-     */
-    fun terminateDetachedChildren(): Boolean
-
-    /**
-     * Called after [evaluate] to terminate old streams.
-     */
-    fun terminateOldUpdates(): Boolean
-
-    /**
-     * Called after [evaluate] to start new streams.
-     */
-    fun startNewUpdates(): Boolean
-
     /**
      * Called when [Formula] is removed. This is should not trigger any external side-effects,
      * only mark itself and its children as terminated.