Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sdk): Implement event_cache::Deduplicator #3580

Closed
wants to merge 14 commits into from

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Jun 19, 2024

This is WIP of an event deduplicator implemented on top of a Bloom filter. It works great, but it doesn't solve all issues we have right now. Right now, it filters duplicated events; instead, it should collect duplicated events with their positions, so that they can be removed from the LinkedChunk before being re-inserted. It's the basis to implement a naive reconciliation in the EventCache.


@Hywan
Copy link
Member Author

Hywan commented Jul 4, 2024

I have notice we have a growable bloom filter in our direct deps already, we should use it instead of introducing a new one:

growable-bloom-filter = "2.1.0"

@Hywan Hywan force-pushed the feat-sdk-event-cache-dedup-events branch 2 times, most recently from bb7fcd1 to 713cc08 Compare October 21, 2024 12:59
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

Attention: Patch coverage is 87.05036% with 18 lines in your changes missing coverage. Please review.

Project coverage is 84.56%. Comparing base (95ae5d1) to head (b578f38).
Report is 120 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk/src/event_cache/store.rs 83.67% 8 Missing ⚠️
...tes/matrix-sdk/src/event_cache/linked_chunk/mod.rs 82.35% 6 Missing ⚠️
...trix-sdk/src/event_cache/linked_chunk/as_vector.rs 92.10% 3 Missing ⚠️
crates/matrix-sdk/src/event_cache/deduplicator.rs 94.44% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3580      +/-   ##
==========================================
- Coverage   84.70%   84.56%   -0.14%     
==========================================
  Files         269      270       +1     
  Lines       28766    28862      +96     
==========================================
+ Hits        24366    24407      +41     
- Misses       4400     4455      +55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Hywan Hywan force-pushed the feat-sdk-event-cache-dedup-events branch from 134d7f6 to b5224aa Compare October 22, 2024 10:07
@Hywan Hywan force-pushed the feat-sdk-event-cache-dedup-events branch 2 times, most recently from 6fecc07 to 5b1949c Compare October 22, 2024 14:47
Hywan added 12 commits October 28, 2024 10:59
This patch adds an `Event` type alias to `SyncTimelineEvent` to (i) make
the code shorter, (ii) remove some cognitive effort, (iii) make things
more convenient.
This patch adds unit tests for the `RoomEvents`' methods.
This patch adds the `LinkedChunk::remove_item_at` method, along with
`Update::RemoveItem` variant.
This is only code move, nothing has changed.
This is a clean up patch, nothing fancy.
This is another clean up patch.
This patch fixes a bug in an optimisation inside `UpdateToVectorDiff`
when an `Update::PushItems` is handled. It can sometimes create
`VectorDiff::Append` instead of a `VectorDiff::Insert`. The tests will
be part of the next patch.
…ateToVectorDiff`.

This patch implements the support of `Update::RemoveItem` inside
`UpdateToVectorDiff` to emit a `VectorDiff::Remove`.
@Hywan Hywan force-pushed the feat-sdk-event-cache-dedup-events branch from e75192f to b578f38 Compare October 28, 2024 09:59
@Hywan
Copy link
Member Author

Hywan commented Oct 28, 2024

It works. End of the experiment. This PR has been split into #4172, #4173 and #4174.

@Hywan Hywan closed this Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant