Skip to content

Commit 50173d4

Browse files
jmartinespjmartinesp
committed
fixup! add max_concurrent_requests as part of TimelineFocus::PinnedEvents so clients can also customise this behaviour
1 parent b07d424 commit 50173d4

File tree

6 files changed

+77
-31
lines changed

6 files changed

+77
-31
lines changed

‎benchmarks/benches/room_bench.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,10 @@ pub fn load_pinned_events_benchmark(c: &mut Criterion) {
198198
client.event_cache().empty_immutable_cache().await;
199199

200200
let timeline = Timeline::builder(&room)
201-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
201+
.with_focus(TimelineFocus::PinnedEvents {
202+
max_events_to_load: 100,
203+
max_concurrent_requests: 10,
204+
})
202205
.build()
203206
.await
204207
.expect("Could not create timeline");

‎bindings/matrix-sdk-ffi/src/room.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ impl Room {
233233
&self,
234234
internal_id_prefix: Option<String>,
235235
max_events_to_load: u16,
236+
max_concurrent_requests: u16,
236237
) -> Result<Arc<Timeline>, ClientError> {
237238
let room = &self.inner;
238239

@@ -242,8 +243,10 @@ impl Room {
242243
builder = builder.with_internal_id_prefix(internal_id_prefix);
243244
}
244245

245-
let timeline =
246-
builder.with_focus(TimelineFocus::PinnedEvents { max_events_to_load }).build().await?;
246+
let timeline = builder
247+
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load, max_concurrent_requests })
248+
.build()
249+
.await?;
247250

248251
Ok(Timeline::new(timeline))
249252
}

‎crates/matrix-sdk-ui/src/timeline/controller/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,12 @@ impl<P: RoomDataProvider> TimelineController<P> {
253253
)
254254
}
255255

256-
TimelineFocus::PinnedEvents { max_events_to_load } => (
256+
TimelineFocus::PinnedEvents { max_events_to_load, max_concurrent_requests } => (
257257
TimelineFocusData::PinnedEvents {
258258
loader: PinnedEventsLoader::new(
259259
Arc::new(room_data_provider.clone()),
260260
max_events_to_load as usize,
261+
max_concurrent_requests as usize,
261262
),
262263
},
263264
TimelineFocusKind::PinnedEvents,

‎crates/matrix-sdk-ui/src/timeline/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,6 @@ pub use self::{
9595
event_type_filter::TimelineEventTypeFilter,
9696
item::{TimelineItem, TimelineItemKind},
9797
pagination::LiveBackPaginationStatus,
98-
pinned_events_loader::MAX_PINNED_EVENTS_CONCURRENT_REQUESTS,
9998
polls::PollResult,
10099
traits::RoomExt,
101100
virtual_item::VirtualTimelineItem,
@@ -174,7 +173,7 @@ pub enum TimelineFocus {
174173
Event { target: OwnedEventId, num_context_events: u16 },
175174

176175
/// Only show pinned events.
177-
PinnedEvents { max_events_to_load: u16 },
176+
PinnedEvents { max_events_to_load: u16, max_concurrent_requests: u16 },
178177
}
179178

180179
impl Timeline {

‎crates/matrix-sdk-ui/src/timeline/pinned_events_loader.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ use ruma::{EventId, MilliSecondsSinceUnixEpoch, OwnedEventId};
2424
use thiserror::Error;
2525
use tracing::{debug, warn};
2626

27-
/// The max number of concurrent `/event` requests to run when loading pinned
28-
/// events.
29-
pub const MAX_PINNED_EVENTS_CONCURRENT_REQUESTS: usize = 10;
30-
3127
/// Utility to load the pinned events in a room.
3228
pub struct PinnedEventsLoader {
3329
/// Backend to load pinned events.
@@ -36,12 +32,21 @@ pub struct PinnedEventsLoader {
3632
/// Maximum number of pinned events to load (either from network or the
3733
/// cache).
3834
max_events_to_load: usize,
35+
36+
/// Number of requests to load pinned events that can run concurrently. This
37+
/// is used to avoid overwhelming a home server with dozens or hundreds
38+
/// of concurrent requests.
39+
max_concurrent_requests: usize,
3940
}
4041

4142
impl PinnedEventsLoader {
4243
/// Creates a new `PinnedEventsLoader` instance.
43-
pub fn new(room: Arc<dyn PinnedEventsRoom>, max_events_to_load: usize) -> Self {
44-
Self { room, max_events_to_load }
44+
pub fn new(
45+
room: Arc<dyn PinnedEventsRoom>,
46+
max_events_to_load: usize,
47+
max_concurrent_requests: usize,
48+
) -> Self {
49+
Self { room, max_events_to_load, max_concurrent_requests }
4550
}
4651

4752
/// Loads the pinned events in this room, using the cache first and then
@@ -67,6 +72,7 @@ impl PinnedEventsLoader {
6772
}
6873

6974
let request_config = Some(RequestConfig::default().retry_limit(3));
75+
let max_concurrent_requests = self.max_concurrent_requests;
7076

7177
let mut loaded_events: Vec<SyncTimelineEvent> =
7278
stream::iter(pinned_event_ids.into_iter().map(|event_id| {
@@ -85,7 +91,7 @@ impl PinnedEventsLoader {
8591
}
8692
}
8793
}))
88-
.buffer_unordered(MAX_PINNED_EVENTS_CONCURRENT_REQUESTS)
94+
.buffer_unordered(max_concurrent_requests)
8995
// Get only the `Some<Vec<_>>` results
9096
.flat_map(stream::iter)
9197
// Flatten the `Vec`s into a single one containing all their items

‎crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ use matrix_sdk::{
1212
use matrix_sdk_base::deserialized_responses::TimelineEvent;
1313
use matrix_sdk_test::{async_test, JoinedRoomBuilder, StateTestEvent, SyncResponseBuilder, BOB};
1414
use matrix_sdk_ui::{
15-
timeline::{
16-
RoomExt, TimelineFocus, TimelineItemContent, MAX_PINNED_EVENTS_CONCURRENT_REQUESTS,
17-
},
15+
timeline::{RoomExt, TimelineFocus, TimelineItemContent},
1816
Timeline,
1917
};
2018
use ruma::{
@@ -53,7 +51,10 @@ async fn test_new_pinned_events_are_added_on_sync() {
5351

5452
let room = test_helper.client.get_room(&room_id).unwrap();
5553
let timeline = Timeline::builder(&room)
56-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
54+
.with_focus(TimelineFocus::PinnedEvents {
55+
max_events_to_load: 100,
56+
max_concurrent_requests: 10,
57+
})
5758
.build()
5859
.await
5960
.unwrap();
@@ -138,7 +139,10 @@ async fn test_new_pinned_event_ids_reload_the_timeline() {
138139

139140
let room = test_helper.client.get_room(&room_id).unwrap();
140141
let timeline = Timeline::builder(&room)
141-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
142+
.with_focus(TimelineFocus::PinnedEvents {
143+
max_events_to_load: 100,
144+
max_concurrent_requests: 10,
145+
})
142146
.build()
143147
.await
144148
.unwrap();
@@ -210,7 +214,10 @@ async fn test_max_events_to_load_is_honored() {
210214

211215
let room = test_helper.client.get_room(&room_id).unwrap();
212216
let ret = Timeline::builder(&room)
213-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 1 })
217+
.with_focus(TimelineFocus::PinnedEvents {
218+
max_events_to_load: 1,
219+
max_concurrent_requests: 10,
220+
})
214221
.build()
215222
.await;
216223

@@ -249,7 +256,10 @@ async fn test_cached_events_are_kept_for_different_room_instances() {
249256
let room = test_helper.client.get_room(&room_id).unwrap();
250257
let (room_cache, _drop_handles) = room.event_cache().await.unwrap();
251258
let timeline = Timeline::builder(&room)
252-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 2 })
259+
.with_focus(TimelineFocus::PinnedEvents {
260+
max_events_to_load: 2,
261+
max_concurrent_requests: 10,
262+
})
253263
.build()
254264
.await
255265
.unwrap();
@@ -281,7 +291,10 @@ async fn test_cached_events_are_kept_for_different_room_instances() {
281291

282292
// And a new timeline one
283293
let timeline = Timeline::builder(&room)
284-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 2 })
294+
.with_focus(TimelineFocus::PinnedEvents {
295+
max_events_to_load: 2,
296+
max_concurrent_requests: 10,
297+
})
285298
.build()
286299
.await
287300
.unwrap();
@@ -305,7 +318,10 @@ async fn test_cached_events_are_kept_for_different_room_instances() {
305318

306319
// And a new timeline one
307320
let ret = Timeline::builder(&room)
308-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 2 })
321+
.with_focus(TimelineFocus::PinnedEvents {
322+
max_events_to_load: 2,
323+
max_concurrent_requests: 10,
324+
})
309325
.build()
310326
.await;
311327

@@ -331,7 +347,10 @@ async fn test_pinned_timeline_with_pinned_event_ids_and_empty_result_fails() {
331347

332348
let room = test_helper.client.get_room(&room_id).unwrap();
333349
let ret = Timeline::builder(&room)
334-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 1 })
350+
.with_focus(TimelineFocus::PinnedEvents {
351+
max_events_to_load: 1,
352+
max_concurrent_requests: 10,
353+
})
335354
.build()
336355
.await;
337356

@@ -355,7 +374,10 @@ async fn test_pinned_timeline_with_no_pinned_event_ids_is_just_empty() {
355374

356375
let room = test_helper.client.get_room(&room_id).unwrap();
357376
let timeline = Timeline::builder(&room)
358-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 1 })
377+
.with_focus(TimelineFocus::PinnedEvents {
378+
max_events_to_load: 1,
379+
max_concurrent_requests: 10,
380+
})
359381
.build()
360382
.await
361383
.unwrap();
@@ -390,7 +412,10 @@ async fn test_edited_events_are_reflected_in_sync() {
390412

391413
let room = test_helper.client.get_room(&room_id).unwrap();
392414
let timeline = Timeline::builder(&room)
393-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
415+
.with_focus(TimelineFocus::PinnedEvents {
416+
max_events_to_load: 100,
417+
max_concurrent_requests: 10,
418+
})
394419
.build()
395420
.await
396421
.unwrap();
@@ -469,7 +494,10 @@ async fn test_redacted_events_are_reflected_in_sync() {
469494

470495
let room = test_helper.client.get_room(&room_id).unwrap();
471496
let timeline = Timeline::builder(&room)
472-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
497+
.with_focus(TimelineFocus::PinnedEvents {
498+
max_events_to_load: 100,
499+
max_concurrent_requests: 10,
500+
})
473501
.build()
474502
.await
475503
.unwrap();
@@ -539,7 +567,10 @@ async fn test_edited_events_survive_pinned_event_ids_change() {
539567

540568
let room = test_helper.client.get_room(&room_id).unwrap();
541569
let timeline = Timeline::builder(&room)
542-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 })
570+
.with_focus(TimelineFocus::PinnedEvents {
571+
max_events_to_load: 100,
572+
max_concurrent_requests: 10,
573+
})
543574
.build()
544575
.await
545576
.unwrap();
@@ -648,6 +679,8 @@ async fn test_ensure_max_concurrency_is_observed() {
648679

649680
let pinned_event_ids: Vec<String> = (0..100).map(|idx| format!("${idx}")).collect();
650681

682+
let max_concurrent_requests: u16 = 10;
683+
651684
let joined_room_builder = JoinedRoomBuilder::new(&room_id)
652685
// Set up encryption
653686
.add_state_event(StateTestEvent::Encryption)
@@ -681,7 +714,7 @@ async fn test_ensure_max_concurrency_is_observed() {
681714
.set_body_json(pinned_event.json()),
682715
)
683716
// Verify this endpoint is only called the max concurrent amount of times.
684-
.expect(MAX_PINNED_EVENTS_CONCURRENT_REQUESTS as u64)
717+
.expect(max_concurrent_requests as u64)
685718
.mount(&server)
686719
.await;
687720

@@ -696,9 +729,10 @@ async fn test_ensure_max_concurrency_is_observed() {
696729

697730
// Start loading the pinned event timeline asynchronously.
698731
let handle = tokio::spawn({
699-
let timeline_builder = room
700-
.timeline_builder()
701-
.with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100 });
732+
let timeline_builder = room.timeline_builder().with_focus(TimelineFocus::PinnedEvents {
733+
max_events_to_load: 100,
734+
max_concurrent_requests: 10,
735+
});
702736
async {
703737
let _ = timeline_builder.build().await;
704738
}

0 commit comments

Comments
 (0)