Skip to content

Commit afd29fb

Browse files
committed
fix(ui): Fix performance of AllRemoteEvents::(in|de)crement_all_timeline_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%.
1 parent e8edb9d commit afd29fb

File tree

1 file changed

+22
-2
lines changed

1 file changed

+22
-2
lines changed

crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,10 +1254,20 @@ impl AllRemoteEvents {
12541254
/// Shift to the right all timeline item indexes that are equal to or
12551255
/// greater than `new_timeline_item_index`.
12561256
fn increment_all_timeline_item_index_after(&mut self, new_timeline_item_index: usize) {
1257-
for event_meta in self.0.iter_mut() {
1257+
// Traverse items from back to front because:
1258+
// - if `new_timeline_item_index` is 0, we need to shift all items anyways, so
1259+
// all items must be traversed,
1260+
// - otherwise, it's unlikely we want to traverse all items: the item has been
1261+
// either inserted or pushed back, so there is no need to traverse the first
1262+
// items; we can also break the iteration as soon as all timeline item index
1263+
// after `new_timeline_item_index` has been updated.
1264+
for event_meta in self.0.iter_mut().rev() {
12581265
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
12591266
if *timeline_item_index >= new_timeline_item_index {
12601267
*timeline_item_index += 1;
1268+
} else {
1269+
// Items are ordered.
1270+
break;
12611271
}
12621272
}
12631273
}
@@ -1266,10 +1276,20 @@ impl AllRemoteEvents {
12661276
/// Shift to the left all timeline item indexes that are greater than
12671277
/// `removed_wtimeline_item_index`.
12681278
fn decrement_all_timeline_item_index_after(&mut self, removed_timeline_item_index: usize) {
1269-
for event_meta in self.0.iter_mut() {
1279+
// Traverse items from back to front because:
1280+
// - if `new_timeline_item_index` is 0, we need to shift all items anyways, so
1281+
// all items must be traversed,
1282+
// - otherwise, it's unlikely we want to traverse all items: the item has been
1283+
// either inserted or pushed back, so there is no need to traverse the first
1284+
// items; we can also break the iteration as soon as all timeline item index
1285+
// after `new_timeline_item_index` has been updated.
1286+
for event_meta in self.0.iter_mut().rev() {
12701287
if let Some(timeline_item_index) = event_meta.timeline_item_index.as_mut() {
12711288
if *timeline_item_index > removed_timeline_item_index {
12721289
*timeline_item_index -= 1;
1290+
} else {
1291+
// Items are ordered.
1292+
break;
12731293
}
12741294
}
12751295
}

0 commit comments

Comments
 (0)