From ee2e3db92884e2ca1f996cc6e9b965fee571d154 Mon Sep 17 00:00:00 2001 From: Ben Weiss Date: Mon, 10 Oct 2022 11:23:34 +0100 Subject: [PATCH] Add more jank tracking --- .../samples/apps/nowinandroid/ui/NiaAppState.kt | 4 ++-- .../apps/nowinandroid/core/ui/JankStatsExtensions.kt | 11 +++++++---- .../apps/nowinandroid/feature/author/AuthorScreen.kt | 7 ++++++- .../nowinandroid/feature/bookmarks/BookmarksScreen.kt | 5 +++++ .../apps/nowinandroid/feature/foryou/ForYouScreen.kt | 8 +++----- .../nowinandroid/feature/interests/InterestsScreen.kt | 4 ++-- .../apps/nowinandroid/feature/topic/TopicScreen.kt | 5 +++++ 7 files changed, 30 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt index bf80984fa..b4d28c251 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt @@ -33,7 +33,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.Drawable import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination -import com.google.samples.apps.nowinandroid.core.ui.JankMetricDisposableEffect +import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank import com.google.samples.apps.nowinandroid.feature.bookmarks.R as bookmarksR import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.BookmarksDestination import com.google.samples.apps.nowinandroid.feature.foryou.R as forYouR @@ -141,7 +141,7 @@ class NiaAppState( */ @Composable private fun NavigationTrackingSideEffect(navController: NavHostController) { - JankMetricDisposableEffect(navController) { metricsHolder -> + TrackDisposableJank(navController) { metricsHolder -> val listener = NavController.OnDestinationChangedListener { _, destination, _ -> metricsHolder.state?.putState("Navigation", destination.route.toString()) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt index ef7375f48..7a3500fbb 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt @@ -46,10 +46,10 @@ fun rememberMetricsStateHolder(): Holder { /** * Convenience function to work with [PerformanceMetricsState] state. The side effect is * re-launched if any of the [keys] value is not equal to the previous composition. - * @see JankMetricDisposableEffect if you need to work with DisposableEffect to cleanup added state. + * @see TrackDisposableJank if you need to work with DisposableEffect to cleanup added state. */ @Composable -fun JankMetricEffect( +fun TrackJank( vararg keys: Any?, reportMetric: suspend CoroutineScope.(state: Holder) -> Unit ) { @@ -64,7 +64,7 @@ fun JankMetricEffect( * The side effect is re-launched if any of the [keys] value is not equal to the previous composition. */ @Composable -fun JankMetricDisposableEffect( +fun TrackDisposableJank( vararg keys: Any?, reportMetric: DisposableEffectScope.(state: Holder) -> DisposableEffectResult ) { @@ -74,9 +74,12 @@ fun JankMetricDisposableEffect( } } +/** + * Track jank while scrolling anything that's scrollable. + */ @Composable fun TrackScrollJank(scrollableState: ScrollableState, stateName: String) { - JankMetricEffect(scrollableState) { metricsHolder -> + TrackJank(scrollableState) { metricsHolder -> snapshotFlow { scrollableState.isScrollInProgress }.collect { isScrollInProgress -> metricsHolder.state?.apply { if (isScrollInProgress) { diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt index 7cd32a64b..9c53701de 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.layout.windowInsetsTopHeight import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope +import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape import androidx.compose.material.icons.Icons.Filled import androidx.compose.material.icons.filled.ArrowBack @@ -59,6 +60,7 @@ 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.ui.DevicePreviews +import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems @OptIn(ExperimentalLifecycleComposeApi::class) @@ -91,9 +93,12 @@ internal fun AuthorScreen( onBookmarkChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { + val scrollableState = rememberLazyListState() + TrackScrollJank(scrollableState = scrollableState, stateName = "author:column") LazyColumn( modifier = modifier, - horizontalAlignment = Alignment.CenterHorizontally + horizontalAlignment = Alignment.CenterHorizontally, + state = scrollableState ) { item { Spacer(Modifier.windowInsetsTopHeight(WindowInsets.safeDrawing)) diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt index cdd1e2575..94fd35b66 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt @@ -31,6 +31,7 @@ import androidx.compose.foundation.layout.wrapContentSize import androidx.compose.foundation.lazy.grid.GridCells.Adaptive import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyVerticalGrid +import androidx.compose.foundation.lazy.grid.rememberLazyGridState import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Scaffold import androidx.compose.material3.TopAppBarDefaults @@ -48,6 +49,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadi import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopAppBar import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState +import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @@ -87,11 +89,14 @@ fun BookmarksScreen( containerColor = Color.Transparent, contentWindowInsets = WindowInsets(0, 0, 0, 0) ) { innerPadding -> + val scrollableState = rememberLazyGridState() + TrackScrollJank(scrollableState = scrollableState, stateName = "bookmarks:grid") LazyVerticalGrid( columns = Adaptive(300.dp), contentPadding = PaddingValues(16.dp), horizontalArrangement = Arrangement.spacedBy(32.dp), verticalArrangement = Arrangement.spacedBy(24.dp), + state = scrollableState, modifier = modifier .fillMaxSize() .testTag("bookmarks:feed") diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index 1fce535fd..a089b16e4 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -175,10 +175,8 @@ fun ForYouScreen( } } - val tag = "forYou:feed" - - val lazyGridState = rememberLazyGridState() - TrackScrollJank(scrollableState = lazyGridState, stateName = tag) + val state = rememberLazyGridState() + TrackScrollJank(scrollableState = state, stateName = "forYou:feed") val notConnected = stringResource(R.string.for_you_not_connected) LaunchedEffect(isOffline) { @@ -198,7 +196,7 @@ fun ForYouScreen( .consumedWindowInsets(innerPadding) .fillMaxSize() .testTag("forYou:feed"), - state = lazyGridState + state = state ) { interestsSelection( interestsSelectionState = interestsSelectionState, diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt index b60fddb6a..551ece704 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt @@ -41,7 +41,7 @@ import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.previewAuthors import com.google.samples.apps.nowinandroid.core.model.data.previewTopics import com.google.samples.apps.nowinandroid.core.ui.DevicePreviews -import com.google.samples.apps.nowinandroid.core.ui.JankMetricDisposableEffect +import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank @OptIn(ExperimentalLifecycleComposeApi::class) @Composable @@ -65,7 +65,7 @@ fun InterestsRoute( modifier = modifier ) - JankMetricDisposableEffect(tabState) { metricsHolder -> + TrackDisposableJank(tabState) { metricsHolder -> metricsHolder.state?.putState("Interests:TabState", "currentIndex:${tabState.currentIndex}") onDispose { diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index 942922720..ccd93f8a5 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -30,6 +30,7 @@ import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.layout.windowInsetsTopHeight import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope +import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.material.icons.Icons.Filled import androidx.compose.material.icons.filled.ArrowBack import androidx.compose.material3.Icon @@ -56,6 +57,7 @@ import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource 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.DevicePreviews +import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems import com.google.samples.apps.nowinandroid.feature.topic.R.string import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading @@ -90,7 +92,10 @@ internal fun TopicScreen( onBookmarkChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { + val state = rememberLazyListState() + TrackScrollJank(scrollableState = state, stateName = "topic:screen") LazyColumn( + state = state, modifier = modifier, horizontalAlignment = Alignment.CenterHorizontally ) {