Skip to content

Conversation

@Hywan
Copy link
Member

@Hywan Hywan commented Sep 12, 2025

After the merge of #5648, we want all events to get a TimelineEvent::timestamp value (extracted from origin_server_ts).

To accomplish that, we are emptying the event cache. New synced events will be built correctly, with a valid TimelineEvent::timestamp, allowing a clear, stable situation.

This is required to also solve a performance problem. When using the new room_list_service::sorter::recency, the TimelineEvent::timestamp() method is called a lot (depending of the number of rooms). If the TimelineEvent::timestamp field is empty (None), extract_timestamp will be called to… extract the timestamp from the origin_server_ts value, which implies parsing the event every time. This value isn't kept in-memory at this step because TimelineEvent::timestamp() is a getter. The timestamp value is set during the sync. Hence the clearing of the cache: we restart from fresh, and we are sure all events will have a correct timestamp value.


@Hywan Hywan requested a review from a team as a code owner September 12, 2025 12:19
@Hywan Hywan requested review from andybalaam, pixlwave, poljar and stefanceriu and removed request for a team and andybalaam September 12, 2025 12:19
Copy link
Member

@stefanceriu stefanceriu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll trust you know this is all of it. Ship it! 🔥

…::timestamp`.

After the merge of
matrix-org#5648, we want
all events to get a `TimelineEvent::timestamp` value (extracted from
`origin_server_ts`).

To accomplish that, we are emptying the event cache. New synced events
will be built correctly, with a valid `TimelineEvent::timestamp`,
allowing a clear, stable situation.
@Hywan Hywan force-pushed the fix-sqlite-empty-event-cache-because-of-timelineevent-timestamp branch from e3fdafa to 6f34829 Compare September 12, 2025 12:26
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 12, 2025

CodSpeed Performance Report

Merging #5665 will not alter performance

Comparing Hywan:fix-sqlite-empty-event-cache-because-of-timelineevent-timestamp (6f34829) with main (c2bc465)

Summary

✅ 49 untouched

@Hywan Hywan enabled auto-merge (rebase) September 12, 2025 12:54
@Hywan Hywan disabled auto-merge September 12, 2025 12:55
@Hywan Hywan enabled auto-merge (rebase) September 12, 2025 12:57
@Hywan Hywan merged commit 5ccbc1c into matrix-org:main Sep 12, 2025
53 checks passed
@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.37%. Comparing base (c2bc465) to head (6f34829).
⚠️ 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 66.66% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5665      +/-   ##
==========================================
- Coverage   88.38%   88.37%   -0.01%     
==========================================
  Files         355      355              
  Lines       97472    97481       +9     
  Branches    97472    97481       +9     
==========================================
+ Hits        86148    86152       +4     
- Misses       7260     7263       +3     
- Partials     4064     4066       +2     

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

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.

3 participants