From 81e328d0903b2370c0411ba9b3d6bdc59ebc7779 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 11 Jan 2024 11:44:14 +0100 Subject: [PATCH 1/5] chore: Remove the `get_` of some `Store`'s methods. This patch renames `Store::get_rooms`, `::get_rooms_filtered` and `::get_room` to respectively `::rooms`, `::rooms_filtered` and `::room`. This `get_` prefix isn't really Rust idiomatic. --- crates/matrix-sdk-base/src/client.rs | 19 +++++++++---------- crates/matrix-sdk-base/src/sliding_sync.rs | 2 +- crates/matrix-sdk-base/src/store/mod.rs | 6 +++--- crates/matrix-sdk-base/src/sync.rs | 2 +- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 26adfb424..811a59b1e 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -161,12 +161,12 @@ impl BaseClient { /// Get all the rooms this client knows about. pub fn get_rooms(&self) -> Vec { - self.store.get_rooms() + self.store.rooms() } /// Get all the rooms this client knows about, filtered by room state. pub fn get_rooms_filtered(&self, filter: RoomStateFilter) -> Vec { - self.store.get_rooms_filtered(filter) + self.store.rooms_filtered(filter) } /// Lookup the Room for the given RoomId, or create one, if it didn't exist @@ -572,7 +572,7 @@ impl BaseClient { on_room_info(room_info); } // The `BaseClient` has the `Room`, which has the `RoomInfo`. - else if let Some(room) = client.store.get_room(room_id) { + else if let Some(room) = client.store.room(room_id) { // Clone the `RoomInfo`. let mut room_info = room.clone_info(); @@ -637,7 +637,7 @@ impl BaseClient { if let Some(room) = changes.room_infos.get_mut(room_id) { room.base_info.dm_targets.insert(user_id.clone()); - } else if let Some(room) = self.store.get_room(room_id) { + } else if let Some(room) = self.store.room(room_id) { let mut info = room.clone_info(); if info.base_info.dm_targets.insert(user_id.clone()) { changes.add_room(info); @@ -1111,8 +1111,8 @@ impl BaseClient { } for (room_id, room_info) in &changes.room_infos { - if let Some(room) = self.store.get_room(room_id) { - room.set_room_info(room_info.clone(), trigger_room_list_update); + if let Some(room) = self.store.room(room_id) { + room.set_room_info(room_info.clone(), trigger_room_list_update) } } } @@ -1143,13 +1143,12 @@ impl BaseClient { return Err(Error::InvalidReceiveMembersParameters); } - let mut chunk = Vec::with_capacity(response.chunk.len()); - - let Some(room) = self.store.get_room(room_id) else { + let Some(room) = self.store.room(room_id) else { // The room is unknown to us: leave early. return Ok(()); }; + let mut chunk = Vec::with_capacity(response.chunk.len()); let mut changes = StateChanges::default(); #[cfg(feature = "e2e-encryption")] @@ -1309,7 +1308,7 @@ impl BaseClient { /// /// * `room_id` - The id of the room that should be fetched. pub fn get_room(&self, room_id: &RoomId) -> Option { - self.store.get_room(room_id) + self.store.room(room_id) } /// Get the olm machine. diff --git a/crates/matrix-sdk-base/src/sliding_sync.rs b/crates/matrix-sdk-base/src/sliding_sync.rs index 939d3cbdd..42c0f52b1 100644 --- a/crates/matrix-sdk-base/src/sliding_sync.rs +++ b/crates/matrix-sdk-base/src/sliding_sync.rs @@ -223,7 +223,7 @@ impl BaseClient { for (room_id, raw) in &rooms_account_data { self.handle_room_account_data(room_id, raw, &mut changes).await; - if let Some(room) = self.store.get_room(room_id) { + if let Some(room) = self.store.room(room_id) { match room.state() { RoomState::Joined => new_rooms .join diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index f2784caa8..a8c2f858a 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -203,12 +203,12 @@ impl Store { } /// Get all the rooms this store knows about. - pub fn get_rooms(&self) -> Vec { + pub fn rooms(&self) -> Vec { self.rooms.read().unwrap().values().cloned().collect() } /// Get all the rooms this store knows about, filtered by state. - pub fn get_rooms_filtered(&self, filter: RoomStateFilter) -> Vec { + pub fn rooms_filtered(&self, filter: RoomStateFilter) -> Vec { self.rooms .read() .unwrap() @@ -219,7 +219,7 @@ impl Store { } /// Get the room with the given room id. - pub fn get_room(&self, room_id: &RoomId) -> Option { + pub fn room(&self, room_id: &RoomId) -> Option { self.rooms.read().unwrap().get(room_id).cloned() } diff --git a/crates/matrix-sdk-base/src/sync.rs b/crates/matrix-sdk-base/src/sync.rs index 50cc6210b..d9015e72e 100644 --- a/crates/matrix-sdk-base/src/sync.rs +++ b/crates/matrix-sdk-base/src/sync.rs @@ -89,7 +89,7 @@ impl RoomUpdates { .keys() .chain(self.join.keys()) .chain(self.invite.keys()) - .filter_map(|room_id| store.get_room(room_id)) + .filter_map(|room_id| store.room(room_id)) { let _ = room.compute_display_name().await; } From adff8938350120594581c814df1300451459ed0f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 13 Jun 2024 16:28:38 +0200 Subject: [PATCH 2/5] feat(base): Faster `Store::rooms_filtered`. Just like in https://github.com/matrix-org/matrix-rust-sdk/pull/3552, this patch updates `Store::rooms_filtered` to not call `Store::room` so that it doesn't lock `rooms` for each item, thus making it way faster. --- crates/matrix-sdk-base/src/store/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index a8c2f858a..5e908e6b3 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -213,8 +213,8 @@ impl Store { .read() .unwrap() .iter() - .filter(|(_, r)| filter.matches(r.state())) - .filter_map(|(id, _)| self.get_room(id)) + .filter(|(_, room)| filter.matches(room.state())) + .map(|(_, room)| room.clone()) .collect() } From 408c12fc669c1b64c0f99f61ca83b29cdeaa98f5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 13 Jun 2024 16:32:21 +0200 Subject: [PATCH 3/5] doc(base): Update the `CHANGELOG.md` file. --- crates/matrix-sdk-base/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/matrix-sdk-base/CHANGELOG.md b/crates/matrix-sdk-base/CHANGELOG.md index dc875c7b6..5e20bed19 100644 --- a/crates/matrix-sdk-base/CHANGELOG.md +++ b/crates/matrix-sdk-base/CHANGELOG.md @@ -3,6 +3,10 @@ - Replace the `Notification` type from Ruma in `SyncResponse` and `StateChanges` by a custom one - The ambiguity maps in `SyncResponse` are moved to `JoinedRoom` and `LeftRoom` - `AmbiguityCache` contains the room member's user ID +- `Store::get_rooms` and `Store::get_rooms_filtered` are way faster because they + don't acquire the lock for every room they read. +- `Store::get_rooms`, `Store::get_rooms_filtered` and `Store::get_room` are + renamed `Store::rooms`, `Store::rooms_filtered` and `Store::room`. # 0.7.0 From 771ddcb91e6dcd8a36ae4bc91279523a75c27147 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 11 Jan 2024 11:48:53 +0100 Subject: [PATCH 4/5] chore(base): Remove the `get_` of some `Client`'s methods. This patch renames `Client::get_rooms`, `::get_rooms_filtered` and `::get_room` to respectively `::rooms`, `::rooms_filtered` and `::room`. This `get_` prefix isn't really Rust idiomatic. --- crates/matrix-sdk-base/src/client.rs | 10 ++-------- crates/matrix-sdk/src/client/mod.rs | 14 +++++--------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 811a59b1e..6a8dbf9c0 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -160,12 +160,12 @@ impl BaseClient { } /// Get all the rooms this client knows about. - pub fn get_rooms(&self) -> Vec { + pub fn rooms(&self) -> Vec { self.store.rooms() } /// Get all the rooms this client knows about, filtered by room state. - pub fn get_rooms_filtered(&self, filter: RoomStateFilter) -> Vec { + pub fn rooms_filtered(&self, filter: RoomStateFilter) -> Vec { self.store.rooms_filtered(filter) } @@ -175,12 +175,6 @@ impl BaseClient { self.store.get_or_create_room(room_id, room_state, self.roominfo_update_sender.clone()) } - /// Get all the rooms this client knows about. - #[deprecated = "Use get_rooms_filtered with RoomStateFilter::INVITED instead."] - pub fn get_stripped_rooms(&self) -> Vec { - self.get_rooms_filtered(RoomStateFilter::INVITED) - } - /// Get a reference to the store. #[allow(unknown_lints, clippy::explicit_auto_deref)] pub fn store(&self) -> &DynStateStore { diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index af3c6e37e..b2a2e756f 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -901,17 +901,13 @@ impl Client { /// /// This will return the list of joined, invited, and left rooms. pub fn rooms(&self) -> Vec { - self.base_client() - .get_rooms() - .into_iter() - .map(|room| Room::new(self.clone(), room)) - .collect() + self.base_client().rooms().into_iter().map(|room| Room::new(self.clone(), room)).collect() } /// Get all the rooms the client knows about, filtered by room state. pub fn rooms_filtered(&self, filter: RoomStateFilter) -> Vec { self.base_client() - .get_rooms_filtered(filter) + .rooms_filtered(filter) .into_iter() .map(|room| Room::new(self.clone(), room)) .collect() @@ -920,7 +916,7 @@ impl Client { /// Returns the joined rooms this client knows about. pub fn joined_rooms(&self) -> Vec { self.base_client() - .get_rooms_filtered(RoomStateFilter::JOINED) + .rooms_filtered(RoomStateFilter::JOINED) .into_iter() .map(|room| Room::new(self.clone(), room)) .collect() @@ -929,7 +925,7 @@ impl Client { /// Returns the invited rooms this client knows about. pub fn invited_rooms(&self) -> Vec { self.base_client() - .get_rooms_filtered(RoomStateFilter::INVITED) + .rooms_filtered(RoomStateFilter::INVITED) .into_iter() .map(|room| Room::new(self.clone(), room)) .collect() @@ -938,7 +934,7 @@ impl Client { /// Returns the left rooms this client knows about. pub fn left_rooms(&self) -> Vec { self.base_client() - .get_rooms_filtered(RoomStateFilter::LEFT) + .rooms_filtered(RoomStateFilter::LEFT) .into_iter() .map(|room| Room::new(self.clone(), room)) .collect() From b21b8c4a53cabeba4e4602d32cf439d310cebe8f Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 13 Jun 2024 16:52:37 +0200 Subject: [PATCH 5/5] doc(base): Update the `CHANGELOG.md` file. --- crates/matrix-sdk-base/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/matrix-sdk-base/CHANGELOG.md b/crates/matrix-sdk-base/CHANGELOG.md index 5e20bed19..b0e94d8d8 100644 --- a/crates/matrix-sdk-base/CHANGELOG.md +++ b/crates/matrix-sdk-base/CHANGELOG.md @@ -7,6 +7,9 @@ don't acquire the lock for every room they read. - `Store::get_rooms`, `Store::get_rooms_filtered` and `Store::get_room` are renamed `Store::rooms`, `Store::rooms_filtered` and `Store::room`. +- `Client::get_rooms` and `Client::get_rooms_filtered` are renamed + `Client::rooms` and `Client::rooms_filtered`. +- `Client::get_stripped_rooms` has finally been removed. # 0.7.0