feat(sdk): Flatten the hierarchy of caches in the Event Cache, part 1: Thread#6517
Draft
Hywan wants to merge 29 commits intomatrix-org:event-cache-refactoringfrom
Draft
feat(sdk): Flatten the hierarchy of caches in the Event Cache, part 1: Thread#6517Hywan wants to merge 29 commits intomatrix-org:event-cache-refactoringfrom
Hywan wants to merge 29 commits intomatrix-org:event-cache-refactoringfrom
Conversation
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
5 times, most recently
from
8109f7f to
0e243c6
Compare
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
3 times, most recently
from
efc8a37 to
c9c1f45
Compare
Hywan
changed the title
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
from
98d95f8 to
01ccd32
Compare
stefanceriu
reviewed
May 8, 2026
Member
stefanceriu
left a comment
stefanceriu
left a comment
There was a problem hiding this comment.
This is significantly above my pay grade but having read the plan and the implementation, commit by commit, it makes perfect sense and I think it's a great addition/change. Nicely done! 👏
| /// an event that is not a thread root. | ||
| /// | ||
| /// [`ThreadEventCache`]: thread::ThreadEventCache | ||
| pub async fn thread( |
Member
There was a problem hiding this comment.
Should this be called get or create then? 🤔
| let Some(event_id) = | ||
| extract_redaction_target(event.raw(), &self.room_version_rules.redaction) | ||
| else { | ||
| warn!("missing target event id from the redaction event"); |
Member
There was a problem hiding this comment.
Are you sure you want to print this out of any event that's not a redaction.
|
|
||
| let mut associated_thread_root = None; | ||
|
|
||
| for (thread_root, thread) in existing_threads { |
Member
There was a problem hiding this comment.
Can it be that the target thread cache doesn't exist locally yet? Does the redaction need to be stored and applied later if that's the case?
| /// | ||
| /// This will remove all the information related to the event cache, in | ||
| /// memory and in the persisted storage, if enabled. | ||
| pub async fn clear_event_cache_storage(&self) -> Result<(), ClientError> { |
Member
There was a problem hiding this comment.
Should probably add a changelog for this.
Member
Author
There was a problem hiding this comment.
Yes, I'll update the changelogs when merging event-cache-refactoring to main to avoid conflicts, but thanks for noticing!
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
from
1e854ef to
28a9795
Compare
This patch implements `StateLock::write_owned` along with the `OwnedStateLockWriteGuard`. This is going to be useful to acquire an exclusive owned lock when we will need to reset all the caches.
…o `ThreadEventCacheState` directly. This method moves a couple of methods to `ThreadEventCacheState` so they can be shared with other types in the next patches.
This patch implements the exclusive owned lock guard for the thread cache state.
This patch starts flattening caches by starting the extract thread caches from the room cache. This patch comments some methods or adds a `todo!()` to make the code compile. It's the price for smaller commits. The thread cache gains new methods like `handle_joined_room_update` or `handle_joined_left_update`. It mimics the flow in the room cache. A new `caches::aggregator` module is created to introduce “pipelines”: a `Timeline` is created for each cache. It lives in front of the room cache, and basically reverses the “post process” flow to a “pre process” flow. More commits will improve it. `ResetCaches` is now also responsible to clear the threads in addition to the room (previously, the room cache was responsible of that).
This patch removes `RoomEventCache::thread_pagination` as it is now available on `ThreadEventCache::pagination`.
This patch removes `RoomEventCache::subscribe_to_thread` as it is now available on `ThreadEventCache::subscribe`.
This patch moves the `find_event` implementation in `persistence` with the idea to share it with the thread caches.
…ions` to `persistence`. This patch moves the `find_event_relations` and `find_event_with_relations` implementations in `persistence` with the idea to share it with the thread caches.
…oom's post process to thread's post process. The room is post-processing thread events. This patch moves the handling of `bunlded_latest_thread_event` from `RoomEventCache` to `ThreadEventCache`.
…to `ThreadEventCache`. This patch updates the `ThreadSummary` flow. `Caches` ask each thread to compute its `ThreadSummary` and ask the room to update/store it. The edit events targeting in-thread events are now stored in the thread cache! This is new as it was only stored in the room cache previously. This unlocks the possibility to correctly compute the latest event for a thread with the `matrix_sdk::latest_events` API in the future.
…` and `ThreadEventCache`. This patch handles `m.room.redaction` for thread caches. This is a naive approach for now to have small commits.
This patch removes `RoomEventCacheState::threads` along with `get_or_reload_patch` and `update_threads`.
This patch removes `RoomEventCache::find_event_in_thread` as we now have `ThreadEventCache::find_event`.
…Guard::post_process_new_events`. This patch cleans up `post_process_new_events` arguments that were specific to thread or thread summary. I am aware that R2D2 had special treatement here, and it's going to be re-introduced later.
This patch adds `EventCache::thread`, sibling of `EventCache::room`. Finally!
This patch updates `TimelineFocusKind` to hold its own event cache type for each kind of timeline. The idea is to stop fetching each cache for every action and makes some path infallible. The performance are also perceivably faster according to a real test by Jorge on Android.
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
from
28a9795 to
4ef3d0b
Compare
…ciated threads. This patch stores all events with a relation to an in-thread event, plus the `m.room.redaction` events targeting in-thread events, in their respective thread cache. This patch also introduces a new `extract_redaction_target` helper, that is used in the `maybe_apply_new_redaction` methods of the room and the thread cache. The aggregator for the threads is updated to aggregate the `m.room.redaction` event, and annotation relation type. A refactoring is done to use a `match` over `RelationType`, instead of using multiple `extract_*` helper, which clarifies the code I believe. A limitation has been found and a `TODO` has been added to keep the commits “small”.
…` `Timeline`. Previously, because a `ThreadEventCache` didn't receive all its events, a `Timeline` with a `Thread` focus had to be updated via two sources: updates from `RoomEventCache` and from `ThreadEventCache`. Now that this problem is solved, i.e. a `ThreadEventCache` contains all its events, a `Timeline` with a `Thread` focus doesn't have to be updated by two sources. This is what this patch does.
This patch updates the `compute_thread_summary` method on the thread cache to use the `matrix_sdk::latest_events` API. It bends it, also a crime. We don't use it at its full potential, but at least it fixes several bugs and makes it more aligned with the room latest events: it skips `m.room.redaction` (new), the majority of the state events (new), mostly correctly handle edits (new) etc. This patch also allows the system to erase the `ThreadSummary` if none can be computed.
Hywan
force-pushed
the
feat-sdk-event-cache-flatten-threads
branch
from
4ef3d0b to
19d6050
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR constitutes the first part of a series of 2 or 3 PR. It's about flattening the hierarchy of caches (see #6152). It starts with threads.
Why do we want to do that? The room cache owns the thread caches, the pinned-events cache, and the event-focused cache. Not only this is bad from a design point of view, but it also creates bugs. Notably, the room cache has a lock to access its state, that needs to be acquired to access to, for example, the threads, which have their own locks for each of their states. We have a hierarchy of locks and it's easy to get a deadlock (see the very recent #6542 for example).
Also, this hierarchy of caches make it hard to unit test and to share code.
This PR starts by “flattening” the thread caches, i.e. it extracts the threads from the room cache to move them directly in
Caches. The tree goes from:graph TD; EventCache-->Caches; Caches-->RoomEventCache; RoomEventCache-->RoomEventCacheState; RoomEventCacheState-->ThreadEventCache; ThreadEventCache-->ThreadEventCacheState;to:
graph TD; EventCache-->Caches; Caches-->RoomEventCache; Caches-->ThreadEventCache; RoomEventCache-->RoomEventCacheState; ThreadEventCache-->ThreadEventCacheState;Many tests must be updated which increases the
diffstat, but do not be afraid, the patches are as small and atomic as possible. This is mostly code moves, except the newaggregatormodule (see below), which explains the difference between additions and deletions.Breaking Changes
All methods related to threads on
RoomEventCacheare removed, for example:RoomEventCache::thread_paginationis nowThreadEventCache::pagination, orRoomEventCache::subscribe_to_threadis nowThreadEventCache::subscribe.EventCachenow provides 2 new methods:room(previouslyfor_room) andthreadto access to aRoomEventCacheand aThreadEventCache.New Features
Because, yeah, we have new features coming with this refactoring!
Pipelines
As anticipated, we need “pipelines” under the form of
event_caches::caches::aggregator. Pipelines are actually necessary to make this flattening possible.An aggregator takes a
Timelineand returns a newTimelinetailored for a particular cache. Two aggregators exist so far:aggregate_timeline_for_room,aggregate_timeline_for_threads.It transforms the “post-processing of events“ that lived in the room cache to a “pre-processing of events”. It allows a couple of nice things, such as:
Timelinevalue,The room cache gets all the events for the moment, it's simpler like this, until threads are always enabled at least. The thread cache initially got all the in-thread events. However, it excluded the edit of in-thread events (because an edit of an in-thread event doesn't have a relation to the thread!), and the redaction of in-thread events. Now, the events for edits and redactions of in-thread events exist in the threads! This is new and it unlocks nice future features, like the fact we can use the
matrix_sdk::latest_eventsAPI to compute the latest event for theThreadSummarycorrectly since we have all the edits, redactions etc.Improved
ThreadSummaryOne of the last commit update
ThreadSummaryto use an internal API ofmatrix_sdk::latest_events: we don't use the Latest Event API at its maximum potential, but at least it fixes a couple of bugs we had withThreadSummary. This is not very efficient right now, but I prefer to be correct first. At least, now, we have a single place to think about the latest event!CHANGELOG.mdfiles.