Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions formula/src/main/java/com/instacart/formula/FormulaRuntime.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.cancel
import java.util.LinkedList
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicReference
import kotlin.coroutines.CoroutineContext
Expand Down Expand Up @@ -82,7 +81,7 @@ class FormulaRuntime<Input : Any, Output : Any>(
/**
* Global transition effect queue which executes side-effects after all formulas are idle.
*/
private val globalEffectQueue = LinkedList<Effect>()
private val globalEffectQueue = ArrayDeque<Effect>()

/**
* Determines if we are iterating through [globalEffectQueue]. It prevents us from
Expand Down Expand Up @@ -342,7 +341,7 @@ class FormulaRuntime<Input : Any, Output : Any>(
private fun executeTransitionEffects() {
isExecutingEffects = true
while (globalEffectQueue.isNotEmpty()) {
val effect = globalEffectQueue.pollFirst()
val effect = globalEffectQueue.removeFirst()
val dispatcher = when (effect.type) {
Effect.Unconfined -> Dispatcher.None
Effect.Main -> Dispatcher.Main
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.instacart.formula.batch

import java.util.LinkedList
import java.util.concurrent.atomic.AtomicBoolean

internal class BatchImpl internal constructor(
Expand All @@ -10,10 +9,10 @@ internal class BatchImpl internal constructor(
) : BatchScheduler.Batch {

private val isScheduled = AtomicBoolean(false)
private val updates = LinkedList<() -> Unit>()
private val updates = ArrayDeque<() -> Unit>()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things to consider with ArrayDeque

  • What is the initial memory allocation? Is that right for our use case?
  • I'm not sure if ArrayDeque sizes down. How likely is it that it spikes in items for a brief moment and then stays there long-term.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good callout. It sounds as though the initial memory size would be quite small, less than 100 bytes. As for growing infinitely, that is in fact a problem that this would introduce, though I think even in the thousands, it would be maybe a few kilobytes of memory usage.

We can size down manually. Let me look into that.


fun add(update: () -> Unit) {
updates.add(update)
updates.addLast(update)
}

override fun execute() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import com.instacart.formula.lifecycle.LifecycleScheduler
import com.instacart.formula.lifecycle.ValidationException
import com.instacart.formula.plugin.ChildAlreadyUsedException
import com.instacart.formula.plugin.FormulaError
import java.util.LinkedList

/**
* Responsible for keeping track of formula's state, running actions, and child formulas. The
Expand Down Expand Up @@ -69,7 +68,7 @@ internal class FormulaManagerImpl<Input, State, Output>(
* while [isRunning] is true. If [isRunning] is false, we will pass the transitions
* to [ManagerDelegate].
*/
private val transitionQueue = LinkedList<DeferredTransition<*, *, *>>()
private val transitionQueue = ArrayDeque<DeferredTransition<*, *, *>>()

fun canUpdatesContinue(evaluationId: Long): Boolean {
return !isEvaluationNeeded(evaluationId) && transitionQueue.isEmpty()
Expand Down Expand Up @@ -307,7 +306,7 @@ internal class FormulaManagerImpl<Input, State, Output>(

// Execute deferred transitions
while (transitionQueue.isNotEmpty()) {
transitionQueue.pollFirst().execute()
transitionQueue.removeFirst().execute()
}

inspector?.onFormulaFinished(formulaType)
Expand Down Expand Up @@ -381,7 +380,7 @@ internal class FormulaManagerImpl<Input, State, Output>(
*/
private fun handleTransitionQueue(evaluationId: Long): Boolean {
while (transitionQueue.isNotEmpty()) {
val event = transitionQueue.pollFirst()
val event = transitionQueue.removeFirst()
event.execute()
if (isEvaluationNeeded(evaluationId)) {
return true
Expand Down
Loading