Rewriting internals for better performance.

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,18 +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.executeUpdates()) {
-                    continue
-                }
-            }
-
             // We execute pending side-effects even after termination
             if (executeEffects(manager, transitionId)) {
                 continue
@@ -166,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

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
      *

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
                 }
             }

formula/src/main/java/com/instacart/formula/internal/ChildrenManager.kt

@@ -13,31 +13,19 @@ internal class ChildrenManager(
     private var children: SingleRequestMap<Any, FormulaManager<*, *>>? = null
     private var pendingRemoval: MutableList<FormulaManager<*, *>>? = null
 
-    private val childrenToUpdate = PendingFormulaManagerList(delegate) { manager ->
-        manager.executeUpdates()
-    }
     fun evaluationFinished() {
         children?.clearUnrequested {
             pendingRemoval = pendingRemoval ?: mutableListOf()
             it.markAsTerminated()
             pendingRemoval?.add(it)
         }
-
-        childrenToUpdate.evaluationFinished()
-    }
-
-    fun executeChildUpdates(transitionID: Long): Boolean {
-        return childrenToUpdate.iterate(children, transitionID)
     }
 
     fun terminateChildren(transitionID: Long): Boolean {
         val local = pendingRemoval
         pendingRemoval = null
         local?.forEach { it.performTerminationSideEffects() }
-        if (delegate.hasTransitioned(transitionID)) {
-            return true
-        }
-        return false
+        return delegate.hasTransitioned(transitionID) || delegate.hasPendingTransitions()
     }
 
     fun markAsTerminated() {

formula/src/main/java/com/instacart/formula/internal/Event.kt

@@ -0,0 +1,5 @@
+package com.instacart.formula.internal
+
+fun interface Event {
+    fun dispatch()
+}

formula/src/main/java/com/instacart/formula/internal/FormulaManager.kt

@@ -15,12 +15,6 @@ interface FormulaManager<Input, Output> {
      */
     fun evaluate(input: Input): Evaluation<Output>
 
-    /**
-     * Called after [evaluate] to update child formulas and actions.
-     *
-     * @return True if transition happened while performing this.
-     */
-    fun executeUpdates(): Boolean
 
     /**
      * Called when [Formula] is removed. This is should not trigger any external side-effects,

formula/src/main/java/com/instacart/formula/internal/FormulaManagerImpl.kt

@@ -6,6 +6,7 @@ import com.instacart.formula.IFormula
 import com.instacart.formula.Inspector
 import com.instacart.formula.Snapshot
 import com.instacart.formula.Transition
+import java.util.LinkedList
 import kotlin.reflect.KClass
 
 /**
@@ -39,12 +40,30 @@ internal class FormulaManagerImpl<Input, State, Output>(
 
     var transitionID: Long = 0
     var terminated = false
+    private var isEvaluating: Boolean = false
+
+    private val eventQueue = LinkedList<Event>()
+
+    fun onEvent(event: Event) {
+        if (terminated) {
+            event.dispatch()
+        } else if (isEvaluating) {
+            eventQueue.addLast(event)
+        } else {
+            delegate.onPostponedTransition(event)
+        }
+    }
+
+    fun hasPendingTransitions(): Boolean {
+        return eventQueue.isNotEmpty()
+    }
 
     fun hasTransitioned(id: Long): Boolean {
         return transitionID != id
     }
 
     fun handleTransitionResult(result: Transition.Result<State>) {
+        val effects = result.effects
         if (terminated) {
             // State transitions are ignored, only side effects are passed up to be executed.
             delegate.onTransition(type, result, evaluate = false)
@@ -60,8 +79,8 @@ internal class FormulaManagerImpl<Input, State, Output>(
             }
         }
 
-        val evaluate = frame == null || frame.transitionID != transitionID
-        if (evaluate || result.effects != null) {
+        val evaluate = !isEvaluating && frame?.transitionID != transitionID
+        if (evaluate || effects != null) {
             delegate.onTransition(type, result, evaluate)
         }
     }
@@ -75,7 +94,39 @@ internal class FormulaManagerImpl<Input, State, Output>(
      */
     override fun evaluate(input: Input): Evaluation<Output> {
         // TODO: assert main thread.
-        val transitionID = transitionID
+
+        var result: Evaluation<Output>? = null
+        isEvaluating = true
+        while (result == null) {
+            val lastFrame = frame
+            if (lastFrame != null && executeTransitions(lastFrame.transitionID)) {
+                continue
+            }
+
+            val transitionID = transitionID
+
+            val evaluation = evaluation(input, transitionID)
+            if (executeTransitions(transitionID)) {
+                continue
+            }
+
+            if (executeUpdates(transitionID)) {
+                continue
+            }
+
+            result = evaluation
+        }
+        isEvaluating = false
+
+        return result
+    }
+
+    /**
+     * Creates the current [Output] and prepares the next frame that will need to be processed.
+     */
+    private fun evaluation(input: Input, transitionID: Long): Evaluation<Output> {
+        // TODO: assert main thread.
+
         val lastFrame = frame
         if (lastFrame == null && isValidationEnabled) {
             throw ValidationException("Formula should already have run at least once before the validation mode.")
@@ -103,7 +154,6 @@ internal class FormulaManagerImpl<Input, State, Output>(
                     throw ValidationException("$type - input changed during identical re-evaluation - old: $prevInput, new: $input")
                 }
                 state = formula.onInputChanged(prevInput, input, state)
-
                 inspector?.onInputChanged(type, prevInput, input)
             }
         }
@@ -125,25 +175,39 @@ internal class FormulaManagerImpl<Input, State, Output>(
         }
 
         val frame = Frame(snapshot, result, transitionID)
-        actionManager.onNewFrame(frame.evaluation.actions)
         this.frame = frame
 
+        actionManager.onNewFrame(frame.evaluation.actions)
         listeners.evaluationFinished()
         childrenManager?.evaluationFinished()
 
         snapshot.running = true
         if (!isValidationEnabled) {
             inspector?.onEvaluateFinished(type, frame.evaluation.output, evaluated = true)
         }
-        return result
+
+        return frame.evaluation
     }
 
-    override fun executeUpdates(): Boolean {
-        val transitionID = transitionID
-        if (childrenManager?.executeChildUpdates(transitionID) == true) {
-            return true
+    private fun executeTransitions(transitionID: Long): Boolean {
+        if (!hasTransitioned(transitionID)) {
+            while (eventQueue.isNotEmpty()) {
+                val event = eventQueue.pollFirst()
+                event.dispatch()
+                if (hasTransitioned(transitionID)) {
+                    return true
+                }
+            }
         }
 
+        return false
+    }
+
+    /**
+     * Called after [evaluate] to remove detached children, stop detached actions and start
+     * new ones. It will start with child formulas first and then perform execution it self.
+     */
+    private fun executeUpdates(transitionID: Long): Boolean {
         if (childrenManager?.terminateChildren(transitionID) == true) {
             return true
         }
@@ -184,7 +248,15 @@ internal class FormulaManagerImpl<Input, State, Output>(
         if (evaluate) {
             transitionID += 1
         }
-        delegate.onTransition(formulaType, result, evaluate)
+        delegate.onTransition(formulaType, result, !isEvaluating && evaluate)
+    }
+
+    override fun onPostponedTransition(event: Event) {
+        if (isEvaluating) {
+            eventQueue.addLast(event)
+        } else {
+            delegate.onPostponedTransition(event)
+        }
     }
 
     private fun getOrInitChildrenManager(): ChildrenManager {

formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt

@@ -7,23 +7,28 @@ import com.instacart.formula.Transition
  * Note: this class is not a data class because equality is based on instance and not [key].
  */
 @PublishedApi
-internal class ListenerImpl<Input, State, Event>(internal var key: Any) : Listener<Event> {
+internal class ListenerImpl<Input, State, EventT>(internal var key: Any) : Listener<EventT> {
 
+    internal var manager: FormulaManagerImpl<Input, State, *>? = null
     internal var snapshotImpl: SnapshotImpl<Input, State>? = null
-    internal var transition: Transition<Input, State, Event>? = null
+    internal var transition: Transition<Input, State, EventT>? = null
 
-    override fun invoke(event: Event) {
-        snapshotImpl?.let { snapshot ->
-            transition?.let { transition ->
-                val result = transition.toResult(snapshot, event)
-                snapshot.dispatch(result)
-                return
+    override fun invoke(event: EventT) {
+        val manager = manager ?: return
+        val formulaEvent = Event {
+            snapshotImpl?.let { snapshot ->
+                transition?.let { transition ->
+                    val result = transition.toResult(snapshot, event)
+                    snapshot.dispatch(result)
+                }
             }
         }
+        manager.onEvent(formulaEvent)
         // TODO: log if null listener (it might be due to formula removal or due to callback removal)
     }
 
     fun disable() {
+        manager = null
         snapshotImpl = null
         transition = null
     }

formula/src/main/java/com/instacart/formula/internal/ManagerDelegate.kt

@@ -9,4 +9,8 @@ interface ManagerDelegate {
         result: Transition.Result<*>,
         evaluate: Boolean,
     )
+
+    fun onPostponedTransition(
+        event: Event
+    )
 }