mirror/matrix-rust-sdk
1
0
Fork 0
mirror of https://github.com/matrix-org/matrix-rust-sdk.git synced 2026-05-24 08:36:10 -04:00
Code Issues Packages Projects Releases Wiki Activity

feat(base): Add new_latest_event_timestamp and new_latest_event_is_local.

Browse Source 
This patch adds 2 methods on `RoomInfo`: `new_latest_event_timestamp`
and `new_latest_event_is_local`, which respectively returns
`LatestEventValue::timestamp` and `LatestEventValue::is_local`. The goal
is to avoid cloning a `LatestEventValue` when it's useless. For example,
in the room list sorters!

This patch also updates the room list sorters `recency` and
`latest_event` to use these new methods. It improves the speed and
throughput by 18%.
This commit is contained in:
Ivan Enderlin
2025-09-17 16:27:08 +02:00
parent ec5c31a19d
commit 62eb1996d9
4 changed files with 133 additions and 83 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
crates/matrix-sdk-base/src/latest_event.rs
View File

@@ -62,6 +62,18 @@ impl LatestEventValue {
| Self::LocalCannotBeSent(LocalLatestEventValue { timestamp, .. }) => Some(*timestamp),
}
}
/// Check whether the [`LatestEventValue`] represents a local value or not,
/// i.e. it is [`LocalIsSending`] or [`LocalCannotBeSent`].
///
/// [`LocalIsSending`]: LatestEventValue::LocalIsSending
/// [`LocalCannotBeSent`]: LatestEventValue::LocalCannotBeSent
pub fn is_local(&self) -> bool {
match self {
Self::LocalIsSending(_) | Self::LocalCannotBeSent(_) => true,
Self::None | Self::Remote(_) => false,
}
}
}
/// Represents the value for [`LatestEventValue::Remote`].

14
crates/matrix-sdk-base/src/room/latest_event.rs
View File

@@ -15,6 +15,7 @@
#[cfg(feature = "e2e-encryption")]
use std::{collections::BTreeMap, num::NonZeroUsize};
use ruma::MilliSecondsSinceUnixEpoch;
#[cfg(feature = "e2e-encryption")]
use ruma::{OwnedRoomId, events::AnySyncTimelineEvent, serde::Raw};
@@ -35,10 +36,23 @@ impl Room {
}
/// Return the [`LatestEventValue`] of this room.
///
/// Note that it clones the [`LatestEventValue`]! This can be add pressure
/// on the memory if used in a hot path.
pub fn new_latest_event(&self) -> LatestEventValue {
self.inner.read().new_latest_event.clone()
}
/// Return the value of [`LatestEventValue::timestamp`].
pub fn new_latest_event_timestamp(&self) -> Option<MilliSecondsSinceUnixEpoch> {
self.inner.read().new_latest_event.timestamp()
}
/// Return the value of [`LatestEventValue::is_local`].
pub fn new_latest_event_is_local(&self) -> bool {
self.inner.read().new_latest_event.is_local()
}
/// Return the most recent few encrypted events. When the keys come through
/// to decrypt these, the most recent relevant one will replace
/// latest_event. (We can't tell which one is relevant until

144
crates/matrix-sdk-ui/src/room_list_service/sorters/latest_event.rs
View File

@@ -14,45 +14,35 @@
use std::cmp::Ordering;
use matrix_sdk::latest_events::LatestEventValue;
use super::{Room, Sorter};
fn cmp<F>(latest_events: F, left: &Room, right: &Room) -> Ordering
fn cmp<F>(are_latest_events_locals: F, left: &Room, right: &Room) -> Ordering
where
F: Fn(&Room, &Room) -> (LatestEventValue, LatestEventValue),
F: Fn(&Room, &Room) -> (bool, bool),
{
// We want local latest event to come first. When there is a remote latest event
// or no latest event, we don't want to sort them.
match latest_events(left, right) {
// `None` == `None`.
// `None` == `Remote`.
// `Remote` == `None`.
// `Remote` == `Remote`.
(
LatestEventValue::None | LatestEventValue::Remote(_),
LatestEventValue::None | LatestEventValue::Remote(_),
) => Ordering::Equal,
match are_latest_events_locals(left, right) {
// `false` and `false`, i.e.:
// - `None` == `None`.
// - `None` == `Remote`.
// - `Remote` == `None`.
// - `Remote` == `Remote`.
(false, false) => Ordering::Equal,
// `None` > `Local*`.
// `Remote` > `Local*`.
(
LatestEventValue::None | LatestEventValue::Remote(_),
LatestEventValue::LocalIsSending(_) | LatestEventValue::LocalCannotBeSent(_),
) => Ordering::Greater,
// `false` and `true`, i.e.:
// - `None` > `Local*`.
// - `Remote` > `Local*`.
(false, true) => Ordering::Greater,
// `Local*` < `None`.
// `Local*` < `Remote`.
(
LatestEventValue::LocalIsSending(_) | LatestEventValue::LocalCannotBeSent(_),
LatestEventValue::None | LatestEventValue::Remote(_),
) => Ordering::Less,
// `true` and `false`, i.e.:
// - `Local*` < `None`.
// - `Local*` < `Remote`.
(true, false) => Ordering::Less,
// `Local*` == `Local*`
(
LatestEventValue::LocalIsSending(_) | LatestEventValue::LocalCannotBeSent(_),
LatestEventValue::LocalIsSending(_) | LatestEventValue::LocalCannotBeSent(_),
) => Ordering::Equal,
// `true` and `false, i.e.:
// - `Local*` == `Local*`
(true, true) => Ordering::Equal,
}
}
@@ -61,9 +51,18 @@ where
/// ([`LatestEventValue::LocalIsSending`] or
/// [`LatestEventValue::LocalCannotBeSent`]) come first, and latest event
/// representing a remote event ([`LatestEventValue::Remote`]) come last.
///
/// [`LatestEventValue::LocalIsSending`]: matrix_sdk_base::latest_event::LatestEventValue::LocalIsSending
/// [`LatestEventValue::LocalCannotBeSent`]: matrix_sdk_base::latest_event::LatestEventValue::LocalCannotBeSent
/// [`LatestEventValue::Remote`]: matrix_sdk_base::latest_event::LatestEventValue::Remote
pub fn new_sorter() -> impl Sorter {
let latest_events =
|left: &Room, right: &Room| (left.new_latest_event(), right.new_latest_event());
let latest_events = |left: &Room, right: &Room| {
// Be careful. This method is called **a lot** in the context of a sorter. Using
// `Room::new_latest_event` would be dramatic as it returns a clone of the
// `LatestEventValue`. It's better to use the more specific method
// `Room::new_latest_event_is_local`.
(left.new_latest_event_is_local(), right.new_latest_event_is_local())
};
move |left, right| -> Ordering { cmp(latest_events, left, right) }
}
@@ -71,7 +70,7 @@ pub fn new_sorter() -> impl Sorter {
#[cfg(test)]
mod tests {
use matrix_sdk::{
latest_events::{LocalLatestEventValue, RemoteLatestEventValue},
latest_events::{LatestEventValue, LocalLatestEventValue, RemoteLatestEventValue},
store::SerializableEventContent,
test_utils::logged_in_client_with_server,
};
@@ -143,22 +142,34 @@ mod tests {
// `None` and `None`.
{
assert_eq!(cmp(|_, _| (none(), none()), &room_a, &room_b), Ordering::Equal);
assert_eq!(
cmp(|_, _| (none().is_local(), none().is_local()), &room_a, &room_b),
Ordering::Equal
);
}
// `None` and `Remote`.
{
assert_eq!(cmp(|_, _| (none(), remote()), &room_a, &room_b), Ordering::Equal);
assert_eq!(
cmp(|_, _| (none().is_local(), remote().is_local()), &room_a, &room_b),
Ordering::Equal
);
}
// `Remote` and `None`.
{
assert_eq!(cmp(|_, _| (remote(), none()), &room_a, &room_b), Ordering::Equal);
assert_eq!(
cmp(|_, _| (remote().is_local(), none().is_local()), &room_a, &room_b),
Ordering::Equal
);
}
// `Remote` and `None`.
{
assert_eq!(cmp(|_, _| (remote(), remote()), &room_a, &room_b), Ordering::Equal);
assert_eq!(
cmp(|_, _| (remote().is_local(), remote().is_local()), &room_a, &room_b),
Ordering::Equal
);
}
}
@@ -172,11 +183,15 @@ mod tests {
// `None` and `Local*`.
{
assert_eq!(
cmp(|_, _| (none(), local_is_sending()), &room_a, &room_b),
cmp(|_, _| (none().is_local(), local_is_sending().is_local()), &room_a, &room_b),
Ordering::Greater
);
assert_eq!(
cmp(|_, _| (none(), local_cannot_be_sent()), &room_a, &room_b),
cmp(
|_, _| (none().is_local(), local_cannot_be_sent().is_local()),
&room_a,
&room_b
),
Ordering::Greater
);
}
@@ -184,11 +199,15 @@ mod tests {
// `Remote` and `Local*`.
{
assert_eq!(
cmp(|_, _| (remote(), local_is_sending()), &room_a, &room_b),
cmp(|_, _| (remote().is_local(), local_is_sending().is_local()), &room_a, &room_b),
Ordering::Greater
);
assert_eq!(
cmp(|_, _| (remote(), local_cannot_be_sent()), &room_a, &room_b),
cmp(
|_, _| (remote().is_local(), local_cannot_be_sent().is_local()),
&room_a,
&room_b
),
Ordering::Greater
);
}
@@ -203,9 +222,16 @@ mod tests {
// `Local*` and `None`.
{
assert_eq!(cmp(|_, _| (local_is_sending(), none()), &room_a, &room_b), Ordering::Less);
assert_eq!(
cmp(|_, _| (local_cannot_be_sent(), none()), &room_a, &room_b),
cmp(|_, _| (local_is_sending().is_local(), none().is_local()), &room_a, &room_b),
Ordering::Less
);
assert_eq!(
cmp(
|_, _| (local_cannot_be_sent().is_local(), none().is_local()),
&room_a,
&room_b
),
Ordering::Less
);
}
@@ -213,11 +239,15 @@ mod tests {
// `Local*` and `Remote`.
{
assert_eq!(
cmp(|_, _| (local_is_sending(), remote()), &room_a, &room_b),
cmp(|_, _| (local_is_sending().is_local(), remote().is_local()), &room_a, &room_b),
Ordering::Less
);
assert_eq!(
cmp(|_, _| (local_cannot_be_sent(), remote()), &room_a, &room_b),
cmp(
|_, _| (local_cannot_be_sent().is_local(), remote().is_local()),
&room_a,
&room_b
),
Ordering::Less
);
}
@@ -233,19 +263,35 @@ mod tests {
// `Local*` and `Local*`.
{
assert_eq!(
cmp(|_, _| (local_is_sending(), local_is_sending()), &room_a, &room_b),
cmp(
|_, _| (local_is_sending().is_local(), local_is_sending().is_local()),
&room_a,
&room_b
),
Ordering::Equal
);
assert_eq!(
cmp(|_, _| (local_is_sending(), local_cannot_be_sent()), &room_a, &room_b),
cmp(
|_, _| (local_is_sending().is_local(), local_cannot_be_sent().is_local()),
&room_a,
&room_b
),
Ordering::Equal
);
assert_eq!(
cmp(|_, _| (local_cannot_be_sent(), local_is_sending()), &room_a, &room_b),
cmp(
|_, _| (local_cannot_be_sent().is_local(), local_is_sending().is_local()),
&room_a,
&room_b
),
Ordering::Equal
);
assert_eq!(
cmp(|_, _| (local_cannot_be_sent(), local_cannot_be_sent()), &room_a, &room_b),
cmp(
|_, _| (local_cannot_be_sent().is_local(), local_cannot_be_sent().is_local()),
&room_a,
&room_b
),
Ordering::Equal
);
}

46
crates/matrix-sdk-ui/src/room_list_service/sorters/recency.rs
View File

@@ -14,10 +14,6 @@
use std::cmp::Ordering;
// TODO @hywan: remove once the `TODO` in `extract_rank` is solved.
#[allow(unused)]
use matrix_sdk::latest_events::LatestEventValue;
use super::{Room, Sorter};
fn cmp<F>(ranks: F, left: &Room, right: &Room) -> Ordering
@@ -64,6 +60,7 @@ where
///
/// [`RoomInfo::recency_stamp`]: matrix_sdk_base::RoomInfo::recency_stamp
/// [`RoomInfo::new_latest_event`]: matrix_sdk_base::RoomInfo::new_latest_event
/// [`LatestEventValue::None`]: matrix_sdk_base::latest_event::LatestEventValue::None
pub fn new_sorter() -> impl Sorter {
let ranks = |left: &Room, right: &Room| extract_rank(left, right);
@@ -84,43 +81,27 @@ type Rank = u64;
/// `RoomInfo::recency_stamp` is not a timestamp, while `LatestEventValue` uses
/// a timestamp.
fn extract_rank(left: &Room, right: &Room) -> (Option<Rank>, Option<Rank>) {
// TODO @hywan: calling `LatestEventValue::timestamp` here is apparently
// causing issues. While investigating, let's disable this part of the code,
// and let's fallback to the recency stamp (as it was the case before).
(left.recency_stamp().map(Into::into), right.recency_stamp().map(Into::into))
/*
match (left.new_latest_event(), right.new_latest_event()) {
// One of both rooms has NO latest event value. Let's fallback to the recency stamp from the
// `RoomInfo` for both room.
(LatestEventValue::None, _) | (_, LatestEventValue::None) => {
// Be careful. This method is called **a lot** in the context of a sorter. Using
// `Room::new_latest_event` would be dramatic as it returns a clone of the
// `LatestEventValue`. It's better to use the more specific method
// `Room::new_latest_event_timestamp`.
match (left.new_latest_event_timestamp(), right.new_latest_event_timestamp()) {
// One of the two rooms has no latest event timestamp. Let's fallback to
// the recency stamp from the `RoomInfo` for both room.
(None, _) | (_, None) => {
(left.recency_stamp().map(Into::into), right.recency_stamp().map(Into::into))
}
// Both rooms have a non-`None` latest event. We can use their timestamps as a rank.
(
left @ LatestEventValue::Remote(_)
| left @ LatestEventValue::LocalIsSending(_)
| left @ LatestEventValue::LocalCannotBeSent(_),
right @ LatestEventValue::Remote(_)
| right @ LatestEventValue::LocalIsSending(_)
| right @ LatestEventValue::LocalCannotBeSent(_),
) => (
left.timestamp().map(|ms| ms.get().into()),
right.timestamp().map(|ms| ms.get().into()),
),
// Both rooms have a timestamp. We can use them as a rank.
(Some(left), Some(right)) => (Some(left.get().into()), Some(right.get().into())),
}
*/
}
#[cfg(test)]
mod tests {
// TODO @hywan: remove once the `TODO` in `extract_rank` is solved.
#![allow(unused)]
use matrix_sdk::{
RoomRecencyStamp,
latest_events::{LocalLatestEventValue, RemoteLatestEventValue},
latest_events::{LatestEventValue, LocalLatestEventValue, RemoteLatestEventValue},
store::SerializableEventContent,
test_utils::logged_in_client_with_server,
};
@@ -229,8 +210,6 @@ mod tests {
}
}
// TODO @hywan: uncomment once the `TODO` in `extract_rank` is solved.
/*
#[async_test]
async fn test_extract_recency_stamp_with_remote_or_local() {
let (client, server) = logged_in_client_with_server().await;
@@ -254,7 +233,6 @@ mod tests {
}
}
}
*/
#[async_test]
async fn test_with_two_recency_stamps() {