element-hq / element-hq/synapse
Synapse synchrotron performance for timeline entries can be improved
- Dominant language
- Python
- Stars
- 4.6k
- Forks
- 600
- Avg merge
- 5d 22h
- Merged PRs (30d)
- 51
Description
This issue has been migrated from [#13327](https://github.com/matrix-org/synapse/issues/13327).
---
When Synapse works out a /sync response, it should:
- Use a more conservative [load limit](https://github.com/matrix-org/synapse/blob/5099b5ecc735b98ac9d559ef6191554bafff964b/synapse/handlers/sync.py#L540) when the timeline limit is set to low numbers like 1. Otherwise, Synapse will load up way more events than it needs to (10 instead of 1), which then has knock-on implications as each event then has state-before-the-event fetched to do history visibility checks. Ideally this would be set to the `timeline_limit`, but Erik has mentioned good reasons why maybe it shouldn't be:
> I think all of that code is to make sure we correctly handle the case where we filter out events that we received from the DB, so we always load at least 10 to make sure we don't pointlessly pull out things one by one. We can filter events out for other reasons too, e.g. they don't have permission to see the message (as it was before they'd joined or whatever).
Perhaps as a compromise the `load_limit` could eagerly try being the `timeline_limit` and then progressively increase on repeated queries if it cannot be satisfied. This means in the 'happy path' we can cut down on pulling out literally an order of magnitude more events (when timeline limit is 1).
- Bail early when the timeline events have been satisfied. `filter_events_for_client` is expensive as it loads the state-before-the-event, and it's called for every [loaded event](https://github.com/matrix-org/synapse/blob/5099b5ecc735b98ac9d559ef6191554bafff964b/synapse/handlers/sync.py#L588) (which is compounded when this includes events the client didn't ask for). This hard work is then [discarded](https://github.com/matrix-org/synapse/blob/5099b5ecc735b98ac9d559ef6191554bafff964b/synapse/handlers/sync.py#L607) - instead it would be good if `filter_events_for_client` bailed early the moment it had the required number of events. This could just be an optional kwarg on the function to bail once it has a certain number of events?
### Background
I have been profiling sync v2 performance in an effort to try to identify areas where performance can be improved, mostly as a stop-gap until sliding sync can land. The key part to mention is this image:

The red lines are the time spent to calculate the `timeline` section of the response, specifically [this code](https://github.com/matrix-org/synapse/blob/5099b5ecc735b98ac9d559ef6191554bafff964b/synapse/handlers/sync.py#L1999-L2018). In an effort to reduce this time, clients can send up a `/sync` request with a timeline limit of 1. However, this doesn't result in much improvement because of the two points mentioned above. If those issues were resolved, setting a low timeline limit could significantly speed up sync v2.
Contributor guide
Research direction
Start in synapse/handlers/sync.py at the load-limit logic around the /sync timeline handling and the timeline response code referenced in the issue. Examine filter_events_for_client and its use for loaded events; done means low timeline limits avoid unnecessary event and state-before-event work while still returning the requested visible timeline events correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, backend, performance
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 42/100