Add timeline item prefetching #4399
Conversation
|
Note scrolling is still choppy in debug builds, so ideally this should be tested in nightly/release builds. |
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #4399 +/- ##
===========================================
+ Coverage 80.04% 80.05% +0.01%
===========================================
Files 2067 2067
Lines 55234 55265 +31
Branches 6768 6774 +6
===========================================
+ Hits 44210 44243 +33
+ Misses 8707 8705 -2
Partials 2317 2317 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I tried this and I'm not sure it made much/any difference? I even tried increasing this number here to 50 and it felt about the same again https://github.com/element-hq/element-x-android/pull/4399/files#diff-594345b3940e91c8414ebe16d73e789d073cb7f057f03b7e9dc4d0941bc97096R182 |
Oh, that's weird, there was definitely a difference for me in a 5 y.o. device, it wasn't always smooth when scrolling back, but it was most of the time, while it was definitely a lot more janky before. Are you enabling the persistent event cache for the tests? Also, I think it doesn't work as well as it should in encrypted rooms because the encryption layer does extra work that slows down delivering new items from the SDK to the app. I tried it in some non-encrypted rooms and I didn't see a single indicator while scrolling up, while I still saw a few in some small encrypted ones.
You need to enable those by changing this value here to |
Yep. Clearing cache first too.
With this enabled, I do see it doing something, thanks for the tip 💡 I'll have a play around with it. As I'm understanding it, it will attempt to request pagination at most one time (although in practice it's usually zero unless you scroll in just the right way) and then stop until the user scrolls again. What I would probably expect as a user (and what I'm pretty sure most of the other "mainstream" chat apps do is paginate repeatedly until a desired number of messages are ready but off-screen. Is that a desirable behaviour here? I understand that it's a tradeoff between responsiveness and conservative resource use. |
Yes, but we'll get more improvements later from the rust sdk with probably some automatic pagination! |
|
Out of my own curiosity, I had a poke at trying to make this do what I would like, and came up with this adaptation of Jorge's change from this PR frebib@6e3b479 |
|
200 items seems like way too much IMO 😅 . I guess we should play around a bit with that value. Also, removing the 'is scrolling' check seems to help, noted. I don't really understand what the |
|
@jmartinesp what are we doing with this then? Trying to improve or merge as is? |
|
I think we may want to tweak it a bit more before merging it. I'll try to get some time today to look at this again. |
This should trigger when getting close the start of the loaded timeline, making scrolling back smoother, specially when combined with the persistent event cache.
|
I think I found a sweet spot prefetching at -40 items and using |
…t, as well as an extended threshold (-40 items)
jmartinesp
force-pushed
the
feat/add-timeline-prefetching-mechanism
branch
from
847cf07 to
8f129eb
Compare
| prefetch: () -> Unit, | ||
| ) { | ||
| // We're using snapshot flows for these because using `LaunchedEffect` with `derivedState` doesn't seem to be responsive enough | ||
| val firstVisibleItemIndexFlow = snapshotFlow { |
There was a problem hiding this comment.
This looks wrong, I don't think snapshotFlow should be created in the Composable scope, but are intended to be created in the CoroutineScope they are used. I think here you are recreating flows each time
There was a problem hiding this comment.
You're right, it should be used inside the LaunchedEffect. Now I'm worried that this was actually working and the tests passed 😓 .
|
Content
This should trigger when getting close the start of the loaded timeline, making scrolling back smoother, specially when combined with the persistent event cache.
Motivation and context
Try to make scrolling a bit smoother.
Fixes #4391 (or at least, it should help).
Tests
Ideally with the persistent event cache enabled, open a room where you've already paginated for a while and start scrolling back.
The scrolling should be smoother than before as we start prefetching the data way sooner. There may still be some 'pauses' as loading new items is not instant.
Tested devices
Checklist