feat(sqlite): Improve throughput of SqliteEventCacheStore::load_all_chunks_metadata by 1140%#5411
Merged
bnjbvr merged 2 commits intomatrix-org:mainfrom Jul 21, 2025
Conversation
This patch changes the query used by `SqliteEventCacheStore::load_all_chunks_metadata`. It was the cause of severe slowness. The new query improves the throughput by +1140% and the time by -91.916%. The benchmark will follow in the next patch. Metrics for 10'000 events (with 1 gap every 80 events). - Before: - throughput: 20.686 Kelem/s, - time: 483.43 ms. - After: - throughput: 253.52 Kelem/s, - time: 39.478 ms. This query will visit all chunks of a linked chunk with ID `hashed_linked_chunk_id`. For each chunk, it collects its ID (`ChunkIdentifier`), previous chunk, next chunk, and number of events (`num_events`). If it's a gap, `num_events` is equal to 0, otherwise it counts the number of events in `event_chunks` where `event_chunks.chunk_id = linked_chunks.id`. Why not using a `(LEFT) JOIN` + `COUNT`? Because for gaps, the entire `event_chunks` will be traversed every time. It's extremely inefficient. To speed that up, we could use an `INDEX` but it will consume more storage space. Finally, traversing an `INDEX` boils down to traverse a B-tree, which is O(log n), whilst this `CASE` approach is O(1). This solution is nice trade-off and offers great performance.
load_all_chunks_metadata by 1140%SqliteEventCacheStore::load_all_chunks_metadata by 1140%
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #5411 +/- ##
=======================================
Coverage 88.80% 88.81%
=======================================
Files 334 334
Lines 91256 91266 +10
Branches 91256 91266 +10
=======================================
+ Hits 81044 81059 +15
+ Misses 6399 6393 -6
- Partials 3813 3814 +1 ☔ View full report in Codecov by Sentry. |
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
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 patch changes the query used by
SqliteEventCacheStore::load_all_chunks_metadata. It was the cause of severe slowness (see #5407). The new query improves the throughput by +1140% and the time by -91.916%.This query will visit all chunks of a linked chunk with ID
hashed_linked_chunk_id. For each chunk, it collects its ID (ChunkIdentifier), previous chunk, next chunk, and number of events (num_events). If it's a gap,num_eventsis equal to 0, otherwise it counts the number of events inevent_chunkswhereevent_chunks.chunk_id = linked_chunks.id.Why not using a
(LEFT) JOIN+COUNT? Because for gaps, the entireevent_chunkswill be traversed every time to always count zero events (a gap has no event). It's extremely inefficient. To speed that up, we could use anINDEXbut it will consume more storage space. Finally, traversing anINDEXboils down to traversing a B-tree, which is O(log n), whilst thisCASEapproach is O(1). This solution is a nice trade-off and offers great performance.Note
Please, note that the more gaps, the slower the query was. Now, the number of gaps isn't impactful.
This change doesn't involve a new schema, no migration is necessary.
Metrics for 10'000 events (with 1 gap every 80 events)
Before
After