Skip to content

Commit

Permalink
For mozilla-mobile#7781: instrument visual completeness for top sites.
Browse files Browse the repository at this point in the history
Eyeballing my output in *Debug builds on my P2, this adds approximately 115ms
or slightly less from first frame drawn to visually complete time.
  • Loading branch information
mcomella committed Feb 24, 2020
1 parent fa2cd6c commit d6f71dc
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 30 deletions.
1 change: 0 additions & 1 deletion app/src/main/java/org/mozilla/fenix/HomeActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {

setupThemeAndBrowsingMode(getModeFromIntentOrLastKnown(intent))
setContentView(R.layout.activity_home)
Performance.instrumentColdStartupToHomescreenTime(this)

externalSourceIntentProcessors.any { it.process(intent, navHost.navController, this.intent) }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.ListAdapter
import mozilla.components.feature.top.sites.TopSite
import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor
import org.mozilla.fenix.perf.ColdStartPerfMonitor

class TopSitesAdapter(
private val interactor: TopSiteInteractor
Expand All @@ -21,6 +22,7 @@ class TopSitesAdapter(
}

override fun onBindViewHolder(holder: TopSiteItemViewHolder, position: Int) {
ColdStartPerfMonitor.onTopSitesItemBound(holder)
holder.bind(getItem(position))
}

Expand Down
43 changes: 43 additions & 0 deletions app/src/main/java/org/mozilla/fenix/perf/ColdStartPerfMonitor.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/* 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 androidx.core.view.doOnPreDraw
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.home.sessioncontrol.viewholders.topsites.TopSiteItemViewHolder

/**
* Functionality related to measuring the performance of cold startup. Note: this is cold startup in
* the context of FNPRMS: we have yet to define cold startup in the context of the production app.
*/
object ColdStartPerfMonitor {

private var isVisuallyComplete = false

/**
* Instruments "visually complete" cold startup time to homescreen for use with our internal
* measuring system, FNPRMS. This value may also appear in the Google Play Vitals dashboards.
*
* For FNPRMS, we define "visually complete" to be when top sites is loaded with placeholders;
* the animation to display top sites will occur after this point, as will the asynchronous
* loading of the actual top sites icons. Our focus for visually complete is usability.
* There are no tabs available in our FNPRMS tests so they are ignored for this instrumentation.
*
* This will need to be rewritten if any parts of the UI are changed to be displayed asynchronously.
*/
fun onTopSitesItemBound(holder: TopSiteItemViewHolder) {
if (isVisuallyComplete) { return }
isVisuallyComplete = true

// For greater accuracy, we could add an onDrawListener instead of a preDrawListener but:
// - single use onDrawListeners are not built-in and it's non-trivial to write one
// - the difference in timing is minimal (< 7ms on Pixel 2)
// - if we compare against another app using a preDrawListener, as we are with Fennec, it
// should be comparable
//
// Ideally we wouldn't cast to HomeActivity but we want to save implementation time.
holder.itemView.doOnPreDraw { (it.context as HomeActivity).reportFullyDrawn() }
}
}
29 changes: 1 addition & 28 deletions app/src/main/java/org/mozilla/fenix/perf/Performance.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ import android.content.Context
import android.content.Intent
import android.content.IntentFilter
import android.os.BatteryManager
import androidx.core.view.doOnPreDraw
import kotlinx.android.synthetic.main.activity_home.*
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.onboarding.FenixOnboarding
import android.provider.Settings as AndroidSettings
import org.mozilla.fenix.utils.Settings
import android.provider.Settings as AndroidSettings

/**
* A collection of objects related to app performance.
Expand All @@ -22,30 +19,6 @@ object Performance {
const val TAG = "FenixPerf"
private const val EXTRA_IS_PERFORMANCE_TEST = "performancetest"

/**
* Instruments cold startup time for use with our internal measuring system, FNPRMS. This may
* also appear in Google Play Vitals dashboards.
*
* This will need to be rewritten if any parts of the UI are changed to be displayed
* asynchronously.
*
* In the current implementation, we only intend to instrument cold startup to the homescreen.
* To save implementation time, we ignore the fact that the RecyclerView draws twice if the user
* has tabs, collections, etc. open: the "No tabs" placeholder and a tab list. This
* instrumentation will only capture the "No tabs" draw.
*/
fun instrumentColdStartupToHomescreenTime(activity: HomeActivity) {
// For greater accuracy, we could add an onDrawListener instead of a preDrawListener but:
// - single use onDrawListeners are not built-in and it's non-trivial to write one
// - the difference in timing is minimal (< 7ms on Pixel 2)
// - if we compare against another app using a preDrawListener, it should be comparable
//
// Unfortunately, this is tightly coupled to the root view of HomeActivity's view hierarchy
activity.rootContainer.doOnPreDraw {
activity.reportFullyDrawn()
}
}

/**
* Processes intent for Performance testing to remove protection pop up ( but keeps the TP
* on) and removes the onboarding screen.
Expand Down
1 change: 0 additions & 1 deletion app/src/main/res/layout/activity_home.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:id="@+id/rootContainer"
tools:context=".HomeActivity">

<ViewStub
Expand Down

0 comments on commit d6f71dc

Please sign in to comment.