feat(sqlite): make sqlite's implementation of load_all_chunks_metadata even faster

[bnjbvr]

#5411 was already an immense improvement for the loading of the chunks metadata, but running the benchmark on my machine it can take up to 25 ms for a single room with 10K events, which is unfortunately still a lot (considering we can have hundreds/thousands of rooms).

The strategy used there is good:

1. select all chunks
2. for each event chunk, run a query to count the number of events

This implies that we'll have one additional select query per event chunk. This gave me the idea for this strategy:

• in a single query, get all the chunk ids and associated number of events, using a plain COUNT + GROUP BY, and put that into a hashmap (from chunk id to number of events in that chunk)
• in another query, get all the chunk ids / next / prev.

Then, when reforming the chunk metadata, use the hashmap to get the associated number of events. This is doing step 2 of the initial strategy in the app, to avoid running an extra SELECT query per event chunk.

This leads to the following improvement, as reported by cargo bench on the new benchmark:

reading/metadata/sqlite store/10000
                        time:   [1.9737 ms 2.0377 ms 2.0966 ms]
                        thrpt:  [4.7697 Melem/s 4.9075 Melem/s 5.0666 Melem/s]
                 change:
                        time:   [-92.816% -92.595% -92.388%] (p = 0.00 < 0.05)
                        thrpt:  [+1213.7% +1250.5% +1292.0%]
                        Performance has improved.
reading/metadata/sqlite store/100000
                        time:   [23.049 ms 23.256 ms 23.424 ms]
                        thrpt:  [4.2692 Melem/s 4.3000 Melem/s 4.3385 Melem/s]
                 change:
                        time:   [-99.277% -99.269% -99.260%] (p = 0.00 < 0.05)
                        thrpt:  [+13410% +13583% +13729%]
                        Performance has improved.

That is, it takes around 2ms for loading the metadata of a single room, instead of the initial 25ms that it took on my machine.

[codecov]

Codecov Report

Attention: Patch coverage is 68.42105% with 6 lines in your changes missing coverage. Please review.

Project coverage is 88.85%. Comparing base (165ec9d) to head (92d7349).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-sqlite/src/event_cache_store.rs	68.42%	0 Missing and 6 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5425   +/-   ##
=======================================
  Coverage   88.85%   88.85%           
=======================================
  Files         333      333           
  Lines       91244    91252    +8     
  Branches    91244    91252    +8     
=======================================
+ Hits        81072    81082   +10     
+ Misses       6348     6344    -4     
- Partials     3824     3826    +2     

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

[poljar]

Seems sensible, left a question.

[poljar]

Doesn't a gap have 0 events by definition? Making this if/else redundant?

[bnjbvr]

It is, but my local testing showed it's 1% slower to not have this branch. The theory is that, at least on my machine, it's faster to allocate the chunk type string and compare against it, than it is to do a hashmap lookup that results in a cache miss.

But 1% doesn't matter much, so happy to get rid of it if you prefer?

[poljar]

Nah it's fine. A comment explaining this might be nice though.

[Hywan]

Fantastic!