Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Commit

Permalink
For #20596 remove startup timeline probes
Browse files Browse the repository at this point in the history
  • Loading branch information
RotBolt authored and mcomella committed Aug 20, 2021
1 parent 693fbef commit 233be62
Show file tree
Hide file tree
Showing 11 changed files with 33 additions and 630 deletions.
123 changes: 0 additions & 123 deletions app/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4405,129 +4405,6 @@ addons:
- [email protected]
expires: "2021-12-01"

startup.timeline:
framework_primary:
disabled: true
send_in_pings:
- startup-timeline
type: timespan
time_unit: millisecond
description: |
The duration the Android framework takes to start before letting us run
code in `*Application.init` when this device has
`clock_ticks_per_second_v2` equal to 100: if it's not equal to 100, then
this value is captured in `framework_secondary`. We split this into two
metrics to make it easier to analyze in GLAM. We split on 100 because
when we did our initial brief analysis -
https://sql.telemetry.mozilla.org/queries/75591 - the results for clocks
ticks were overwhelmingly 100.
The duration is calculated from `appInitTimestamp -
processStartTimestamp`. `processStartTimestamp` is derived from the clock
tick time unit, which is expected to be less granular than nanoseconds.
Therefore, we convert and round our timestamps to clock ticks before
computing the difference and convert back to nanoseconds to report.
For debugging purposes, `clock_ticks_per_second_v2`, which may vary
between devices, is also reported as a metric
bugs:
- https://github.com/mozilla-mobile/fenix/issues/8803
- https://github.com/mozilla-mobile/fenix/issues/17972
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/9788#pullrequestreview-394228626
- https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068
- https://github.com/mozilla-mobile/fenix/pull/18043#issue-575389284
data_sensitivity:
- technical
notification_emails:
- [email protected]
- [email protected]
expires: "2021-08-01"
framework_secondary:
disabled: true
send_in_pings:
- startup-timeline
type: timespan
time_unit: millisecond
description: |
The duration the Android framework takes to start before letting us run
code in `*Application.init` when this device has
`clock_ticks_per_second_v2` not equal to 100. For more details on this
metric, see `framework_primary`
bugs:
- https://github.com/mozilla-mobile/fenix/issues/17972
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/18043#issue-575389284
data_sensitivity:
- technical
notification_emails:
- [email protected]
- [email protected]
expires: "2021-08-01"
framework_start_error:
disabled: true
send_in_pings:
- startup-timeline
type: boolean
description: |
An error when attempting to record `framework_primary/secondary` - the
application init timestamp returned a negative value - which is likely
indicative of a bug in the implementation.
bugs:
- https://github.com/mozilla-mobile/fenix/issues/8803
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/9788#pullrequestreview-394228626
- https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068
data_sensitivity:
- technical
notification_emails:
- [email protected]
- [email protected]
expires: "2021-08-01"
framework_start_read_error:
disabled: true
send_in_pings:
- startup-timeline
type: boolean
description: |
An error when attempting to read stats from /proc pseudo-filesystem -
privacy managers can block access to reading these files -
the application will catch a file reading exception.
bugs:
- https://github.com/mozilla-mobile/fenix/issues/10434
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/10481
- https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068
data_sensitivity:
- technical
notification_emails:
- [email protected]
- [email protected]
expires: "2021-08-01"
clock_ticks_per_second_v2:
disabled: true
send_in_pings:
- startup-timeline
type: quantity
unit: clock ticks per second
description: |
The number of clock tick time units that occur in one second on this
particular device. This value is expected to be used in conjunction with
the `framework_primary/secondary` metrics.
bugs:
- https://github.com/mozilla-mobile/fenix/issues/8803
- https://github.com/mozilla-mobile/fenix/issues/18157
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/9788#pullrequestreview-394228626
- https://github.com/mozilla-mobile/fenix/pull/15713#issuecomment-703972068
- https://github.com/mozilla-mobile/fenix/pull/18158#issue-579593943
data_sensitivity:
- technical
notification_emails:
- [email protected]
- [email protected]
expires: "2021-08-01"

perf.startup:
cold_main_app_to_first_frame:
type: timing_distribution
Expand Down
21 changes: 0 additions & 21 deletions app/pings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,3 @@ first-session:
- https://github.com/mozilla-mobile/fenix/pull/8074#issuecomment-586512202
notification_emails:
- [email protected]

startup-timeline:
description: |
This ping is intended to provide an understanding of startup performance.
In addition to being captured on real devices, the ping data was prematurely
optimized into this separate ping to be isolated from other metrics to be
more easily captured by performance testing automation but that hasn't
happened in practice. We would have removed it but implementation
details don't make that possible:
https://github.com/mozilla-mobile/fenix/issues/17972#issuecomment-781002987
include_client_id: true
bugs:
- https://github.com/mozilla-mobile/fenix/issues/8803
- https://github.com/mozilla-mobile/fenix/issues/17972
data_reviews:
- https://github.com/mozilla-mobile/fenix/pull/9788#pullrequestreview-394228626
- https://github.com/mozilla-mobile/fenix/pull/18043#issue-575389284
notification_emails:
- [email protected]
- [email protected]
5 changes: 1 addition & 4 deletions app/src/main/java/org/mozilla/fenix/HomeActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -242,10 +242,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
}
supportActionBar?.hide()

lifecycle.addObservers(
webExtensionPopupFeature,
StartupTimeline.homeActivityLifecycleObserver
)
lifecycle.addObservers(webExtensionPopupFeature)

if (shouldAddToRecentsScreen(intent)) {
intent.removeExtra(START_IN_RECENTS_SCREEN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ abstract class BaseBrowserFragment :
}

final override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
// weird indentation to avoid breaking blame.
initializeUI(view)

if (customTabSessionId == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* 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.os.SystemClock

/**
* A class to store the application initialization time.
* Time is stores in elapsed real time nano seconds
*/
internal class ApplicationInitTimeContainer(
private val getElapsedRealtimeNanos: () -> Long = SystemClock::elapsedRealtimeNanos
) {

var applicationInitNanos = -1L
private set
private var isApplicationInitCalled = false

fun onApplicationInit() {
// This gets called from multiple processes: don't do anything expensive. See call site for details.
//
// In the main process, there are multiple Application impl so we ensure it's only set by
// the first one.
if (!isApplicationInitCalled) {
isApplicationInitCalled = true
applicationInitNanos = getElapsedRealtimeNanos()
}
}
}

This file was deleted.

43 changes: 1 addition & 42 deletions app/src/main/java/org/mozilla/fenix/perf/StartupTimeline.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,6 @@
package org.mozilla.fenix.perf

import androidx.annotation.UiThread
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.LifecycleObserver
import androidx.lifecycle.OnLifecycleEvent
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.DelicateCoroutinesApi
import kotlinx.coroutines.GlobalScope
import kotlinx.coroutines.launch
import mozilla.components.service.glean.private.NoReasonCodes
import mozilla.components.service.glean.private.PingType
import org.mozilla.fenix.GleanMetrics.Pings
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.home.sessioncontrol.viewholders.topsites.TopSiteItemViewHolder
import org.mozilla.fenix.perf.StartupTimeline.onApplicationInit
Expand All @@ -41,10 +31,7 @@ object StartupTimeline {
private var state: StartupState = StartupState.Cold(StartupDestination.UNKNOWN)

private val reportFullyDrawn by lazy { StartupReportFullyDrawn() }
internal val frameworkStartMeasurement by lazy { StartupFrameworkStartMeasurement() }
internal val homeActivityLifecycleObserver by lazy {
StartupHomeActivityLifecycleObserver(frameworkStartMeasurement)
}
internal val frameworkStartMeasurement by lazy { ApplicationInitTimeContainer() }

fun onApplicationInit() {
// This gets called from multiple processes: don't do anything expensive. See call site for details.
Expand All @@ -71,31 +58,3 @@ object StartupTimeline {
state = StartupTimelineStateMachine.getNextState(state, startingActivity)
}
}

/**
* A [LifecycleObserver] for [HomeActivity] focused on startup performance measurement.
*/
@OptIn(DelicateCoroutinesApi::class) // GlobalScope usage
internal class StartupHomeActivityLifecycleObserver(
private val frameworkStartMeasurement: StartupFrameworkStartMeasurement,
private val startupTimeline: PingType<NoReasonCodes> = Pings.startupTimeline,
private val scope: CoroutineScope = GlobalScope
) : LifecycleObserver {

@OnLifecycleEvent(Lifecycle.Event.ON_STOP)
fun onStop() {
scope.launch { // use background thread due to expensive metrics.
// Ensure any last metrics are set before submission.
frameworkStartMeasurement.setExpensiveMetric()

// Startup metrics placed in the Activity should be re-recorded each time the Activity
// is started so we need to clear the ping lifetime by submitting once per each startup.
// It's less complex to add it here rather than the visual completeness task manager.
//
// N.B.: this submission location may need to be changed if we add metrics outside of the
// HomeActivity startup path (e.g. if the user goes directly to a separate activity and
// closes the app, they will never hit this) to appropriately adjust for the ping lifetimes.
startupTimeline.submit()
}
}
}
Loading

0 comments on commit 233be62

Please sign in to comment.