diff --git a/formula/src/main/java/com/instacart/formula/FormulaRuntime.kt b/formula/src/main/java/com/instacart/formula/FormulaRuntime.kt index 5f923443..294368fa 100644 --- 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( 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( run(shouldEvaluate = evaluate) } + override fun onPostponedTransition(event: Event) { + event.dispatch() + } + private fun forceRun() = run(shouldEvaluate = true) /** @@ -80,6 +86,8 @@ class FormulaRuntime( * @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( 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( onOutput(checkNotNull(lastOutput)) } } catch (e: Throwable) { + isEvaluating = false + manager?.markAsTerminated() onError(e) manager?.performTerminationSideEffects() @@ -138,26 +152,12 @@ class FormulaRuntime( /** * Executes operations containing side-effects such as starting/terminating streams. */ - private fun executionPhase(manager: FormulaManagerImpl) { + private fun effectPhase(manager: FormulaManagerImpl) { 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( 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 index 5471f95b..5a9d6495 100644 --- 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 index 4745bac9..b430de7f 100644 --- 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 index da490770..23130223 100644 --- 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>? = null private var pendingRemoval: MutableList>? = 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 new file mode 100644 index 00000000..5c9f1037 --- /dev/null +++ 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() +} \ No newline at end of file diff --git a/formula/src/main/java/com/instacart/formula/internal/FormulaManager.kt b/formula/src/main/java/com/instacart/formula/internal/FormulaManager.kt index 9d33afdd..41f7f9b5 100644 --- 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 { */ fun evaluate(input: Input): Evaluation - /** - * 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. diff --git a/formula/src/main/java/com/instacart/formula/internal/FormulaManagerImpl.kt b/formula/src/main/java/com/instacart/formula/internal/FormulaManagerImpl.kt index 0a5e0d0d..824f9b3a 100644 --- a/formula/src/main/java/com/instacart/formula/internal/FormulaManagerImpl.kt +++ b/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( var transitionID: Long = 0 var terminated = false + private var isEvaluating: Boolean = false + + private val eventQueue = LinkedList() + + 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) { + 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( } } - 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( */ override fun evaluate(input: Input): Evaluation { // TODO: assert main thread. - val transitionID = transitionID + + var result: Evaluation? = 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 { + // 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( 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,9 +175,9 @@ internal class FormulaManagerImpl( } val frame = Frame(snapshot, result, transitionID) - actionManager.onNewFrame(frame.evaluation.actions) this.frame = frame + actionManager.onNewFrame(frame.evaluation.actions) listeners.evaluationFinished() childrenManager?.evaluationFinished() @@ -135,37 +185,41 @@ internal class FormulaManagerImpl( if (!isValidationEnabled) { inspector?.onEvaluateFinished(type, frame.evaluation.output, evaluated = true) } - return result - } - override fun terminateDetachedChildren(): Boolean { - return childrenManager?.terminateDetachedChildren(transitionID) == true + return frame.evaluation } - // TODO: should probably terminate children streams, then self. - override fun terminateOldUpdates(): Boolean { - val transitionID = transitionID - if (actionManager.terminateOld(transitionID)) { - return true - } - - // Step through children frames - if (childrenManager?.terminateOldUpdates(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 } - override fun startNewUpdates(): Boolean { - val transitionID = transitionID - // Update parent workers so they are ready to handle events - if (actionManager.startNew(transitionID)) { + /** + * 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 { + // Terminate detached child formulas + if (childrenManager?.terminateChildren(transitionID) == true) { + return true + } + + // Terminate old actions + if (actionManager.terminateOld(transitionID)) { return true } - // Step through children frames - if (childrenManager?.startNewUpdates(transitionID) == true) { + // Start new actions + if (actionManager.startNew(transitionID)) { return true } @@ -201,7 +255,15 @@ internal class FormulaManagerImpl( 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 { diff --git a/formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt b/formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt index 9aa162fe..24ced375 100644 --- a/formula/src/main/java/com/instacart/formula/internal/ListenerImpl.kt +++ b/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(internal var key: Any) : Listener { +internal class ListenerImpl(internal var key: Any) : Listener { + internal var manager: FormulaManagerImpl? = null internal var snapshotImpl: SnapshotImpl? = null - internal var transition: Transition? = null + internal var transition: Transition? = 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 } diff --git a/formula/src/main/java/com/instacart/formula/internal/ManagerDelegate.kt b/formula/src/main/java/com/instacart/formula/internal/ManagerDelegate.kt index ba45e08e..33254d52 100644 --- a/formula/src/main/java/com/instacart/formula/internal/ManagerDelegate.kt +++ b/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 + ) } \ No newline at end of file diff --git a/formula/src/main/java/com/instacart/formula/internal/PendingFormulaManagerList.kt b/formula/src/main/java/com/instacart/formula/internal/PendingFormulaManagerList.kt deleted file mode 100644 index 1418bb23..00000000 --- a/formula/src/main/java/com/instacart/formula/internal/PendingFormulaManagerList.kt +++ /dev/null @@ -1,58 +0,0 @@ -package com.instacart.formula.internal - -internal class PendingFormulaManagerList( - private val manager: FormulaManagerImpl<*, *, *>, - private val action: (FormulaManager<*, *>) -> Boolean, -) { - var invalidated = false - var pending: MutableList>? = null - - - fun evaluationFinished() { - pending?.clear() - invalidated = true - } - - fun iterate( - children: SingleRequestMap>?, - transitionID: Long, - ): Boolean { - prepareList(children) - - if (pending.isNullOrEmpty()) { - return false - } - - val iterator = pending?.iterator() - while (iterator?.hasNext() == true) { - val childFormulaManager = iterator.next() - if (action(childFormulaManager) || manager.hasTransitioned(transitionID)) { - return true - } - - iterator.remove() - } - - return false - } - - - private fun prepareList( - children: SingleRequestMap>? - ) { - if (!invalidated) { - return - } - invalidated = false - - if (!children.isNullOrEmpty()) { - children.forEachValue { - if (pending == null) { - pending = mutableListOf() - } - - pending?.add(it) - } - } - } -} \ No newline at end of file diff --git a/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt b/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt index daa06655..3273a5b0 100644 --- a/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt +++ b/formula/src/main/java/com/instacart/formula/internal/SnapshotImpl.kt @@ -14,7 +14,7 @@ import kotlin.reflect.KClass internal class SnapshotImpl internal constructor( override val input: Input, override val state: State, - val transitionID: Long, + private val transitionID: Long, listeners: Listeners, private val delegate: FormulaManagerImpl, ) : FormulaContext(listeners), Snapshot, TransitionContext { @@ -45,7 +45,6 @@ internal class SnapshotImpl internal constructor( return delegate.child(key, formula, input) } - override fun eventListener( key: Any, useIndex: Boolean, @@ -53,6 +52,7 @@ internal class SnapshotImpl internal constructor( ): Listener { ensureNotRunning() val listener = listeners.initOrFindListener(key, useIndex) + listener.manager = delegate listener.snapshotImpl = this listener.transition = transition return listener @@ -99,7 +99,7 @@ internal class SnapshotImpl internal constructor( if (!terminated && delegate.hasTransitioned(transitionID)) { // We have already transitioned, this should not happen. - throw IllegalStateException("Transition already happened. This is using old event listener: $transition.") + throw IllegalStateException("Transition already happened. This is using old event listener: $transition. Transition: $transitionID != ${delegate.transitionID}") } delegate.handleTransitionResult(transition) diff --git a/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt b/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt index 86b8b3e6..08fdf319 100644 --- a/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt +++ b/formula/src/test/java/com/instacart/formula/FormulaRuntimeTest.kt @@ -1053,6 +1053,7 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) { } } + @Ignore("stack overflows when there are 500 nested child formulas") @Test fun `initialize 500 levels nested formula`() {