WIP performance skills testing #1700
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request sets up macrobenchmarks and baseline profile generation across several sample applications (JetNews, Jetcaster, Jetchat, Reply), alongside UI performance optimizations such as marking models as @immutable, caching callback lambdas with remember, and migrating to lazy layouts. The review feedback highlights several key improvements: ensuring side-effects are not executed directly during composition, providing unique keys for lazy list items to prevent crashes and optimize performance, wrapping unmemoized method references in remembered lambdas, and utilizing the official BaselineProfileRule instead of fragile manual shell commands for profile generation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| when (dismissState.currentValue) { | ||
| SwipeToDismissBoxValue.EndToStart -> { | ||
| removeFromQueue(episode) | ||
| } | ||
|
|
||
| SwipeToDismissBoxValue.StartToEnd -> { | ||
| } | ||
| SwipeToDismissBoxValue.StartToEnd -> { | ||
| } | ||
|
|
||
| SwipeToDismissBoxValue.Settled -> { | ||
| } | ||
| SwipeToDismissBoxValue.Settled -> { | ||
| } | ||
| } |
There was a problem hiding this comment.
Executing side-effects like removeFromQueue(episode) directly during the composition phase is an anti-pattern in Jetpack Compose. Composable functions should be side-effect free, as composition can be run multiple times, cancelled, or run on different threads, leading to unpredictable behavior or infinite recomposition loops.
Wrap this side-effect in a LaunchedEffect keyed on dismissState.currentValue to ensure it only runs when the state actually changes.
LaunchedEffect(dismissState.currentValue) {
if (dismissState.currentValue == SwipeToDismissBoxValue.EndToStart) {
removeFromQueue(episode)
}
}| } | ||
|
|
||
| item { | ||
| item(key = "${content.author}_${content.timestamp}_${content.content}") { |
There was a problem hiding this comment.
Using "${content.author}_${content.timestamp}_${content.content}" as a key in LazyColumn is risky because it is not guaranteed to be unique. If a user sends duplicate messages (e.g., typing "Hello" twice in a row), or if multiple messages are sent with the same timestamp, duplicate keys will be generated. This will cause a runtime crash (IllegalArgumentException: Are you sure all items have unique keys?).
Since Message does not have a unique ID, consider appending the index to the key to guarantee uniqueness while maintaining stability.
| item(key = "${content.author}_${content.timestamp}_${content.content}") { | |
| item(key = "${content.author}_${content.timestamp}_${index}") { |
| onPostTapped, | ||
| favorites, | ||
| onToggleFavorite, | ||
| items(postsFeed.recommendedPosts) { post -> |
There was a problem hiding this comment.
In LazyColumn, it is highly recommended to specify a unique key for items. This helps Jetpack Compose maintain the scroll position and state of items when the list changes (e.g., when a post is favorited or the list is refreshed), preventing unnecessary recompositions and improving scroll performance.
| items(postsFeed.recommendedPosts) { post -> | |
| items(postsFeed.recommendedPosts, key = { it.id }) { post -> |
| } | ||
| if (postsFeed.recentPosts.isNotEmpty()) { | ||
| item { PostListHistorySection(postsFeed.recentPosts, onPostTapped) } | ||
| items(postsFeed.recentPosts) { post -> |
There was a problem hiding this comment.
Specify a unique key for the recent posts items to optimize scroll performance and prevent unnecessary recompositions when the list updates.
| items(postsFeed.recentPosts) { post -> | |
| items(postsFeed.recentPosts, key = { it.id }) { post -> |
| horizontalArrangement = Arrangement.spacedBy(8.dp), | ||
| ) { | ||
| for (post in posts) { | ||
| items(posts) { post -> |
There was a problem hiding this comment.
Specify a unique key for the popular posts items in the LazyRow to optimize horizontal scrolling performance and prevent unnecessary recompositions.
| items(posts) { post -> | |
| items(posts, key = { it.id }) { post -> |
| // Main Content | ||
| val snackBarText = stringResource(id = R.string.episode_added_to_your_queue) | ||
| val showHomeCategoryTabs = featuredPodcasts.isNotEmpty() && homeCategories.isNotEmpty() | ||
| val onHomeActionRemembered = remember(onHomeAction, snackbarHostState, snackBarText, coroutineScope) { |
There was a problem hiding this comment.
The onHomeAction lambda is used as a key for remember here. However, in the parent composable HomeScreenReady, it is passed as a method reference: onHomeAction = viewModel::onHomeAction.
In Kotlin and Jetpack Compose, method references like viewModel::onHomeAction are not memoized and allocate a new function instance on every recomposition. This means onHomeAction will change on every recomposition, defeating this remember block and causing onHomeActionRemembered to be re-created unnecessarily.
To fix this, wrap the call in a remembered lambda in the parent composable HomeScreenReady (similar to how it's done in MainActivity.kt for Reply or HomeScreen in JetNews):
onHomeAction = remember(viewModel) { { viewModel.onHomeAction(it) } }| private val targetPackageName = "com.example.jetnews" | ||
|
|
||
| @Test | ||
| fun generateProfile() { |
There was a problem hiding this comment.
Using manual shell commands to reset, flush, and read profiles from /data/misc/profman/ is fragile and highly likely to fail on non-rooted devices or standard emulators due to permission restrictions.
Since androidx.benchmark:benchmark-macro-junit4 is already a dependency in this module, you should use the official BaselineProfileRule (as done in Jetchat's BaselineProfileGenerator.kt). This completely avoids manual shell commands, is officially supported, and works reliably across devices.
1 participant
WIP