From 06d56b6e0f0963e2fee4430a364c8dbf6bd87530 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 3 Feb 2025 09:52:57 +0100 Subject: [PATCH] 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 https://github.com/matrix-org/matrix-rust-sdk/pull/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%. --- .../timeline/controller/observable_items.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs index b204dd67279..5d1094f4c31 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/observable_items.rs @@ -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; } } } @@ -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; } } }