From ccd5e877eb3ff45b901a0b570e7d0fabd6668475 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 27 Jun 2023 16:05:26 +0200 Subject: [PATCH 1/3] feat: Cache the `RoomListService`'s rooms That avoids recreating a timeline object every time a user calls `RoomListService::room()` with the same room id, so that should speed up operations like fetching the latest event for rooms we've already entered before. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 5248430d5d9..e61462a68b4 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -67,7 +67,7 @@ mod room; mod room_list; mod state; -use std::{future::ready, sync::Arc}; +use std::{collections::BTreeMap, future::ready, sync::Arc}; use async_stream::stream; use eyeball::{shared::Observable, Subscriber}; @@ -87,11 +87,14 @@ use ruma::{ }; pub use state::*; use thiserror::Error; +use tokio::sync::Mutex; /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { sliding_sync: Arc, + /// Room cache, to avoid recreating `Room`s everytime users fetch them. + rooms: Arc>>, state: Observable, } @@ -155,7 +158,7 @@ impl RoomListService { .map(Arc::new) .map_err(Error::SlidingSync)?; - Ok(Self { sliding_sync, state: Observable::new(State::Init) }) + Ok(Self { sliding_sync, state: Observable::new(State::Init), rooms: Default::default() }) } /// Start to sync the room list. @@ -282,10 +285,17 @@ impl RoomListService { /// Get a [`Room`] if it exists. pub async fn room(&self, room_id: &RoomId) -> Result { - match self.sliding_sync.get_room(room_id).await { + let mut rooms = self.rooms.lock().await; + if let Some(room) = rooms.get(room_id) { + return Ok(room.clone()); + } + let room = match self.sliding_sync.get_room(room_id).await { Some(room) => Room::new(self.sliding_sync.clone(), room).await, None => Err(Error::RoomNotFound(room_id.to_owned())), - } + }; + let room = room?; + rooms.insert(room_id.to_owned(), room.clone()); + Ok(room) } #[cfg(any(test, feature = "testing"))] From e3df97421b448703a9ae5e829d6701e8b37e55f4 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 27 Jun 2023 16:27:31 +0200 Subject: [PATCH 2/3] chore: remove useless async --- crates/matrix-sdk-ui/src/room_list/mod.rs | 2 +- crates/matrix-sdk-ui/src/room_list/room.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index e61462a68b4..38e6193d3fa 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -290,7 +290,7 @@ impl RoomListService { return Ok(room.clone()); } let room = match self.sliding_sync.get_room(room_id).await { - Some(room) => Room::new(self.sliding_sync.clone(), room).await, + Some(room) => Room::new(self.sliding_sync.clone(), room), None => Err(Error::RoomNotFound(room_id.to_owned())), }; let room = room?; diff --git a/crates/matrix-sdk-ui/src/room_list/room.rs b/crates/matrix-sdk-ui/src/room_list/room.rs index de07ef926ac..1a9e6e71b9f 100644 --- a/crates/matrix-sdk-ui/src/room_list/room.rs +++ b/crates/matrix-sdk-ui/src/room_list/room.rs @@ -55,7 +55,7 @@ struct RoomInner { impl Room { /// Create a new `Room`. - pub(super) async fn new( + pub(super) fn new( sliding_sync: Arc, sliding_sync_room: SlidingSyncRoom, ) -> Result { From 98fed1d2ebbb90610fb83806d88f11b532dbc73d Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 27 Jun 2023 16:29:35 +0200 Subject: [PATCH 3/3] chore: use smaller critical sections This opens up a possible race condition where an embedder calls the method twice, generating two new rooms, but then it's fine as long as they don't cache them somewhere, which we expect they don't. --- crates/matrix-sdk-ui/src/room_list/mod.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list/mod.rs b/crates/matrix-sdk-ui/src/room_list/mod.rs index 38e6193d3fa..8fe36d50a3e 100644 --- a/crates/matrix-sdk-ui/src/room_list/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list/mod.rs @@ -87,14 +87,14 @@ use ruma::{ }; pub use state::*; use thiserror::Error; -use tokio::sync::Mutex; +use tokio::sync::RwLock; /// The [`RoomListService`] type. See the module's documentation to learn more. #[derive(Debug)] pub struct RoomListService { sliding_sync: Arc, /// Room cache, to avoid recreating `Room`s everytime users fetch them. - rooms: Arc>>, + rooms: Arc>>, state: Observable, } @@ -285,16 +285,20 @@ impl RoomListService { /// Get a [`Room`] if it exists. pub async fn room(&self, room_id: &RoomId) -> Result { - let mut rooms = self.rooms.lock().await; - if let Some(room) = rooms.get(room_id) { - return Ok(room.clone()); + { + let rooms = self.rooms.read().await; + if let Some(room) = rooms.get(room_id) { + return Ok(room.clone()); + } } + let room = match self.sliding_sync.get_room(room_id).await { - Some(room) => Room::new(self.sliding_sync.clone(), room), - None => Err(Error::RoomNotFound(room_id.to_owned())), + Some(room) => Room::new(self.sliding_sync.clone(), room)?, + None => return Err(Error::RoomNotFound(room_id.to_owned())), }; - let room = room?; - rooms.insert(room_id.to_owned(), room.clone()); + + self.rooms.write().await.insert(room_id.to_owned(), room.clone()); + Ok(room) }