Various smaller sliding sync fixes #1372

gnunicorn · 2023-01-18T11:54:35Z

Various sliding sync fixes, providing tests where possible. ref #1359

more tracing around unfreeze code for better performance tracking
when recovering from cold or going live the first time, we used to fire a bunch of Empty-Insert events for every room in the total count. With this change, we are building the new list once and set it instead, leading to a much more feasible single Replace event being triggered
the view generators would only update upon the next loop iteration following the current cycle and they would update regardless of whether they actually succeeded, leading to two annoying bugs: live state only being set at the beginning of the next cycle and growing sync some times skipping beats. This changes the code to do a post-processing of the view-generator code after we've received the new update - based on the data of the update. Leading to one request less before live being triggered and fixing beat-skipping.
selective caching (part 1): we were caching the entire object at once, which meant that if you loaded a lot of rooms, deserializing that would take quite some times (we've seen 800ms being spent on that). The code was refactored to store room data alongside the view it belongs to allowing the API to select the views (by adding them before built) that are even attempted to look up. For me that drops the unfreezing time from 150ms to 30ms for a 14item selective view (ignoring a full-growing-sync-view).
Cancel stoppable spawn on drop, add helper fn subscribe_and_add_timeline_listener #1361 allows us to cancel the spawn of the sync loop - but that means we now might also miss updates sent by the server as any .await might get interrupted and thus processing of the results could fail. We can see in the logs that this happening and might cause gaps in the room list even. This has been fixed by switching to an AtomicBool to inform the inner loop to stop after every iteration rather than cancelling the actual JoinHandle.
properly reset the timeline if the server tells us to (via limited flag) fixes the problem of gappy timelines when the room ran out of scope and came back in.
checker whether a given room id is in the visible range and an optional flag to ask the view to trigger updates for rooms that are in the view regardless of whether their positions was updated in the view.

crates/matrix-sdk/src/sliding_sync.rs

…ache

Before we'd push one empty entry at a time, which lead to a lot of vecdiff updates pushed. With this we now only push one event after the entire operation set has been applied on the fresh or cold view.

…zing

…rocessing before stopping the loop

…thout positional update

bindings/matrix-sdk-ffi/src/sliding_sync.rs

poljar

Look good, left a couple of nits.

poljar · 2023-01-24T08:58:29Z

testing/sliding-sync-integration-test/src/lib.rs

@@ -167,154 +177,239 @@ mod tests {
    // index-based views don't support removing views. Leaving this test for an API
    // update later.
    //
-    // #[tokio::test(flavor = "multi_thread", worker_threads = 4)]


What's up with this test? It seems to be the same one as further bellow with the same name, which is ignored instead of commented out.

there is no commented-out one further below anymore. I removed the comment and instead opted for ignoring it for better visibility. This is probably a mixup in the history... the final result only has the ignored one.

gnunicorn · 2023-01-24T13:47:39Z

bindings/matrix-sdk-ffi/src/sliding_sync.rs

@@ -491,10 +505,8 @@ impl SlidingSyncView {
    ) -> Arc<StoppableSpawn> {
        let mut room_list = self.inner.rooms_list.signal_vec_cloned().to_stream();
        Arc::new(StoppableSpawn::with_handle(RUNTIME.spawn(async move {
-            loop {


grrr... leftover from a bad refactor that wasn't fully reverted: we still need this loop { here ...caused stupid bug...

gnunicorn requested a review from a team January 18, 2023 11:54

gnunicorn force-pushed the ben-fixing-various-1359 branch 2 times, most recently from 7232f65 to c267be8 Compare January 18, 2023 14:52

jplatte reviewed Jan 18, 2023

View reviewed changes

crates/matrix-sdk/src/sliding_sync.rs Outdated Show resolved Hide resolved

crates/matrix-sdk/src/sliding_sync.rs Outdated Show resolved Hide resolved

gnunicorn requested a review from jplatte January 18, 2023 16:02

gnunicorn mentioned this pull request Jan 18, 2023

Sliding Sync Stabilization tracking #1359

Closed

16 tasks

gnunicorn added 4 commits January 18, 2023 16:09

chore(sliding-sync): additional tracing around unfreezing from cold c…

970b6b6

…ache

fix(sliding-sync): use single replace on cold and empty views

366b7f4

Before we'd push one empty entry at a time, which lead to a lot of vecdiff updates pushed. With this we now only push one event after the entire operation set has been applied on the fresh or cold view.

fix(sliding-sync): have views go live right at response processing

06a3229

tests: additional ignored tests for future scenarios

682f547

gnunicorn force-pushed the ben-fixing-various-1359 branch from 2d071a2 to 6cc1e7d Compare January 18, 2023 16:09

gnunicorn added 5 commits January 18, 2023 20:53

feat(sliding-sync): cache room data next to view for selective unfree…

6e410e7

…zing

style: use trace! instead of tracing::trace!

5b584bb

fix(ffi): don't cancel the actual spawn, instead ensure it finishes p…

5a36f72

…rocessing before stopping the loop

chore(base): additional tracing information for sliding sync responses

825489a

fix(sliding-sync): switch Atomic Ordering from Relaxed to SeqCst

b8fdbb1

gnunicorn force-pushed the ben-fixing-various-1359 branch from 1749e08 to b8fdbb1 Compare January 18, 2023 20:53

fix(sliding-sync): refresh timeline if server told us we are limited

a632355

gnunicorn requested a review from a team January 18, 2023 22:43

gnunicorn force-pushed the ben-fixing-various-1359 branch 3 times, most recently from 27d33f5 to e84341f Compare January 23, 2023 16:32

feat(sliding-sync): add configuration to inform about room changes wi…

223b99f

…thout positional update

gnunicorn force-pushed the ben-fixing-various-1359 branch from e84341f to 223b99f Compare January 23, 2023 16:43

poljar reviewed Jan 24, 2023

View reviewed changes

bindings/matrix-sdk-ffi/src/sliding_sync.rs Show resolved Hide resolved

poljar approved these changes Jan 24, 2023

View reviewed changes

gnunicorn merged commit 24ac5b9 into main Jan 24, 2023

gnunicorn deleted the ben-fixing-various-1359 branch January 24, 2023 10:30

gnunicorn commented Jan 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various smaller sliding sync fixes #1372

Various smaller sliding sync fixes #1372

gnunicorn commented Jan 18, 2023 •

edited

poljar left a comment

poljar Jan 24, 2023

gnunicorn Jan 24, 2023

gnunicorn Jan 24, 2023

Various smaller sliding sync fixes #1372

Various smaller sliding sync fixes #1372

Conversation

gnunicorn commented Jan 18, 2023 • edited

poljar left a comment

Choose a reason for hiding this comment

poljar Jan 24, 2023

Choose a reason for hiding this comment

gnunicorn Jan 24, 2023

Choose a reason for hiding this comment

gnunicorn Jan 24, 2023

Choose a reason for hiding this comment

gnunicorn commented Jan 18, 2023 •

edited