fix(ui): Fix performance of TimelineEventHandler::deduplicate_local_timeline_item
#4601
+42
−12
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch drastically improves the performance of
TimelineEventHandler::deduplicate_local_timeline_item
.Before this patch,
rfind_event_item
was used to iterate over all timeline items: for each item in reverse order, if it was an event timeline item, and if it was a local event timeline item, and if it was matching the event ID or transaction ID, then a duplicate was found.The problem is the following: all items are traversed.
However, local event timeline items are always at the back of the items. Even virtual timeline items are before local event timeline items. Thus, it is not necessary to traverse all items. It is possible to stop the iteration as soon as (i) a non event timeline item is met, or (ii) a non local event timeline item is met.
This patch updates
TimelineEventHandler::deduplicate_local_timeline_item
to replace to use ofrfind_event_item
by a custom iterator that stops as soon as a non event timeline item, or a non local event timeline item, is met, or —of course— when a local event timeline item is a duplicate.To do so,
Iterator::try_fold
is probably the best companion.Iterator::try_find
would have been nice, but it is available on nightlies, not on stable versions of Rust. However, many methods inIterator
are usingtry_fold
, likefind
or any other methods that need to do a “short-circuit”. Anyway,try_fold
works pretty nice here, and does exactly what we need.Our use of
try_fold
is to return aControlFlow<Option<(usize, TimelineItem)>, ()>
. Aftertry_fold
, we callControlFlow::break_value
, which returns anOption
. Hence the need to callOption::flatten
at the end to get a singleOption
instead of having anOption<Option<(usize, TimelineItem)>>
.I'm testing this patch with the
test_lazy_back_pagination
test in #4594. With 10_000 events in the sync, the test was taking 13s to run on my machine. With this patch, it takes 10s to run. It's a 23% improvement. Thisdeduplicate_local_timeline_item
method was taking a large part of the computation according to the profiler. With this patch, this method is barely visible in the profiler it is so small.Flamegraph of the
TimelineEventHandler::add_item
method (that callsdeduplicate_local_timeline_item
), before this patch:And after: