Skip to content

Commit

Permalink
base: Box large RoomInfo fields
Browse files Browse the repository at this point in the history
RoomInfo is often passed around by value, including in futures where
holding types with a large stack size is especially problematic¹.
It might make sense to move the actual data of (Base)RoomInfo into
an inner struct that is always held inside a Box or Arc, but this change
should have most of the benefits of that while being a bit simpler.

¹ rust-lang/rust#69826
  • Loading branch information
jplatte committed Nov 3, 2023
1 parent ab4c524 commit dc86835
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 20 deletions.
7 changes: 5 additions & 2 deletions crates/matrix-sdk-base/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,10 @@ impl BaseClient {
/// decrypted event if we found one, along with its index in the
/// latest_encrypted_events list, or None if we didn't find one.
#[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))]
async fn decrypt_latest_suitable_event(&self, room: &Room) -> Option<(LatestEvent, usize)> {
async fn decrypt_latest_suitable_event(
&self,
room: &Room,
) -> Option<(Box<LatestEvent>, usize)> {
let enc_events = room.latest_encrypted_events();

// Walk backwards through the encrypted events, looking for one we can decrypt
Expand All @@ -633,7 +636,7 @@ impl BaseClient {
is_suitable_for_latest_event(&any_sync_event)
{
// The event is the right type for us to use as latest_event
return Some((LatestEvent::new(decrypted), i));
return Some((Box::new(LatestEvent::new(decrypted)), i));
}
}
}
Expand Down
24 changes: 12 additions & 12 deletions crates/matrix-sdk-base/src/rooms/normal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,12 @@ impl Room {
/// sliding sync.
#[cfg(feature = "experimental-sliding-sync")]
pub fn latest_event(&self) -> Option<LatestEvent> {
self.inner.read().latest_event.clone()
self.inner.read().latest_event.as_deref().cloned()
}

/// Update the last event in the room
#[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))]
pub(crate) fn set_latest_event(&self, latest_event: Option<LatestEvent>) {
pub(crate) fn set_latest_event(&self, latest_event: Option<Box<LatestEvent>>) {
self.inner.update(|info| info.latest_event = latest_event);
}

Expand All @@ -429,7 +429,7 @@ impl Room {
/// Panics if index is not a valid index in the latest_encrypted_events
/// list.
#[cfg(all(feature = "e2e-encryption", feature = "experimental-sliding-sync"))]
pub(crate) fn on_latest_event_decrypted(&self, latest_event: LatestEvent, index: usize) {
pub(crate) fn on_latest_event_decrypted(&self, latest_event: Box<LatestEvent>, index: usize) {
self.set_latest_event(Some(latest_event));
self.latest_encrypted_events.write().unwrap().drain(0..=index);
}
Expand Down Expand Up @@ -732,10 +732,10 @@ pub struct RoomInfo {
pub(crate) encryption_state_synced: bool,
/// The last event send by sliding sync
#[cfg(feature = "experimental-sliding-sync")]
pub(crate) latest_event: Option<LatestEvent>,
pub(crate) latest_event: Option<Box<LatestEvent>>,
/// Base room info which holds some basic event contents important for the
/// room state.
pub(crate) base_info: BaseRoomInfo,
pub(crate) base_info: Box<BaseRoomInfo>,
}

#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
Expand Down Expand Up @@ -769,7 +769,7 @@ impl RoomInfo {
encryption_state_synced: false,
#[cfg(feature = "experimental-sliding-sync")]
latest_event: None,
base_info: BaseRoomInfo::new(),
base_info: Box::new(BaseRoomInfo::new()),
}
}

Expand Down Expand Up @@ -1246,10 +1246,10 @@ mod tests {
last_prev_batch: Some("pb".to_owned()),
sync_info: SyncInfo::FullySynced,
encryption_state_synced: true,
latest_event: Some(LatestEvent::new(
latest_event: Some(Box::new(LatestEvent::new(
Raw::from_json_string(json!({"sender": "@u:i.uk"}).to_string()).unwrap().into(),
)),
base_info: BaseRoomInfo::new(),
))),
base_info: Box::new(BaseRoomInfo::new()),
};

let info_json = json!({
Expand Down Expand Up @@ -1698,10 +1698,10 @@ mod tests {
}

#[cfg(feature = "experimental-sliding-sync")]
fn make_latest_event(event_id: &str) -> LatestEvent {
LatestEvent::new(SyncTimelineEvent::new(
fn make_latest_event(event_id: &str) -> Box<LatestEvent> {
Box::new(LatestEvent::new(SyncTimelineEvent::new(
Raw::from_json_string(json!({ "event_id": event_id }).to_string()).unwrap(),
))
)))
}

fn timestamp(minutes_ago: u32) -> MilliSecondsSinceUnixEpoch {
Expand Down
4 changes: 2 additions & 2 deletions crates/matrix-sdk-base/src/sliding_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,11 +524,11 @@ async fn cache_latest_events(
}
}

let latest_event = LatestEvent::new_with_sender_details(
let latest_event = Box::new(LatestEvent::new_with_sender_details(
event.clone(),
sender_profile,
sender_name_is_ambiguous,
);
));

// Store it in the return RoomInfo, and in the Room, to make sure they are
// consistent
Expand Down
11 changes: 7 additions & 4 deletions crates/matrix-sdk-base/src/store/migration_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl RoomInfoV1 {
sync_info,
encryption_state_synced,
#[cfg(feature = "experimental-sliding-sync")]
latest_event: latest_event.map(LatestEvent::new),
latest_event: latest_event.map(|ev| Box::new(LatestEvent::new(ev))),
base_info: base_info.migrate(create),
}
}
Expand Down Expand Up @@ -157,7 +157,10 @@ struct BaseRoomInfoV1 {

impl BaseRoomInfoV1 {
/// Migrate this to a [`BaseRoomInfo`].
fn migrate(self, create: Option<&SyncOrStrippedState<RoomCreateEventContent>>) -> BaseRoomInfo {
fn migrate(
self,
create: Option<&SyncOrStrippedState<RoomCreateEventContent>>,
) -> Box<BaseRoomInfo> {
let BaseRoomInfoV1 {
avatar,
canonical_alias,
Expand Down Expand Up @@ -186,7 +189,7 @@ impl BaseRoomInfoV1 {
MinimalStateEvent::Redacted(ev) => MinimalStateEvent::Redacted(ev),
});

BaseRoomInfo {
Box::new(BaseRoomInfo {
avatar,
canonical_alias,
create,
Expand All @@ -200,7 +203,7 @@ impl BaseRoomInfoV1 {
tombstone,
topic,
rtc_member: BTreeMap::new(),
}
})
}
}

Expand Down

0 comments on commit dc86835

Please sign in to comment.