Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tracing and JankStats #145

Merged
merged 24 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
1033986
Add tracing library and start using it
keyboardsurfer May 20, 2022
4f57975
Add jankStats and rudamentary jank logging
keyboardsurfer May 20, 2022
895ec4c
Inject JankStats with Hilt
keyboardsurfer May 20, 2022
a37c932
Introduce view extension to track jank
keyboardsurfer May 20, 2022
ccb08a1
Add rememberMetricsStateHolder composable
mlykotom May 23, 2022
a076a50
Use rememberMetricsStateHolder for navigation
mlykotom May 23, 2022
1ac3b4d
Use DisposableEffect + rememberMetricsStateHolder for Interests tab s…
mlykotom May 23, 2022
24a876d
Remove InterestItem state
mlykotom May 23, 2022
dc97d15
Add JankMetricEffect
mlykotom May 23, 2022
57a7b5d
Add AuthorsCarousel scrolling state
mlykotom May 23, 2022
bc7270d
Add JankMetricDisposableEffect
mlykotom May 23, 2022
4dc1d0c
Add ForYou feed scrolling state
mlykotom May 23, 2022
5a1ece4
Add ForYou TopicSelection scrolling state
mlykotom May 23, 2022
e949749
Merge branch 'bw/initialMetrics' of github.com:android/nowinandroid i…
keyboardsurfer Jun 6, 2022
42878c5
Move JankStats metric gathering further down
keyboardsurfer Jun 6, 2022
fa16a25
Merge branch 'main' into bw/initialMetrics
keyboardsurfer Jun 13, 2022
5f690d1
Merge branch 'main' into bw/initialMetrics
keyboardsurfer Jun 17, 2022
0271a9d
Fix spotless
keyboardsurfer Jun 20, 2022
6509b6a
Merge branch 'main' into bw/initialMetrics
keyboardsurfer Jun 27, 2022
c3ca38d
Remove redundant dependency
keyboardsurfer Jul 3, 2022
9f92994
Address review comments
keyboardsurfer Jul 14, 2022
efb76d4
Merge branch 'main' into bw/initialMetrics
keyboardsurfer Jul 14, 2022
26bdcc9
Merge branch 'main' into bw/initialMetrics
keyboardsurfer Jul 14, 2022
7c2b584
Consistent tags & named parameter usage
keyboardsurfer Jul 18, 2022
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
Prev Previous commit
Next Next commit
Address review comments
* Introduce gradle plugin for firebase perf
* Create TrackScrollJank composable to mask tracking code
  • Loading branch information
keyboardsurfer committed Jul 14, 2022
commit 9f929944d08bd424610b92c40ad8c84e57c81241
2 changes: 1 addition & 1 deletion app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ plugins {
id("jacoco")
id("dagger.hilt.android.plugin")
id("nowinandroid.spotless")
id("com.google.firebase.firebase-perf") version "1.4.1"
id("nowinandroid.firebase-perf")
}

android {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ import javax.inject.Inject
@AndroidEntryPoint
class MainActivity : ComponentActivity() {

/**
* Lazily inject [JankStats], which is used to track jank throughout the app.
*/
@Inject
lateinit var lazyStats: dagger.Lazy<JankStats>
keyboardsurfer marked this conversation as resolved.
Show resolved Hide resolved

@Inject
lateinit var jankFrameListener: JankStats.OnFrameListener

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand All @@ -51,15 +51,11 @@ class MainActivity : ComponentActivity() {

override fun onResume() {
super.onResume()
isTrackingEnabled(true)
lazyStats.get().isTrackingEnabled = true
}

override fun onPause() {
super.onPause()
isTrackingEnabled(false)
}

private fun isTrackingEnabled(enabled: Boolean) {
lazyStats.get().isTrackingEnabled = enabled
lazyStats.get().isTrackingEnabled = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination
fun NiaApp(windowSizeClass: WindowSizeClass) {
NiaTheme {
val navController = rememberNavController()

val niaTopLevelNavigation = remember(navController) {
NiaTopLevelNavigation(navController)
}
Expand Down
4 changes: 4 additions & 0 deletions build-logic/convention/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,9 @@ gradlePlugin {
id = "nowinandroid.spotless"
implementationClass = "SpotlessConventionPlugin"
}
register("firebase-perf") {
id = "nowinandroid.firebase-perf"
implementationClass = "FirebasePerfConventionPlugin"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import org.gradle.api.Plugin
import org.gradle.api.Project

class FirebasePerfConventionPlugin : Plugin<Project> {
override fun apply(target: Project) {
with(target) {
pluginManager.findPlugin("com.google.firebase.firebase-perf").apply {
version = "1.4.1"
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

package com.google.samples.apps.nowinandroid.core.ui

import androidx.compose.foundation.gestures.ScrollableState
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.DisposableEffectResult
import androidx.compose.runtime.DisposableEffectScope
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.platform.LocalView
import androidx.metrics.performance.PerformanceMetricsState
import androidx.metrics.performance.PerformanceMetricsState.MetricsStateHolder
Expand Down Expand Up @@ -71,3 +73,18 @@ fun JankMetricDisposableEffect(
reportMetric(this, metrics)
}
}

@Composable
fun TrackScrollJank(scrollableState: ScrollableState, stateName: String) {
JankMetricEffect(scrollableState) { metricsHolder ->
snapshotFlow { scrollableState.isScrollInProgress }.collect { isScrollInProgress ->
metricsHolder.state?.apply {
if (isScrollInProgress) {
addState(stateName, "Scrolling=true")
} else {
removeState(stateName)
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import androidx.compose.material3.Surface
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.draw.clip
Expand All @@ -57,9 +56,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme
import com.google.samples.apps.nowinandroid.core.model.data.Author
import com.google.samples.apps.nowinandroid.core.model.data.FollowableAuthor
import com.google.samples.apps.nowinandroid.core.ui.JankMetricEffect
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.combine
import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank

@Composable
fun AuthorsCarousel(
Expand All @@ -69,21 +66,7 @@ fun AuthorsCarousel(
) {
val lazyListState = rememberLazyListState()

JankMetricEffect(lazyListState) { metricsHolder ->
combine(
snapshotFlow { lazyListState.isScrollInProgress },
snapshotFlow { lazyListState.firstVisibleItemIndex }
) { isScrollInProgress, firstVisibleItemIndex ->
if (isScrollInProgress) {
metricsHolder.state?.addState(
"ForYou:AuthorsCarousel:Scrolling",
"Index=$firstVisibleItemIndex"
)
} else {
metricsHolder.state?.removeState("ForYou:AuthorsCarousel:Scrolling")
}
}.collect()
}
TrackScrollJank(lazyListState, "ForYou:AuthorsCarousel")
keyboardsurfer marked this conversation as resolved.
Show resolved Hide resolved

LazyRow(modifier, lazyListState) {
items(items = authors, key = { item -> item.author.id }) { followableAuthor ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass
import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.snapshotFlow
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
Expand Down Expand Up @@ -93,11 +92,9 @@ import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource
import com.google.samples.apps.nowinandroid.core.model.data.previewAuthors
import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources
import com.google.samples.apps.nowinandroid.core.model.data.previewTopics
import com.google.samples.apps.nowinandroid.core.ui.JankMetricEffect
import com.google.samples.apps.nowinandroid.core.ui.NewsResourceCardExpanded
import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank
import kotlin.math.floor
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.combine

@Composable
fun ForYouRoute(
Expand Down Expand Up @@ -168,21 +165,7 @@ fun ForYouScreen(
}

val lazyListState = rememberLazyListState()
JankMetricEffect(lazyListState) { metricsHolder ->
combine(
snapshotFlow { lazyListState.isScrollInProgress },
snapshotFlow { lazyListState.firstVisibleItemIndex },
) { isScrollInProgress, firstVisibleItemIndex ->
if (isScrollInProgress) {
metricsHolder.state?.addState(
"ForYou:Feed:Scrolling",
"index=$firstVisibleItemIndex"
)
} else {
metricsHolder.state?.removeState("ForYou:Feed:Scrolling")
}
}.collect()
}
TrackScrollJank(lazyListState, "ForYou:Feed")

LazyColumn(
state = lazyListState,
Expand Down Expand Up @@ -311,21 +294,8 @@ private fun TopicSelection(
modifier: Modifier = Modifier
) {
val lazyGridState = rememberLazyGridState()
JankMetricEffect(lazyGridState) { metricsHolder ->
combine(
snapshotFlow { lazyGridState.isScrollInProgress },
snapshotFlow { lazyGridState.firstVisibleItemIndex },
) { isScrollInProgress, firstVisibleItemIndex ->
if (isScrollInProgress) {
metricsHolder.state?.addState(
"ForYou:TopicSelection:Scrolling",
"index=$firstVisibleItemIndex"
)
} else {
metricsHolder.state?.removeState("ForYou:TopicSelection:Scrolling")
}
}.collect()
}

TrackScrollJank(scrollableState = lazyGridState, stateName = "ForYou:TopicSelection")
keyboardsurfer marked this conversation as resolved.
Show resolved Hide resolved

LazyHorizontalGrid(
state = lazyGridState,
Expand Down