Skip to content

Commit 95978f1

Browse files
greyson-signalalan-signal
authored andcommitted
Possible fix to getting thrown to the bottom while reading unreads.
Shoutout to @fumiakiy for the excellent research here! Sometimes we get thrown to the bottom of the list (or other list locations) when reading content in the middle of the list. Most often, this happens when you have a lot of unread messages and you open the conversation. FixedSizePagingController#onDataNeededAroundIndex() can be called very fast in rapid succession, and we use the DataStatus class for bookkeeping to know which requests are in-flight. We then make those requests in LIFO order in order to make sure that the data visible on screen now gets the highest priority. ...But in practice, that LIFO ordering can make things a little screwy. Imagine we called onDataNeedAroundIndex() 50 times in rapid succession (1, 2..., 50). Each time it's called, we generate a range and mark that range as being fetched in DataStatus. That could mean that the latest request for index 50 might only have, like, 1 item in it, because a previously-enqueued fetch already got assigned most of it's data. BUT we execute the nearly-empty request for index 50 first because of the LIFO ordering. We give that data to RecyclerView first, and it doesn't like that at all, and it jumps to weird places because we gave it mostly null values, which are rendered as placeholder values (which are smaller than real cells). So then, when we give it the real data right after, its position is all off. I switched to a serial executor. That prevents us from giving back weird lists. The consequence is that if you scroll super fast, you run the risk of the executor getting 'backed up' fetching data that's offscreen. However, in practice, I couldn't trigger this. We'll see how it goes. I think the true solution is a smarter way of fetching and ordering requests, but that gets to be really tricky from a threading perspective, and I'd rather keep things simple.
1 parent d055bba commit 95978f1

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

paging/lib/src/main/java/org/signal/paging/FixedSizePagingController.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class FixedSizePagingController<E> implements PagingController {
2222

2323
private static final String TAG = FixedSizePagingController.class.getSimpleName();
2424

25-
private static final Executor FETCH_EXECUTOR = SignalExecutors.newFixedLifoThreadExecutor("signal-FixedSizePagingController", 1, 1);
25+
private static final Executor FETCH_EXECUTOR = SignalExecutors.newCachedSingleThreadExecutor("signal-FixedSizePagingController");
2626
private static final boolean DEBUG = false;
2727

2828
private final PagedDataSource<E> dataSource;
@@ -49,7 +49,7 @@ class FixedSizePagingController<E> implements PagingController {
4949
/**
5050
* We assume this method is always called on the same thread, so we can read our
5151
* {@code loadState} and construct the parameters of a fetch request. That fetch request can
52-
* then be performed on separate single-thread LIFO executor.
52+
* then be performed on separate single-thread executor.
5353
*/
5454
@Override
5555
public void onDataNeededAroundIndex(int aroundIndex) {

0 commit comments

Comments
 (0)