matrix-org / matrix-org/matrix-rust-sdk

Thread event cache inserts a gap on every non-limited sync (rooms don't)

Open
#7,036 3 comments 0 reactions 1 assignee Claimed by @Hywan View on GitHub
Dominant language
Rust
Stars
2.3k
Forks
500
Avg merge
1d 16h
Merged PRs (30d)
106

Description

# Summary

`ThreadEventCacheState::handle_sync` turns the forwarded `prev_batch` token into a Gap unconditionally. The room cache drops the token on a non-limited sync when it already holds events; the thread cache has no such check. Since the aggregator forwards the room's prev_batch to every thread with an event in the sync — including ordinary non-limited syncs — each live thread reply arriving in its own sync inserts a gap plus a fresh single-event item chunk.

This seems to render the thread cache all but useless when it is built up via live-sync, due to the constant unnecessary gaps.

# Code

Thread, unconditional: [thread/state.rs#L464](https://github.com/matrix-org/matrix-rust-sdk/blob/ac0e7810b7dbc904af31517ff2d2c6a369adc72c/crates/matrix-sdk/src/event_cache/caches/thread/state.rs#L464) and [#L493](https://github.com/matrix-org/matrix-rust-sdk/blob/ac0e7810b7dbc904af31517ff2d2c6a369adc72c/crates/matrix-sdk/src/event_cache/caches/thread/state.rs#L493)
Room, guarded: [room/state.rs#L551](https://github.com/matrix-org/matrix-rust-sdk/blob/ac0e7810b7dbc904af31517ff2d2c6a369adc72c/crates/matrix-sdk/src/event_cache/caches/room/state.rs#L551)
Token forwarded verbatim to threads: [aggregator.rs#L68](https://github.com/matrix-org/matrix-rust-sdk/blob/ac0e7810b7dbc904af31517ff2d2c6a369adc72c/crates/matrix-sdk/src/event_cache/caches/aggregator.rs#L68)
Introduced in [3d975fe "feat(sdk): Insert gaps in threads."](https://github.com/matrix-org/matrix-rust-sdk/commit/3d975fe93dfb9b31586feb26495b2edd44a20af2), part of #6671 — the same PR that added the room-side guard. The thread tests only cover `limited: true` with a token and `limited: false` without one, so the failing combination is untested.

# Effect (measured, web client, IndexedDB store)

8 thread replies sent one per sync: +8 gaps, +8 item chunks in the thread's linked chunk (one event per chunk). 8 room messages sent the same way: +0 / +0. On the next thread open, each gap costs a /relations round trip to re-fetch an event already in the store (dedup then discards it), and after that the history is left as adjacent single-event chunks, so back-pagination emits one timeline update per event. Same code path on SQLite; the store only changes per-chunk latency.

This also diverges from the intended behaviour written in #5123 ("Smart Edition"): known thread + new sync event → append; token only for unknown threads or gappy (limited) syncs.

# Proposed fix

Mirror the room guard:

```rust
let mut prev_batch_token = timeline.prev_batch.take();
if !timeline.limited && self.state.thread_linked_chunk.events().next().is_some() {
prev_batch_token = None;
}
```

Empty threads keep their initial gap; limited syncs are unchanged. With this, the same 8-reply run yields +0 gaps / +0 chunks and the reopen makes one gap fetch (the front gap) instead of eight.

Patch with a test covering the three branches is ready; happy to open a PR.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.