Skip to content

Commit

Permalink
fix(ui): Fix performance of `AllRemoteEvents::(in|de)crement_all_time…
Browse files Browse the repository at this point in the history
…line_item_index_after`.

This patch fixes the performance of
`AllRemoteEvents::increment_all_timeline_item_index_after` and
`decrment_all_timeline_item_index_after`.

It appears that the code was previously iterating over all items. This
is a waste of time. This patch updates the code to iterate over all
items in reverse order:

- if `new_timeline_item_index` is 0, we need to shift all items anyways,
  so all items must be traversed, the iterator direction doesn't matter,
- otherwise, it's unlikely we want to traverse all items: the item has
  been either inserted or pushed back, so there is no need to traverse
  the first items; we can also break the iteration as soon as all
  timeline item index after `new_timeline_item_index` has been updated.

I'm testing this patch with the `test_lazy_back_pagination` test in
#4594. With 10_000
events in the sync, the `ObservableItems::push_back` method (that uses
`AllRemoteEvents::increment_all_timeline_item_index_after`) was taking
7% of the whole execution time. With this patch, it takes 0.7%.
  • Loading branch information
Hywan committed Feb 3, 2025
1 parent df3cb00 commit 9ab5547
Showing 1 changed file with 22 additions and 2 deletions.
24 changes: 22 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1254,10 +1254,20 @@ impl AllRemoteEvents {
/// Shift to the right all timeline item indexes that are equal to or
/// greater than `new_timeline_item_index`.
fn increment_all_timeline_item_index_after(&mut self, new_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
// Traverse items from back to front because:
// - if `new_timeline_item_index` is 0, we need to shift all items anyways, so
// all items must be traversed,
// - otherwise, it's unlikely we want to traverse all items: the item has been
// either inserted or pushed back, so there is no need to traverse the first
// items; we can also break the iteration as soon as all timeline item index
// after `new_timeline_item_index` has been updated.
for event_meta in self.0.iter_mut().rev() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index >= new_timeline_item_index {
*timeline_item_index += 1;
} else {
// Items are ordered.
break;
}
}
}
Expand All @@ -1266,10 +1276,20 @@ impl AllRemoteEvents {
/// Shift to the left all timeline item indexes that are greater than
/// `removed_wtimeline_item_index`.
fn decrement_all_timeline_item_index_after(&mut self, removed_timeline_item_index: usize) {
for event_meta in self.0.iter_mut() {
// Traverse items from back to front because:
// - if `new_timeline_item_index` is 0, we need to shift all items anyways, so
// all items must be traversed,
// - otherwise, it's unlikely we want to traverse all items: the item has been
// either inserted or pushed back, so there is no need to traverse the first
// items; we can also break the iteration as soon as all timeline item index
// after `new_timeline_item_index` has been updated.
for event_meta in self.0.iter_mut().rev() {
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
if *timeline_item_index > removed_timeline_item_index {
*timeline_item_index -= 1;
} else {
// Items are ordered.
break;
}
}
}
Expand Down

0 comments on commit 9ab5547

Please sign in to comment.