Fixed
Status Update
No update yet.
Comments
cl...@google.com
ro...@gmail.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
https://android-review.googlesource.com/1123258
https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
ro...@gmail.com
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
https://android-review.googlesource.com/1288456
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
ro...@gmail.com
Great! Thank you! We can expect this optimization in alpha04 (next Wednesday), I assume?
ae...@google.com
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit dc602125a9e5fc15a37ba4fa7610998a94ead29f
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Fri Sep 25 21:14:09 2020
Do less work in SubcomposeLayout when subcompose called with the same lambda
If the lambda object for the slot didn't change and there were no pending recompositons we can skip the whole subcompositions invocation as it will result in unchanged tree. Starting subcomposition is a heavy weight operation so this change allows to improve the measure benchmarks for LazyColumn scrolling by 88 percents.
To achieve it I added a hasPendingChanges() method for Composition class.
Bug: 168293643
Bug: 167972292
Bug: 165028371
Relnote: The scrolling performance of LazyColumn/Row is improved by doing less work in subcomposition on every scroll. The new hasInvalidations() method was added for Composition class. hasPendingChanges() method from Recomposer was renamed to hasInvalidations()
Test: This is not changing the behavior so I can't cover it with a unit test, but it dramatically improves the benchmark. So if this optimization will stop working in the future we will get a regression.
Change-Id: Ib2f324dd6845fd83321e0d4f3fa6e502c346dbc3
M compose/runtime/runtime/api/current.txt
M compose/runtime/runtime/api/public_plus_experimental_current.txt
M compose/runtime/runtime/api/restricted_current.txt
M compose/runtime/runtime/src/androidAndroidTest/kotlin/androidx/compose/runtime/AmbientTests.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
M compose/test-utils/src/androidMain/kotlin/androidx/compose/testutils/AndroidComposeTestCaseRunner.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/ComposeView.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/Wrapper.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt
M ui/ui-test/src/androidMain/kotlin/androidx/ui/test/android/ComposeIdlingResource.kt
M ui/ui-test/src/androidMain/kotlin/androidx/ui/test/android/CompositionAwaiter.kt
M ui/ui-test/src/desktopMain/kotlin/androidx/ui/test/DesktopComposeTestRule.kt
https://android-review.googlesource.com/1439054
Branch: androidx-master-dev
commit dc602125a9e5fc15a37ba4fa7610998a94ead29f
Author: Andrey Kulikov <andreykulikov@google.com>
Date: Fri Sep 25 21:14:09 2020
Do less work in SubcomposeLayout when subcompose called with the same lambda
If the lambda object for the slot didn't change and there were no pending recompositons we can skip the whole subcompositions invocation as it will result in unchanged tree. Starting subcomposition is a heavy weight operation so this change allows to improve the measure benchmarks for LazyColumn scrolling by 88 percents.
To achieve it I added a hasPendingChanges() method for Composition class.
Bug: 168293643
Bug: 167972292
Bug: 165028371
Relnote: The scrolling performance of LazyColumn/Row is improved by doing less work in subcomposition on every scroll. The new hasInvalidations() method was added for Composition class. hasPendingChanges() method from Recomposer was renamed to hasInvalidations()
Test: This is not changing the behavior so I can't cover it with a unit test, but it dramatically improves the benchmark. So if this optimization will stop working in the future we will get a regression.
Change-Id: Ib2f324dd6845fd83321e0d4f3fa6e502c346dbc3
M compose/runtime/runtime/api/current.txt
M compose/runtime/runtime/api/public_plus_experimental_current.txt
M compose/runtime/runtime/api/restricted_current.txt
M compose/runtime/runtime/src/androidAndroidTest/kotlin/androidx/compose/runtime/AmbientTests.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Composition.kt
M compose/runtime/runtime/src/commonMain/kotlin/androidx/compose/runtime/Recomposer.kt
M compose/test-utils/src/androidMain/kotlin/androidx/compose/testutils/AndroidComposeTestCaseRunner.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/ComposeView.kt
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/platform/Wrapper.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/layout/SubcomposeLayout.kt
M ui/ui-test/src/androidMain/kotlin/androidx/ui/test/android/ComposeIdlingResource.kt
M ui/ui-test/src/androidMain/kotlin/androidx/ui/test/android/CompositionAwaiter.kt
M ui/ui-test/src/desktopMain/kotlin/androidx/ui/test/DesktopComposeTestRule.kt
an...@google.com
There is one optimization for LazyColumn scrolling has been merged in alpha04 and another is upcoming in alpha05.
One more way to optimize it is to add Modifier.drawLayer() directly on a LazyColumn item so it is cheaper to offset such items(feature request to do it automatically:https://issuetracker.google.com/issues/170296989 ).
In your specific example you will need to add Modifier.drawLayer() for ConstraintLayout herehttps://gist.github.com/Rokata9/f9252974343addc0fab2e2c93104382d , or wrap the content of WallpostView into another Column and apply the modifier on it here https://gist.github.com/Rokata9/197100002502a8490e2fe246811c6ec7 .
I will close the bug as fixed for now, Please feel free to comment again if after migrating to alpha05 and adding Modifier.drawLayer() for an item the scrolling performance is still unacceptable. Thanks!
One more way to optimize it is to add Modifier.drawLayer() directly on a LazyColumn item so it is cheaper to offset such items(feature request to do it automatically:
In your specific example you will need to add Modifier.drawLayer() for ConstraintLayout here
I will close the bug as fixed for now, Please feel free to comment again if after migrating to alpha05 and adding Modifier.drawLayer() for an item the scrolling performance is still unacceptable. Thanks!
ro...@gmail.com
Hi, I just tested with Modifiler.drawLayer added arround a wrappning Column (tested with Row/Column version). This didn't make any difference, but as you mention in your previous post, the big optimization (88%, I read) is about to come in alpha05, right? And I assume there isn't much difference in performance between using Row/Column and ConstraintLayout (unlike nested LinearLayout and ConstraintLayout for views)? Correct me if I'm wrong.
Thanks a lot, looking forward to alpha05! ;)
ro...@gmail.com
As for re-composition, yes, when I scroll I can see in Logcat that I'm getting the message I added in the composable body several times (probably it's for each item that's coming into view).
And my Wallpost class is defined like this (you can also check the sample project I attached in the beginning of the discussion -
package com.example.wpcomposable
import com.example.wpcomposable.utils.DateTimeUtils
import java.util.*
import kotlin.collections.ArrayList
import kotlin.time.ExperimentalTime
data class Wallpost(
var id: String = "",
var content: String ="",
var datePosted: Date = Date(),
var username: String = "",
var comments: ArrayList<WallpostComment> = ArrayList(),
var liked: Boolean = false
) {
val headerText: String get() {
return "${this.username} on ${this.datePosted}"
}
val linksText: String get() {
return "Show previous comments (${this.comments.size} of 5) ---- Reply ----"
}
@ExperimentalTime
val timeEllapsed: String get() {
return DateTimeUtils.periodFromNowToString(this.datePosted)
}
}
an...@google.com
oh, right. if you see such log message every time a new item became visible after you scrolled enough it is completely expected. the behavior we had before alpha04 is that everyitem was recomposed on every scroll(even just few pixels).
Columns and Row are currently a bit more performant than ConstraintLayout. So I would suggest to keep the version with Columns, add Modifier.drawLayer() on a first layout inside the item and wait for alpha05. Feel free to share you findings once you try it when this version will be released.
Thanks
Columns and Row are currently a bit more performant than ConstraintLayout. So I would suggest to keep the version with Columns, add Modifier.drawLayer() on a first layout inside the item and wait for alpha05. Feel free to share you findings once you try it when this version will be released.
Thanks
ro...@gmail.com
Thanks, for now it seems Modifier.drawLayer doesn't improve things much - as you suggested I added at the top level of the composable body another wrapper Column with this modifier. But I'm looking forward to alpha05 and will write back as soon as I've tested with it. :)
ro...@gmail.com
I tried alpha06, there's definitely improvement. But still scroll speed is far from that of a RecyclerView. I can no longer consider it "freezing", but it's laggy when scrolling a little bit faster. I understand it's probably too early to measure the final version's performance, so I can comment again let's say after some last beta or RC version. ;) Thanks!
se...@gmail.com
Comment has been deleted.
se...@gmail.com
Any update on this one i have a list of more than 5k and it lags when i passed 3k items
an...@google.com
As you only see it became slower after you scroll a lot this seems like some kind of memory leak and could be related to this bug -
Description
That original XML view is ConstraintLayout so initially I migrated it to the equivalent ConstraintLayout composable. I also tried Row/Column approach, too. Both versions can be found in the links below. The performance issue is expressed in slow scrolling - the initial scroll seems to freeze for a few seconds before moving just to the second item (in total I have 128 items). So I think there's also a problem with scroll speed too, that is, the amount of views which are scrolled with single finger gesture.
Compared to the RecyclerView approach, even when I do fast scrolling everything is smooth and a single scroll goes through a lot more items in total (arround 20-30 items).
---
Setup information:
Jetpack compose 0.1.0-dev16
Phone: Nexus5X
Android 8.1 Oreo
Android Studio 4.2, latest Canary build
---
Following a discussion on #compose channel in Slack, I'm posting relevant code:
The item which is repeated, here are both ConstraintLayout and Row/Column versions:
ConstraintLayout -
Row/Column -
And the custom composables which are used (button and card image):
---
In the attachments you can find the images uesd: no_avatar.png is used in the RoundedImage composable, whereas user.xml is used with the standard Image composable as vector resource.