Skip to content

Commit e3730f8

Browse files
committed
perf(sdk): Do not replace a LatestEventValue::None by itself.
Replacing a `LatestEventValue::None` by a `LatestEventValue::None` is ignored. It reduces the number of (useless) updates in the system.
1 parent bddb14a commit e3730f8

File tree

2 files changed

+73
-3
lines changed

2 files changed

+73
-3
lines changed

crates/matrix-sdk-base/src/latest_event.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ impl LatestEventValue {
5656
Self::None | Self::Remote(_) => false,
5757
}
5858
}
59+
60+
/// Check whether the [`LatestEventValue`] is not set, i.e. [`None`].
61+
///
62+
/// [`None`]: LatestEventValue::None
63+
pub fn is_none(&self) -> bool {
64+
matches!(self, Self::None)
65+
}
5966
}
6067

6168
/// Represents the value for [`LatestEventValue::Remote`].

crates/matrix-sdk/src/latest_events/latest_event.rs

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use std::{
1717
ops::{Deref, DerefMut, Not},
1818
};
1919

20-
use eyeball::{AsyncLock, SharedObservable, Subscriber};
20+
use eyeball::{AsyncLock, ObservableWriteGuard, SharedObservable, Subscriber};
2121
pub use matrix_sdk_base::latest_event::{
2222
LatestEventValue, LocalLatestEventValue, RemoteLatestEventValue,
2323
};
@@ -154,8 +154,24 @@ impl LatestEvent {
154154
/// been found, we want to latest event value to be `None`, so that it is
155155
/// erased correctly.
156156
async fn update(&mut self, new_value: LatestEventValue) {
157-
self.current_value.set(new_value.clone()).await;
158-
self.store(new_value).await;
157+
// Ideally, we would set `new_value` if and only if it is different from the
158+
// previous value. However, `LatestEventValue` cannot implement `PartialEq` at
159+
// the time of writing (2025-12-12). So we are only updating if
160+
// `LatestEventValue` is not `None` and if the previous value isn't `None`;
161+
// basically, replacing `None` with `None` will not update the value.
162+
{
163+
let mut guard = self.current_value.write().await;
164+
let previous_value = guard.deref();
165+
166+
if (previous_value.is_none() && new_value.is_none()).not() {
167+
ObservableWriteGuard::set(&mut guard, new_value.clone());
168+
169+
// Release the write guard over the current value before hitting the store.
170+
drop(guard);
171+
172+
self.store(new_value).await;
173+
}
174+
}
159175
}
160176

161177
/// Update the `RoomInfo` associated to this room to set the new
@@ -245,6 +261,7 @@ mod tests_latest_event {
245261
events::{AnyMessageLikeEventContent, room::message::RoomMessageEventContent},
246262
room_id, user_id,
247263
};
264+
use stream_assert::{assert_next_matches, assert_pending};
248265

249266
use super::{
250267
LatestEvent, LatestEventValue, LocalLatestEventValue, SerializableEventContent, With,
@@ -361,6 +378,52 @@ mod tests_latest_event {
361378
assert_matches!(latest_event.current_value.get().await, LatestEventValue::None);
362379
}
363380

381+
#[async_test]
382+
async fn test_update_ignore_none_if_previous_value_is_none() {
383+
let room_id = room_id!("!r0");
384+
385+
let server = MatrixMockServer::new().await;
386+
let client = server.client_builder().build().await;
387+
let weak_client = WeakClient::from_client(&client);
388+
389+
// Create the room.
390+
client.base_client().get_or_create_room(room_id, RoomState::Joined);
391+
let weak_room = WeakRoom::new(weak_client, room_id.to_owned());
392+
393+
let mut latest_event = LatestEvent::new(&weak_room, None);
394+
395+
let mut stream = latest_event.subscribe().await;
396+
397+
assert_pending!(stream);
398+
399+
// Set a non-`None` value.
400+
latest_event.update(LatestEventValue::LocalIsSending(local_room_message("foo"))).await;
401+
// We get it.
402+
assert_next_matches!(stream, LatestEventValue::LocalIsSending(_));
403+
404+
// Set a `None` value.
405+
latest_event.update(LatestEventValue::None).await;
406+
// We get it.
407+
assert_next_matches!(stream, LatestEventValue::None);
408+
409+
// Set a `None` value, again!
410+
latest_event.update(LatestEventValue::None).await;
411+
// We get it? No!
412+
assert_pending!(stream);
413+
414+
// Set a `None` value, again, and again!
415+
latest_event.update(LatestEventValue::None).await;
416+
// No means No!
417+
assert_pending!(stream);
418+
419+
// Set a non-`None` value.
420+
latest_event.update(LatestEventValue::LocalIsSending(local_room_message("bar"))).await;
421+
// We get it. Oof.
422+
assert_next_matches!(stream, LatestEventValue::LocalIsSending(_));
423+
424+
assert_pending!(stream);
425+
}
426+
364427
#[async_test]
365428
async fn test_local_has_priority_over_remote() {
366429
let room_id = room_id!("!r0").to_owned();

0 commit comments

Comments
 (0)