Commit 8d754af8d109
tarball6 files changed+201-83
@@ -256,6 +256,7 @@ impl TimelineBuilder {
256256
257257
let crypto_drop_handles = spawn_crypto_tasks(controller.clone()).await;258258
259
+ let view = room_event_cache.view();259260
let timeline = Timeline {260261
controller,261262
event_cache: room_event_cache,@@ -266,6 +267,7 @@ impl TimelineBuilder {
266267
_local_echo_listener_handle: local_echo_listener_handle,267268
_focus_drop_handle: focus_task,268269
_event_cache_drop_handle: event_cache_drop,270
+ _view: view,269271
}),270272
};271273
@@ -29,7 +29,9 @@ use matrix_sdk::{
2929
Result,3030
attachment::{AttachmentInfo, Thumbnail},3131
deserialized_responses::TimelineEvent,32
- event_cache::{EventCacheDropHandles, EventFocusThreadMode, RoomEventCache},32
+ event_cache::{33
+ EventCacheDropHandles, EventFocusThreadMode, RoomEventCache, RoomEventCacheView,34
+ },3335
room::{3436
Receipts, Room,3537
edit::EditedContent,@@ -932,6 +934,7 @@ struct TimelineDropHandle {
932934
_event_cache_drop_handle: Arc<EventCacheDropHandles>,933935
_focus_drop_handle: Option<BackgroundTaskHandle>,934936
_crypto_drop_handles: CryptoDropHandles,937
+ _view: RoomEventCacheView,935938
}936939
937940
#[cfg(not(target_family = "wasm"))]@@ -1956,16 +1956,6 @@ async fn test_room_sorting() -> Result<(), Error> {
19561956
end;19571957
};19581958
1959
- // All rooms get new messages, so their entries will get updates because of read1960
- // receipt updates.1961
- //1962
- // Starting with r0.1963
- assert_entries_batch! {1964
- [stream]1965
- set [ 0 ] [ "!r0:bar.org" ];1966
- end;1967
- };1968
-19691959
// Now we have:19701960
//19711961
// | index | room ID | recency | name |@@ -1983,10 +1973,13 @@ async fn test_room_sorting() -> Result<(), Error> {
19831973
end;19841974
};19851975
1986
- // Read receipt update for r1.1976
+ // All rooms get new messages, so their entries will get updates because of read1977
+ // receipt updates, each landing once the next room has moved.1978
+ //1979
+ // Starting with r0.19871980
assert_entries_batch! {19881981
[stream]1989
- set [ 1 ] [ "!r1:bar.org" ];1982
+ set [ 0 ] [ "!r0:bar.org" ];19901983
end;19911984
};19921985
@@ -2007,6 +2000,13 @@ async fn test_room_sorting() -> Result<(), Error> {
20072000
end;20082001
};20092002
2003
+ // Read receipt update for r1.2004
+ assert_entries_batch! {2005
+ [stream]2006
+ set [ 2 ] [ "!r1:bar.org" ];2007
+ end;2008
+ };2009
+20102010
// Read receipt update for r2.20112011
assert_entries_batch! {20122012
[stream]@@ -2126,12 +2126,12 @@ async fn test_room_sorting() -> Result<(), Error> {
21262126
// Rooms are individually updated.21272127
assert_entries_batch! {21282128
[stream]2129
- set [ 0 ] [ "!r3:bar.org" ];2129
+ set [ 2 ] [ "!r6:bar.org" ];21302130
end;21312131
};21322132
assert_entries_batch! {21332133
[stream]2134
- set [ 2 ] [ "!r6:bar.org" ];2134
+ set [ 0 ] [ "!r3:bar.org" ];21352135
end;21362136
};21372137
assert_entries_batch! {@@ -26,7 +26,7 @@ use std::{
2626
fmt,2727
sync::{2828
Arc,29
- atomic::{AtomicU64, Ordering},29
+ atomic::{AtomicU64, AtomicUsize, Ordering},3030
},3131
};3232
@@ -119,6 +119,17 @@ impl RoomEventCache {
119119
Ok(state.room_linked_chunk().events().map(|(_position, item)| item.clone()).collect())120120
}121121
122
+ /// Mark this room as on screen for as long as the returned value lives, so123
+ /// sync handling takes its updates ahead of other rooms'.124
+ pub fn view(&self) -> RoomEventCacheView {125
+ self.inner.viewers.fetch_add(1, Ordering::SeqCst);126
+ RoomEventCacheView { viewers: self.inner.viewers.clone() }127
+ }128
+129
+ pub(in super::super) fn is_viewed(&self) -> bool {130
+ self.inner.viewers.load(Ordering::SeqCst) > 0131
+ }132
+122133
/// Subscribe to this room updates, after getting the initial list of123134
/// events.124135
///@@ -458,6 +469,18 @@ impl RoomEventCache {
458469
}459470
}460471
472
+/// Keeps a room ahead in sync handling while it lives, see [`RoomEventCache::view`].473
+#[derive(Debug)]474
+pub struct RoomEventCacheView {475
+ viewers: Arc<AtomicUsize>,476
+}477
+478
+impl Drop for RoomEventCacheView {479
+ fn drop(&mut self) {480
+ self.viewers.fetch_sub(1, Ordering::SeqCst);481
+ }482
+}483
+461484
/// The (non-cloneable) details of the `RoomEventCache`.462485
pub(super) struct RoomEventCacheInner {463486
history_coordinator: std::sync::OnceLock<mpsc::Sender<history::HistoryRequest>>,@@ -483,6 +506,9 @@ pub(super) struct RoomEventCacheInner {
483506
484507
/// Update sender for this room.485508
update_sender: RoomEventCacheUpdateSender,509
+510
+ /// How many [`RoomEventCacheView`]s are alive.511
+ viewers: Arc<AtomicUsize>,486512
}487513
488514
impl RoomEventCacheInner {@@ -507,6 +533,7 @@ impl RoomEventCacheInner {
507533
pagination_batch_token_notifier: Default::default(),508534
auto_shrink_sender,509535
shared_pagination_status,536
+ viewers: Default::default(),510537
}511538
}512539
@@ -28,7 +28,7 @@
2828
#![forbid(missing_docs)]2929
3030
use std::{31
- collections::HashMap,31
+ collections::{BTreeMap, HashMap},3232
fmt,3333
sync::{3434
Arc, Mutex as StdMutex, OnceLock, PoisonError, RwLock as StdRwLock, RwLockReadGuard,@@ -38,12 +38,12 @@ use std::{
3838
time::Duration,3939
};4040
41
-use futures_util::future::try_join_all;41
+use futures_util::{StreamExt, future::try_join_all};4242
use matrix_sdk_base::{4343
cross_process_lock::CrossProcessLockError,4444
event_cache::store::{EventCacheStoreError, EventCacheStoreLock, EventCacheStoreLockState},4545
linked_chunk::lazy_loader::LazyLoaderError,46
- sync::RoomUpdates,46
+ sync::{JoinedRoomUpdate, LeftRoomUpdate, RoomUpdates},4747
task_monitor::BackgroundTaskHandle,4848
timer,4949
};@@ -78,7 +78,7 @@ pub use caches::{
7878
room::{7979
HistoryGapFillOutcome, HistoryGapPosition, RecentHistoryContinuity, RecentHistoryOutcome,8080
RoomEventCache, RoomEventCacheGenericUpdate, RoomEventCacheSubscriber,81
- RoomEventCacheUpdate, pagination::RoomPagination,81
+ RoomEventCacheUpdate, RoomEventCacheView, pagination::RoomPagination,8282
},8383
thread::pagination::ThreadPagination,8484
};@@ -599,6 +599,60 @@ struct EventCacheInner {
599599
600600
type AutoShrinkChannelPayload = OwnedRoomId;601601
602
+// NOTE: We tried to make this concurrent at some point, but it turned out to be603
+// a performance regression, even for large sync updates. Lacking time604
+// to investigate, this code remains sequential for now. See also605
+// https://github.com/matrix-org/matrix-rust-sdk/pull/5426.606
+#[cfg(not(target_family = "wasm"))]607
+const ROOM_UPDATES_AT_ONCE: usize = 1;608
+609
+/// How many rooms' sync updates are handled at once. On wasm a room's handling is610
+/// mostly waiting on IndexedDB, which other rooms can use.611
+#[cfg(target_family = "wasm")]612
+const ROOM_UPDATES_AT_ONCE: usize = 4;613
+614
+/// One room's part of a [`RoomUpdates`].615
+enum RoomUpdate {616
+ Left(LeftRoomUpdate),617
+ Joined(JoinedRoomUpdate),618
+}619
+620
+/// Sync updates taken off the broadcast and not handled yet, per room, in the621
+/// order sync sent them.622
+#[derive(Default)]623
+struct PendingRoomUpdates {624
+ rooms: BTreeMap<OwnedRoomId, Vec<RoomUpdate>>,625
+626
+ /// Broadcast messages folded in since `rooms` was last empty.627
+ folded: u64,628
+629
+ /// Broadcast messages lost to a lag since `rooms` was last empty.630
+ lagged: u64,631
+}632
+633
+impl PendingRoomUpdates {634
+ fn fold(&mut self, updates: RoomUpdates) {635
+ self.folded += 1;636
+ for (room_id, update) in updates.left {637
+ self.rooms.entry(room_id).or_default().push(RoomUpdate::Left(update));638
+ }639
+ for (room_id, update) in updates.joined {640
+ self.rooms.entry(room_id).or_default().push(RoomUpdate::Joined(update));641
+ }642
+ // Invited rooms.643
+ // TODO: we don't anything with `updates.invite` at this point.644
+ }645
+646
+ fn is_empty(&self) -> bool {647
+ self.rooms.is_empty()648
+ }649
+650
+ /// How many broadcast messages are fully handled, once `rooms` is empty.651
+ fn take_handled(&mut self) -> u64 {652
+ std::mem::take(&mut self.folded) + std::mem::take(&mut self.lagged)653
+ }654
+}655
+602656
impl EventCacheInner {603657
fn client(&self) -> Result<Client> {604658
self.client.get().ok_or(EventCacheError::ClientDropped)@@ -678,49 +732,69 @@ impl EventCacheInner {
678732
self.multiple_room_updates_lock.lock().await679733
};680734
681
- // NOTE: We tried to make this concurrent at some point, but it turned out to be682
- // a performance regression, even for large sync updates. Lacking time683
- // to investigate, this code remains sequential for now. See also684
- // https://github.com/matrix-org/matrix-rust-sdk/pull/5426.735
+ let mut pending = PendingRoomUpdates::default();736
+ pending.fold(updates);737
+ while !pending.is_empty() {738
+ self.handle_pending_room_updates(&mut pending).await;739
+ }685740
686
- // Left rooms.687
- for (room_id, left_room_update) in updates.left {688
- let caches = match self.all_caches_for_room(&room_id).await {689
- Ok(caches) => caches,690
- Err(err) => {691
- self.drop_room_update(&room_id, err);692
- continue;693
- }694
- };741
+ Ok(())742
+ }695743
696
- if let Err(err) = caches.handle_left_room_update(left_room_update).await {697
- // Non-fatal error, try to continue to the next room.698
- error!("handling left room update: {err}");699
- }700
- }744
+ /// Handle the next few rooms of `pending`, those on screen first, with the745
+ /// `multiple_room_updates_lock` held.746
+ async fn handle_pending_room_updates(&self, pending: &mut PendingRoomUpdates) {747
+ let viewed: Vec<OwnedRoomId> = {748
+ let by_room = self.by_room.read().await;749
+ pending750
+ .rooms751
+ .keys()752
+ .filter(|room_id| {753
+ by_room.get(*room_id).is_some_and(|caches| caches.room.is_viewed())754
+ })755
+ .take(ROOM_UPDATES_AT_ONCE)756
+ .cloned()757
+ .collect()758
+ };759
+ // rooms on screen get a round of their own, so a slow cold room can't hold them back760
+ let round: Vec<OwnedRoomId> = if viewed.is_empty() {761
+ pending.rooms.keys().take(ROOM_UPDATES_AT_ONCE).cloned().collect()762
+ } else {763
+ viewed764
+ };765
+ let round: Vec<_> =766
+ round.into_iter().filter_map(|room_id| pending.rooms.remove_entry(&room_id)).collect();701767
702
- // Joined rooms.703
- for (room_id, joined_room_update) in updates.joined {704
- trace!(?room_id, "Handling a `JoinedRoomUpdate`");768
+ futures_util::stream::iter(round)769
+ .for_each_concurrent(ROOM_UPDATES_AT_ONCE, |(room_id, updates)| async move {770
+ self.handle_room_queue(&room_id, updates).await;771
+ })772
+ .await;773
+ }705774
706
- let caches = match self.all_caches_for_room(&room_id).await {707
- Ok(caches) => caches,708
- Err(err) => {709
- self.drop_room_update(&room_id, err);710
- continue;775
+ /// Handle one room's updates, in the order sync sent them.776
+ async fn handle_room_queue(&self, room_id: &RoomId, updates: Vec<RoomUpdate>) {777
+ let caches = match self.all_caches_for_room(room_id).await {778
+ Ok(caches) => caches,779
+ Err(err) => {780
+ self.drop_room_update(room_id, err);781
+ return;782
+ }783
+ };784
+785
+ for update in updates {786
+ let handled = match update {787
+ RoomUpdate::Left(update) => caches.handle_left_room_update(update).await,788
+ RoomUpdate::Joined(update) => {789
+ trace!(?room_id, "Handling a `JoinedRoomUpdate`");790
+ caches.handle_joined_room_update(update).await711791
}712792
};713
-714
- if let Err(err) = caches.handle_joined_room_update(joined_room_update).await {793
+ if let Err(err) = handled {715794
// Non-fatal error, try to continue to the next room.716
- error!(%room_id, "handling joined room update: {err}");795
+ error!(%room_id, "handling a room update: {err}");717796
}718797
}719
-720
- // Invited rooms.721
- // TODO: we don't anything with `updates.invite` at this point.722
-723
- Ok(())724798
}725799
726800
/// Return all the event caches associated to a specific room.@@ -26,14 +26,17 @@ use ruma::{OwnedEventId, OwnedTransactionId};
2626
use tokio::{2727
select,2828
sync::{29
- broadcast::{Receiver, Sender, error::RecvError},29
+ broadcast::{30
+ Receiver, Sender,31
+ error::{RecvError, TryRecvError},32
+ },3033
mpsc,3134
},3235
};3336
use tracing::{Instrument as _, Span, debug, error, info, info_span, instrument, trace, warn};3437
3538
use super::{36
- AutoShrinkChannelPayload, EventCacheError, EventCacheInner, EventsOrigin,39
+ AutoShrinkChannelPayload, EventCacheInner, EventsOrigin, PendingRoomUpdates,3740
RoomEventCacheLinkedChunkUpdate, RoomEventCacheUpdate, TimelineVectorDiffs,3841
};3942
use crate::{@@ -48,44 +51,53 @@ pub(super) async fn room_updates_task(
4851
mut room_updates_feed: Receiver<RoomUpdates>,4952
) {5053
trace!("Spawning the listen task");54
+ let mut pending = PendingRoomUpdates::default();5155
loop {52
- match room_updates_feed.recv().await {53
- Ok(updates) => {54
- trace!("Receiving `RoomUpdates`");55
-56
- if let Err(err) = inner.handle_room_updates(updates).await {57
- match err {58
- EventCacheError::ClientDropped => {59
- // The client has dropped, exit the listen task.60
- info!(61
- "Closing the event cache global listen task because client dropped"62
- );63
- break;64
- }65
- err => {66
- error!("Error when handling room updates: {err}");67
- }68
- }56
+ if pending.is_empty() {57
+ match room_updates_feed.recv().await {58
+ Ok(updates) => {59
+ trace!("Receiving `RoomUpdates`");60
+ pending.fold(updates);61
+ }62
+ Err(RecvError::Lagged(num_skipped)) => pending.lagged += num_skipped,63
+ Err(RecvError::Closed) => {64
+ // The sender has shut down, exit.65
+ info!("Closing the event cache global listen task because receiver closed");66
+ break;6967
}70
- inner.room_updates_handled.send_modify(|handled| *handled += 1);7168
}69
+ }7270
73
- Err(RecvError::Lagged(num_skipped)) => {71
+ // a backlog of responses folded in together lets a room on screen skip the queue,72
+ // and the room's own updates stay in the order sync sent them73
+ loop {74
+ match room_updates_feed.try_recv() {75
+ Ok(updates) => pending.fold(updates),76
+ Err(TryRecvError::Lagged(num_skipped)) => pending.lagged += num_skipped,77
+ Err(TryRecvError::Empty | TryRecvError::Closed) => break,78
+ }79
+ }80
+81
+ {82
+ let _lock = inner.multiple_room_updates_lock.lock().await;83
+ inner.handle_pending_room_updates(&mut pending).await;84
+ }85
+86
+ if pending.is_empty() {87
+ if pending.lagged > 0 {7488
// Forget everything we know; we could have missed events, and we have7589
// no way to reconcile at the moment!7690
// TODO: implement Smart Matching™,77
- warn!(num_skipped, "Lagged behind room updates, clearing all rooms");91
+ warn!(92
+ num_skipped = pending.lagged,93
+ "Lagged behind room updates, clearing all rooms"94
+ );7895
if let Err(err) = inner.clear_all_rooms().await {7996
error!("when clearing storage after lag in listen_task: {err}");8097
}81
- inner.room_updates_handled.send_modify(|handled| *handled += num_skipped);82
- }83
-84
- Err(RecvError::Closed) => {85
- // The sender has shut down, exit.86
- info!("Closing the event cache global listen task because receiver closed");87
- break;8898
}99
+ let handled = pending.take_handled();100
+ inner.room_updates_handled.send_modify(|count| *count += handled);89101
}90102
}91103
}