[meta] Event Cache layers refactoring

[Hywan]

Blinking Sam (Samuel Johnson)

---

I would like to revisit the Event Cache design now it has grown a bit more and has more responsibilities. See the Problems Section to understand the motivation.

Current architecture

Architecture

The current architecture of the Event Cache is the following:

• EventCache
  • contains one RoomEventCache per room
    • contains all events for the main timeline in an EventLinkedChunk
    • contains one ThreadEventCache per thread
      • contains all events for the thread timeline in an EventLinkedChunk
    • contains one PinnedEventCache for the pinned events in an EventLinkedChunk

Sync

The sync flow is the following (super simplified):

• For each updated room, fetch the appropriate RoomEventCache
• Push all events in it
• Post-process the events
  • Copy and dispatch the threaded events
  • Copy and dispatch the pinned events

Pagination

The pagination flow is the following (super simplified):

• Use a dedicated method on a RoomEventCache to paginate:
  • One method for the main timeline
  • One method for threads
  • None for pinned events
  • Push the events in their own buckets

Problems

The major problems with this architecture are the following:

1. Threaded events and pinned events are first and foremost pushed in the RoomEventCache, before being copied over ThreadEventCache and PinnedEventCache.
   • It forces us to filter the threaded events from RoomEventCache, and possibly other events in the future
   • This is a waste of memory and disk usage
2. There is a non-negligible amount of duplicated code between RoomEventCache, ThreadEventCache and PinnedEventCache. Some are using a very similar read-write lock around their state (RoomEventCache and PinnedEventCache), some are sharing some parts of the pagination mechanism (RoomEventCache and ThreadEventCache).
   • The code is really confusing
   • Bugs can be duplicated
   • Fixing a bug can create a new bug somewhere else

Proposal

Pipelines

I propose to improve the sync flow by introducing pipelines. It basically reverses the post process to a pre process:

• Each event is dispatched in its own pipeline
• An event can be present in multiple pipelines, e.g. a pinned event must be part of PinnedEventCache, but also either RoomEventCache or ThreadEventCache

It brings nice benefits:

• Clear separation of events
• Clear knowledge of what we are manipulating where
• No more undesired duplicated events
• All events at the correct place: until now, edits of threaded events live in the RoomEventCache as far as I understand the code
• And… we can remove the dependency between RoomEventCache, and ThreadEventCache and PinnedEventCache. Let me explain.

I woud like to go from:

graph TD;
    EventCache-->RoomEventCache;
    RoomEventCache-->RoomEventCacheState;
    RoomEventCacheState-->ThreadEventCache;
    RoomEventCacheState-->PinnedEventCache;
    RoomEventCacheState-->EventFocusedCache;
    ThreadEventCache-->ThredEventCacheState;
    PinnedEventCache-->PinnedEventCacheState;
    EventFocusedCache-->EventFocusedCacheState;

Loading

to:

graph TD;
    EventCache-->RoomEventCaches;
    RoomEventCaches-->RoomEventCache;
    RoomEventCaches-->ThreadEventCache;
    RoomEventCaches-->PinnedEventCache;
    RoomEventCaches-->EventFocusedCache;
    RoomEventCache-->RoomEventCacheState;
    ThreadEventCache-->ThredEventCacheState;
    PinnedEventCache-->PinnedEventCacheState;
    EventFocusedCache-->EventFocusedCacheState;

Loading

where RoomEventCaches (plural… well, we might want a better name, maybe RoomEventCacheDispatcher or something? it's a bit long, anyway) would play the role of the dispatcher/pipeliner: it will distribute/dispatch the events in their associated buckets/caches.

This new design would:

• Prevent to abuse RoomEventCacheState to contain code related to the main timeline, but also threads, and pinned events. In order terms, it forces to have a clear separation of concerns, making the code robuster and easier to understand.
• This isn't mandatory but it helps to introduce new refactorings regarding the read-write lock and pagination. See next sections.

Common read-write lock implementation

RoomEventCacheState is embedded in an RoomEventCacheStateLock, which is nice (introduced in #5856). But now, PinnedEventCacheState also has its own PinnedEventCacheStateLock (introduced in #6085). It's duplicated code for a sensible, delicate logic.

I propose to have a common generic type for the lock.

struct StateLock<S>
{
    locked_state: RwLock<S>,
    state_lock_upgrade_mutex: Mutex<()>,
}

trait Store {
    fn store(&self) -> &EventCacheStoreLock;
}

impl<S> StateLock<S>
where
    S: Store
{
    pub async fn read(&self) -> Result<StateLockReadGuard<'_>, EventCacheError> {
        // the usual `read` implementation
    }

    pub async fn write(&self) -> Result<StateLockWriteGuard<'_>, EventCacheError> {
        // the usual `write` implementation
    }
}

pub struct StateLockReadGuard<'a, S> {
    state: RwLockReadGuard<'a, S>,
    store: EventCacheStoreLockGuard,
}

pub struct StateLockWriteGuard<'a, S> {
    state: RwLockWriteGuard<'a, S>,
    store: EventCacheStoreLockGuard,
}

impl<'a, S> StateLockWriteGuard<'a, S> {
    fn downgrade(self) -> StateLockReadGuard<'a, S> {
        // the usual `downgrade` implementation
    }
}

If we need more methods on these read and write guards (as it's the case today), we can implement them like so:

impl<'a> StateLockReadGuard<'a, RoomEventCacheState> {
    pub async fn find_event(&self, …) { … }
    // etc.
}

Common pagination implementation

For the moment, here is what we have:

• room_event_cache.pagination().run_backwards_once() to paginate the main timeline/RoomEventCache over the database and the network,
• room_event_cache.paginate_thread_backwards() to paginate over the network only (because there is no storage yet),
• nothing for pinned-events (which makes sense for the moment)

The pagination is cut in multiple steps: most of them makes sense but they are combined manually to produce the desired behaviour. For RoomEvenCache, almost everything happens in Pagination, whilst for ThreadEventCache, it all lives in the RoomEventCache::paginate_thread_backwards method (not in Pagination, not in ThreadEventCache, but in RoomEventCache).

The major difference between a pagination for RoomEventCache and ThreadEventCache is the HTTP endpoint: /messages vs. /relations. It's too bad we don't use Pagination for threads because this API is really well-designed.

With the pipelines and the new design, each cache can have its own Pagination type. It will also help to provide a PaginationStatus to all kind of caches (which is missing for ThreadEventCache as we speak). The Pagination type could be improved to take a generic parameter:

pub struct Pagination<C>
where
    C: Paginable,
{
    subject: C,
}

trait Paginable {
    async fn load_more_events_backwards_from_network(&self) -> Result<Vec<Event>>;
    async fn load_more_events_backwards_from_disk(&self) -> Result<LoadMoreEventsBackwardsOutcome>;

    fn status(&self) -> &SharedObservable<PaginationStatus>;
    fn back_token_notifier(&self) -> &Notify;
    fn waited_for_initial_prev_token(&mut self) -> &mut bool;

    fn send_updates(&self, …);
    fn room(&self) -> Option<Room>;
}

This way, we can implement the following methods:

• RoomEventCache::pagination(&self) -> Pagination<Self>
• ThreadEventCache::pagination(&self) -> Pagination<Self>

The matrix_sdk_ui::timeline::Timeline type will also be happier with a uniform Pagination type, it will simplify the code a little bit more.

Execution

The goods news is that it doesn't require too much work. The tasks can be done in parallel without too much troubles, and they seem to be rather small and self-contained for what I foresee. For example, the work on the read-write lock can be done now without blocking other projects happening on the Event Cache right now.

Tasks

• Pipelines
• New architecture
  • chore(sdk): Reorganise files and modules in event_cache #6216
  • chore(sdk): Clean up event_cache/caches/room/ #6219
  • chore(sdk): Extract EventCache::*_task to new tasks module #6222
  • chore(sdk): Create the Caches type #6223
  • feat(sdk): Create ThreadEventCacheState #6280
  • refactor(sdk): Inline RoomEventCacheStateLockWriteGuard::load_more_events_backwards in RoomPagination #6288
  • feat(sdk): Flatten the hierarchy of caches in the Event Cache, part 1: Thread #6517
  • feat(sdk): Flatten the hierarchy of caches in the Event Cache, part 2: Pinned events #6568
  • feat(sdk): Flatten the hierarchy of caches in the Event Cache, part 3: Event-focused #6603
  • feat(sdk): Flatten the hierarchy of caches in the Event Cache, bonus: Recompute the ThreadSummary when an in-thread event is decrypted #6619
• Shared state lock, refactor(sdk): Introduce the StateLock type in EventCache #6154
• Unique state for the entire Event Cache
  • feat(sdk): Add a unique type that holds all the Event Cache states #6629
  • Ensure all caches are shrunk when needed
    • fix(sdk): Introduce SubscribersHandle and SubscriberHandle #6684
    • feat(sdk): All caches have an auto-shrink mechanism #6690
• Shared pagination, refactor(sdk): Introduce the shared Pagination type and PaginatedCache trait #6174

To do after the main tasks:

• Re-enable test_latest_thread_event_is_redecrypted_and_updated
  • feat(sdk): Flatten the hierarchy of caches in the Event Cache, bonus: Recompute the ThreadSummary when an in-thread event is decrypted #6619