From be1fa8df7d5cd95270c7486987badf4438e071c7 Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Thu, 26 Mar 2020 23:12:00 -0700 Subject: [PATCH] Pre: introduce a RunWhenReadyQueue This replaces the StartupTaskManager we had with a more general class. New implementation is a thread-safe "gated task executor", which either runs the task right away if it's marked as 'ready', or queries it to be executed later on. This ability to either execute or queue a task will be useful later on in the commit series. --- .../org/mozilla/fenix/FenixApplication.kt | 6 +- .../java/org/mozilla/fenix/HomeActivity.kt | 10 ++-- .../fenix/components/PerformanceComponent.kt | 4 +- .../components/metrics/GleanMetricsService.kt | 20 +++---- .../mozilla/fenix/perf/StartupTaskManager.kt | 55 ------------------- .../PerformanceActivityLifecycleCallbacks.kt | 12 ++-- .../mozilla/fenix/utils/RunWhenReadyQueue.kt | 50 +++++++++++++++++ 7 files changed, 74 insertions(+), 83 deletions(-) delete mode 100644 app/src/main/java/org/mozilla/fenix/perf/StartupTaskManager.kt create mode 100644 app/src/main/java/org/mozilla/fenix/utils/RunWhenReadyQueue.kt diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 0b91a4e47..41a13bc08 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -150,12 +150,12 @@ open class FenixApplication : LocaleAwareApplication() { // } registerActivityLifecycleCallbacks( - PerformanceActivityLifecycleCallbacks(components.performance.visualCompletenessTaskManager) + PerformanceActivityLifecycleCallbacks(components.performance.visualCompletenessQueue) ) - components.performance.visualCompletenessTaskManager.add { + components.performance.visualCompletenessQueue.runIfReadyOrQueue { GlobalScope.launch(Dispatchers.IO) { - logger.info("Initializing storage after visual completeness...") + logger.info("Running post-visual completeness tasks...") logElapsedTime(logger, "Storage initialization") { components.core.historyStorage.warmUp() components.core.bookmarksStorage.warmUp() diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 6989079ec..cbc22a76f 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -70,7 +70,7 @@ import org.mozilla.fenix.settings.logins.SavedLoginsFragmentDirections import org.mozilla.fenix.theme.DefaultThemeManager import org.mozilla.fenix.theme.ThemeManager import org.mozilla.fenix.utils.BrowsersCache -import org.mozilla.fenix.utils.StartupTaskManager +import org.mozilla.fenix.utils.RunWhenReadyQueue @SuppressWarnings("TooManyFunctions", "LargeClass") open class HomeActivity : LocaleAwareAppCompatActivity() { @@ -81,7 +81,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity() { private var isVisuallyComplete = false - private var visualCompletenessTaskManager: StartupTaskManager? = null + private var visualCompletenessQueue: RunWhenReadyQueue? = null private var sessionObserver: SessionManager.Observer? = null @@ -118,7 +118,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity() { // This delay is temporary. We are delaying 5 seconds until the performance // team can locate the real point of visual completeness. it.postDelayed({ - visualCompletenessTaskManager!!.start() + visualCompletenessQueue!!.ready() }, delay) } } @@ -399,9 +399,9 @@ open class HomeActivity : LocaleAwareAppCompatActivity() { * The root container is null at this point, so let the HomeActivity know that * we are visually complete. */ - fun postVisualCompletenessQueue(visualCompletenessTaskManager: StartupTaskManager) { + fun postVisualCompletenessQueue(visualCompletenessQueue: RunWhenReadyQueue) { isVisuallyComplete = true - this.visualCompletenessTaskManager = visualCompletenessTaskManager + this.visualCompletenessQueue = visualCompletenessQueue } companion object { diff --git a/app/src/main/java/org/mozilla/fenix/components/PerformanceComponent.kt b/app/src/main/java/org/mozilla/fenix/components/PerformanceComponent.kt index c1bc24003..88f03aa40 100644 --- a/app/src/main/java/org/mozilla/fenix/components/PerformanceComponent.kt +++ b/app/src/main/java/org/mozilla/fenix/components/PerformanceComponent.kt @@ -4,11 +4,11 @@ package org.mozilla.fenix.components -import org.mozilla.fenix.utils.StartupTaskManager +import org.mozilla.fenix.utils.RunWhenReadyQueue /** * Component group for all functionality related to performance. */ class PerformanceComponent { - val visualCompletenessTaskManager by lazy { StartupTaskManager() } + val visualCompletenessQueue by lazy { RunWhenReadyQueue() } } diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index acdad31c9..7088a6f15 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -520,20 +520,16 @@ class GleanMetricsService(private val context: Context) : MetricsService { // The code below doesn't need to execute immediately, so we'll add them to the visual // completeness task queue to be run later. - val taskManager = context.components.performance.visualCompletenessTaskManager - - // We have to initialize Glean *on* the main thread, because it registers lifecycle - // observers. However, the activation ping must be sent *off* of the main thread, - // because it calls Google ad APIs that must be called *off* of the main thread. - // These two things actually happen in parallel, but that should be ok because Glean - // can handle events being recorded before it's initialized. - taskManager.add { + context.components.performance.visualCompletenessQueue.runIfReadyOrQueue { + // We have to initialize Glean *on* the main thread, because it registers lifecycle + // observers. However, the activation ping must be sent *off* of the main thread, + // because it calls Google ad APIs that must be called *off* of the main thread. + // These two things actually happen in parallel, but that should be ok because Glean + // can handle events being recorded before it's initialized. Glean.registerPings(Pings) - } - // setStartupMetrics is not a fast function. It does not need to be done before we can consider - // ourselves initialized. So, let's do it, well, later. - taskManager.add { + // setStartupMetrics is not a fast function. It does not need to be done before we can consider + // ourselves initialized. So, let's do it, well, later. setStartupMetrics() } } diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupTaskManager.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupTaskManager.kt deleted file mode 100644 index 58d7eb203..000000000 --- a/app/src/main/java/org/mozilla/fenix/perf/StartupTaskManager.kt +++ /dev/null @@ -1,55 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.utils - -import org.mozilla.gecko.util.ThreadUtils - -typealias StartupTask = () -> Unit - -/** - * A queue of tasks that are performed at specific points during Fenix startup. - * - * This queue contains a list of startup tasks. Each task in the queue will be started once Fenix - * is visually complete. - * - * This class is not thread safe and should only be called from the main thread. - */ -class StartupTaskManager { - private var tasks = mutableListOf() - private var hasStarted = false - private set - - /** - * Add a task to the queue. - * Each task will execute on the main thread. - * - * @param task: The task to add to the queue. - */ - @Synchronized - fun add(task: StartupTask) { - ThreadUtils.assertOnUiThread() - if (hasStarted) { - throw IllegalStateException("New tasks should not be added because queue already " + - "started, and these newly added tasks will not execute.") - } - - tasks.add(task) - } - - /** - * Start all tasks in the queue. When all the tasks have been started, - * clear the queue. - */ - fun start() { - ThreadUtils.assertOnUiThread() - hasStarted = true - - tasks.forEach { it.invoke() } - - // Anything captured by the lambda will remain captured if we hold on to these tasks, - // which takes up more memory than we need to. - tasks.clear() - } -} diff --git a/app/src/main/java/org/mozilla/fenix/session/PerformanceActivityLifecycleCallbacks.kt b/app/src/main/java/org/mozilla/fenix/session/PerformanceActivityLifecycleCallbacks.kt index a60a9f65a..63a14cf5e 100644 --- a/app/src/main/java/org/mozilla/fenix/session/PerformanceActivityLifecycleCallbacks.kt +++ b/app/src/main/java/org/mozilla/fenix/session/PerformanceActivityLifecycleCallbacks.kt @@ -11,7 +11,7 @@ import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.IntentReceiverActivity import org.mozilla.fenix.browser.BrowserPerformanceTestActivity import org.mozilla.fenix.settings.account.AuthIntentReceiverActivity -import org.mozilla.fenix.utils.StartupTaskManager +import org.mozilla.fenix.utils.RunWhenReadyQueue import org.mozilla.fenix.widget.VoiceSearchActivity /** @@ -19,7 +19,7 @@ import org.mozilla.fenix.widget.VoiceSearchActivity */ @SuppressWarnings("EmptyFunctionBlock") class PerformanceActivityLifecycleCallbacks( - private val visualCompletenessTaskManager: StartupTaskManager + private val visualCompletenessQueue: RunWhenReadyQueue ) : Application.ActivityLifecycleCallbacks { override fun onActivityCreated(activity: Activity, bundle: Bundle?) { @@ -42,7 +42,7 @@ class PerformanceActivityLifecycleCallbacks( } /** - * This starts the StartupTaskManager, eitther delayed or right away, if the activity is a + * This marks visualCompletenessQueue as ready, either delayed or right away, if the activity is a * terminal activity. If not, do nothing. */ private fun ifNecessaryPostVisualCompleteness(activity: Activity) { @@ -51,13 +51,13 @@ class PerformanceActivityLifecycleCallbacks( } if (activity is HomeActivity) { - // We should delay the visualCompletenessTaskManager when reaching the HomeActivity + // We should delay the visualCompletenessQueue when reaching the HomeActivity // to ensure all tasks are delayed until after visual completeness - activity.postVisualCompletenessQueue(visualCompletenessTaskManager) + activity.postVisualCompletenessQueue(visualCompletenessQueue) } else if (shouldStartVisualCompletenessQueueImmediately()) { // If we do not go through the home activity, we have to start the tasks // immediately to avoid spending time implementing it. - visualCompletenessTaskManager.start() + visualCompletenessQueue.ready() } } diff --git a/app/src/main/java/org/mozilla/fenix/utils/RunWhenReadyQueue.kt b/app/src/main/java/org/mozilla/fenix/utils/RunWhenReadyQueue.kt new file mode 100644 index 000000000..2a820a652 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/utils/RunWhenReadyQueue.kt @@ -0,0 +1,50 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.utils + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.atomic.AtomicBoolean + +/** + * A queue that acts as a gate, either executing tasks right away if the queue is marked as "ready", + * i.e. gate is open, or queues them to be executed whenever the queue is marked as ready in the + * future, i.e. gate becomes open. + */ +class RunWhenReadyQueue { + private val tasks = CopyOnWriteArrayList<() -> Unit>() + private val isReady = AtomicBoolean(false) + + /** + * Runs the [task] if this queue is marked as ready, or queues it for later execution. + * Task will be executed on the main thread. + * + * @param task: The task to run now if queue is ready or queue for later execution. + */ + fun runIfReadyOrQueue(task: () -> Unit) { + if (isReady.get()) { + CoroutineScope(Dispatchers.Main).launch { task.invoke() } + } else { + tasks.add(task) + } + } + + /** + * Mark queue as ready. Pending tasks will execute, and all tasks passed to [runIfReadyOrQueue] + * after this point will be executed immediately. + */ + fun ready() { + // Make sure that calls to `ready` are idempotent. + if (!isReady.compareAndSet(false, true)) { + return + } + + CoroutineScope(Dispatchers.Main).launch { + tasks.forEach { it.invoke() }.also { tasks.clear() } + } + } +}