From 0cbedaadb18981996ec609f6e3754af96b610aae Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Wed, 7 Apr 2021 17:15:09 -0700 Subject: [PATCH] For #18836: add StartupStateProvider. --- .../mozilla/fenix/components/Components.kt | 2 + .../fenix/perf/StartupStateProvider.kt | 71 ++++++ .../fenix/perf/StartupStateProviderTest.kt | 236 ++++++++++++++++++ 3 files changed, 309 insertions(+) create mode 100644 app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt create mode 100644 app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 3a2fa03e4..fb8dfda18 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -31,6 +31,7 @@ import org.mozilla.fenix.ext.settings import org.mozilla.fenix.perf.AppStartReasonProvider import org.mozilla.fenix.perf.StartupActivityLog import org.mozilla.fenix.perf.StartupActivityStateProvider +import org.mozilla.fenix.perf.StartupStateProvider import org.mozilla.fenix.perf.lazyMonitored import org.mozilla.fenix.utils.ClipboardHandler import org.mozilla.fenix.utils.Mockable @@ -178,4 +179,5 @@ class Components(private val context: Context) { val appStartReasonProvider by lazyMonitored { AppStartReasonProvider() } val startupActivityStateProvider by lazyMonitored { StartupActivityStateProvider() } val startupActivityLog by lazyMonitored { StartupActivityLog() } + val startupStateProvider by lazyMonitored { StartupStateProvider(startupActivityLog, appStartReasonProvider) } } diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt new file mode 100644 index 000000000..f22cebb6e --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt @@ -0,0 +1,71 @@ +/* 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.perf + +import android.app.Activity +import org.mozilla.fenix.perf.AppStartReasonProvider.StartReason +import org.mozilla.fenix.perf.StartupActivityLog.LogEntry + +/** + * Identifies the "state" of start up where state can be COLD/WARM/HOT and possibly others. See + * the [Fenix perf glossary](https://wiki.mozilla.org/index.php?title=Performance/Fenix/Glossary) + * for specific definitions. + * + * This class is nuanced: **please read the kdoc carefully before using it.** Consider contacting + * the perf team with your use case. + * + * For this class, we use the terminology from the [StartupActivityLog] such as STARTED and STOPPED. + * However, we're assuming STARTED means foregrounded and STOPPED means backgrounded. If this + * assumption is false, the logic in this class may be incorrect. + */ +class StartupStateProvider( + private val startupLog: StartupActivityLog, + private val startReasonProvider: AppStartReasonProvider +) { + + /** + * Returns true if the current startup state is COLD and the currently started activity is the + * first started activity (i.e. we can use it for performance measurements). + * + * This method must be called after the foreground Activity is STARTED. + */ + fun isColdStartForStartedActivity(activityClass: Class): Boolean { + // A cold start means: + // - the process was started for the first started activity (e.g. not a service) + // - the first started activity ever is still active + // + // Thus, for the activity log we expect: + // [... Activity-STARTED, App-STARTED] + // since if another Activity was started, it would appear after App-STARTED. This is where: + // - the app has not been stopped ever + if (startReasonProvider.reason != StartReason.ACTIVITY) { + return false + } + + val firstActivityStartedIndex = startupLog.log.indexOfFirst { it is LogEntry.StartedActivityLogEntry } + if (firstActivityStartedIndex < 0) { + return false // if no activities are started, then we haven't started up yet. + } + + val firstActivityStartedAndAfter = startupLog.log.subList(firstActivityStartedIndex, startupLog.log.size) + val isFirstActivityStartedStillForegrounded = firstActivityStartedAndAfter == listOf( + LogEntry.StartedActivityLogEntry(activityClass), + LogEntry.AppStarted + ) + + val hasAppBeenStopped = startupLog.log.contains(LogEntry.AppStopped) + + return isFirstActivityStartedStillForegrounded && !hasAppBeenStopped + } + + /** + * A short-circuit implementation of [isColdStartForStartedActivity] that will return false early + * so we don't have to call [isColdStartForStartedActivity]. + * + * When this can be called might be tightly coupled to [ColdStartupDurationTelemetry]: use at + * your own risk. + */ + fun shouldShortCircuitColdStart(): Boolean = startupLog.log.contains(LogEntry.AppStopped) +} diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt new file mode 100644 index 000000000..2dcf7d819 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt @@ -0,0 +1,236 @@ +/* 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.perf + +import io.mockk.MockKAnnotations +import io.mockk.every +import io.mockk.impl.annotations.MockK +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.mozilla.fenix.HomeActivity +import org.mozilla.fenix.IntentReceiverActivity +import org.mozilla.fenix.perf.AppStartReasonProvider.StartReason +import org.mozilla.fenix.perf.StartupActivityLog.LogEntry + +class StartupStateProviderTest { + + private lateinit var provider: StartupStateProvider + @MockK private lateinit var startupActivityLog: StartupActivityLog + @MockK private lateinit var startReasonProvider: AppStartReasonProvider + + private lateinit var logEntries: MutableList + + private val homeActivityClass = HomeActivity::class.java + private val irActivityClass = IntentReceiverActivity::class.java + + @Before + fun setUp() { + MockKAnnotations.init(this) + + provider = StartupStateProvider(startupActivityLog, startReasonProvider) + + logEntries = mutableListOf() + every { startupActivityLog.log } returns logEntries + + every { startReasonProvider.reason } returns StartReason.ACTIVITY // default to minimize repetition. + } + + @Test + fun `GIVEN the app started for an activity WHEN we launched to HomeActivity directly THEN start up is cold`() { + // These entries mimic observed behavior. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertTrue(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity WHEN we launched to HA through a non-drawing IntentRA THEN start up is cold`() { + // These entries mimic observed behavior. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertTrue(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity WHEN we launched HA through a drawing IntentRA THEN start up is not cold`() { + // These entries mimic observed behavior for local code changes. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.StartedActivityLogEntry(irActivityClass), + LogEntry.AppStarted, + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.StoppedActivityLogEntry(irActivityClass) + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity WHEN two HomeActivities are created THEN start up is not cold`() { + // We're making an assumption about how this would work based on previous observed patterns. + // AIUI, we should never have more than one HomeActivity. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted, + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.StoppedActivityLogEntry(homeActivityClass) + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity and we're truncating the log for optimization WHEN warm start THEN start up is not cold`() { + // These entries are from observed behavior. + logEntries.addAll(listOf( + LogEntry.AppStopped, + LogEntry.StoppedActivityLogEntry(homeActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity and we're truncating the log for optimization WHEN hot start THEN start up is not cold`() { + // These entries are from observed behavior. + logEntries.addAll(listOf( + LogEntry.AppStopped, + LogEntry.StoppedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity and we're not truncating the log for optimization WHEN warm start THEN start up is not cold`() { + // While the entries are from observed behavior, this log shouldn't occur in the wild due to + // our log optimizations. However, just in case the behavior changes, we check for it. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted, + LogEntry.AppStopped, + LogEntry.StoppedActivityLogEntry(homeActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity and we're not truncating the log for optimization WHEN hot start THEN start up is not cold`() { + // This shouldn't occur in the wild due to the optimization but, just in case the behavior changes, + // we check for it. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted, + LogEntry.AppStopped, + LogEntry.StoppedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity WHEN multiple activities are started but not stopped (maybe impossible) THEN start up is not cold`() { + fun assertIsNotCold() { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) } + + // Since we've never observed this, there are multiple ways the events could + // theoretically be ordered: we try a few. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.StartedActivityLogEntry(irActivityClass), + LogEntry.AppStarted, + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass) + )) + assertIsNotCold() + + logEntries.clear() + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.StartedActivityLogEntry(irActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertIsNotCold() + + logEntries.clear() + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(irActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertIsNotCold() + } + + @Test + fun `GIVEN the app started for an activity WHEN an activity hasn't been created yet THEN start up is not cold`() { + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app started for an activity WHEN an activity hasn't started yet THEN start up is not cold`() { + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass) + )) + assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) + } + + @Test + fun `GIVEN the app did not start for an activity WHEN is cold is checked THEN it returns false`() { + fun assertIsNotCold() { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) } + + every { startReasonProvider.reason } returns StartReason.NON_ACTIVITY + assertIsNotCold() // 🔥 + + // These are normally the success paths. + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertIsNotCold() + + logEntries.clear() + logEntries.addAll(listOf( + LogEntry.CreatedActivityLogEntry(irActivityClass), + LogEntry.CreatedActivityLogEntry(homeActivityClass), + LogEntry.StartedActivityLogEntry(homeActivityClass), + LogEntry.AppStarted + )) + assertIsNotCold() + } + + @Test + fun `GIVEN the app has been stopped WHEN is cold short circuit is called THEN it returns true`() { + logEntries.add(LogEntry.AppStopped) + assertTrue(provider.shouldShortCircuitColdStart()) + } + + @Test + fun `GIVEN the app has not been stopped WHEN is cold short circuit is called THEN it returns false`() { + assertFalse(provider.shouldShortCircuitColdStart()) + } +}