diff --git a/changelog.d/17788.misc b/changelog.d/17788.misc
new file mode 100644
index 0000000000..1ef6f6e2ba
--- /dev/null
+++ b/changelog.d/17788.misc
@@ -0,0 +1 @@
+Sliding sync minor performance improvement by omitting unchanged data from incremental responses.
diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py
index cb6a0b9f35..8c12cea8eb 100644
--- a/synapse/handlers/sliding_sync/__init__.py
+++ b/synapse/handlers/sliding_sync/__init__.py
@@ -1057,22 +1057,42 @@ class SlidingSyncHandler:
)
)
- # Figure out the last bump event in the room
- #
- # By default, just choose the membership event position for any non-join membership
- bump_stamp = room_membership_for_user_at_to_token.event_pos.stream
+ # Figure out the last bump event in the room. If the bump stamp hasn't
+ # changed we omit it from the response.
+ bump_stamp = None
+
+ always_return_bump_stamp = (
+ # We use the membership event position for any non-join
+ room_membership_for_user_at_to_token.membership != Membership.JOIN
+ # We didn't fetch any timeline events but we should still check for
+ # a bump_stamp that might be somewhere
+ or limited is None
+ # There might be a bump event somewhere before the timeline events
+ # that we fetched, that we didn't previously send down
+ or limited is True
+ # Always give the client some frame of reference if this is the
+ # first time they are seeing the room down the connection
+ or initial
+ )
+
# If we're joined to the room, we need to find the last bump event before the
# `to_token`
if room_membership_for_user_at_to_token.membership == Membership.JOIN:
- # Try and get a bump stamp, if not we just fall back to the
- # membership token.
+ # Try and get a bump stamp
new_bump_stamp = await self._get_bump_stamp(
- room_id, to_token, timeline_events
+ room_id,
+ to_token,
+ timeline_events,
+ check_outside_timeline=always_return_bump_stamp,
)
if new_bump_stamp is not None:
bump_stamp = new_bump_stamp
- if bump_stamp < 0:
+ if bump_stamp is None and always_return_bump_stamp:
+ # By default, just choose the membership event position for any non-join membership
+ bump_stamp = room_membership_for_user_at_to_token.event_pos.stream
+
+ if bump_stamp is not None and bump_stamp < 0:
# We never want to send down negative stream orderings, as you can't
# sensibly compare positive and negative stream orderings (they have
# different meanings).
@@ -1165,14 +1185,23 @@ class SlidingSyncHandler:
@trace
async def _get_bump_stamp(
- self, room_id: str, to_token: StreamToken, timeline: List[EventBase]
+ self,
+ room_id: str,
+ to_token: StreamToken,
+ timeline: List[EventBase],
+ check_outside_timeline: bool,
) -> Optional[int]:
- """Get a bump stamp for the room, if we have a bump event
+ """Get a bump stamp for the room, if we have a bump event and it has
+ changed.
Args:
room_id
to_token: The upper bound of token to return
timeline: The list of events we have fetched.
+ limited: If the timeline was limited.
+ check_outside_timeline: Whether we need to check for bump stamp for
+ events before the timeline if we didn't find a bump stamp in
+ the timeline events.
"""
# First check the timeline events we're returning to see if one of
@@ -1192,6 +1221,11 @@ class SlidingSyncHandler:
if new_bump_stamp > 0:
return new_bump_stamp
+ if not check_outside_timeline:
+ # If we are not a limited sync, then we know the bump stamp can't
+ # have changed.
+ return None
+
# We can quickly query for the latest bump event in the room using the
# sliding sync tables.
latest_room_bump_stamp = await self.store.get_latest_bump_stamp_for_room(
diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py
index 364cf40153..122708e933 100644
--- a/synapse/rest/client/sync.py
+++ b/synapse/rest/client/sync.py
@@ -1010,11 +1010,13 @@ class SlidingSyncRestServlet(RestServlet):
serialized_rooms: Dict[str, JsonDict] = {}
for room_id, room_result in rooms.items():
serialized_rooms[room_id] = {
- "bump_stamp": room_result.bump_stamp,
"notification_count": room_result.notification_count,
"highlight_count": room_result.highlight_count,
}
+ if room_result.bump_stamp is not None:
+ serialized_rooms[room_id]["bump_stamp"] = room_result.bump_stamp
+
if room_result.joined_count is not None:
serialized_rooms[room_id]["joined_count"] = room_result.joined_count
diff --git a/synapse/types/handlers/sliding_sync.py b/synapse/types/handlers/sliding_sync.py
index 5dd2c9d411..aae60fddea 100644
--- a/synapse/types/handlers/sliding_sync.py
+++ b/synapse/types/handlers/sliding_sync.py
@@ -158,6 +158,7 @@ class SlidingSyncResult:
name changes to mark the room as unread and bump it to the top. For
encrypted rooms, we just have to consider any activity as a bump because we
can't see the content and the client has to figure it out for themselves.
+ This may not be included if there hasn't been a change.
joined_count: The number of users with membership of join, including the client's
own user ID. (same as sync `v2 m.joined_member_count`)
invited_count: The number of users with membership of invite. (same as sync v2
@@ -193,7 +194,7 @@ class SlidingSyncResult:
limited: Optional[bool]
# Only optional because it won't be included for invite/knock rooms with `stripped_state`
num_live: Optional[int]
- bump_stamp: int
+ bump_stamp: Optional[int]
joined_count: Optional[int]
invited_count: Optional[int]
notification_count: int
diff --git a/tests/rest/client/sliding_sync/test_rooms_meta.py b/tests/rest/client/sliding_sync/test_rooms_meta.py
index c619dd83fb..0a8b2c02c2 100644
--- a/tests/rest/client/sliding_sync/test_rooms_meta.py
+++ b/tests/rest/client/sliding_sync/test_rooms_meta.py
@@ -1096,6 +1096,92 @@ class SlidingSyncRoomsMetaTestCase(SlidingSyncBase):
self.assertGreater(response_body["rooms"][room_id]["bump_stamp"], 0)
+ def test_rooms_bump_stamp_no_change_incremental(self) -> None:
+ """Test that the bump stamp is omitted if there has been no change"""
+
+ user1_id = self.register_user("user1", "pass")
+ user1_tok = self.login(user1_id, "pass")
+
+ room_id1 = self.helper.create_room_as(
+ user1_id,
+ tok=user1_tok,
+ )
+
+ # Make the Sliding Sync request
+ sync_body = {
+ "lists": {
+ "foo-list": {
+ "ranges": [[0, 1]],
+ "required_state": [],
+ "timeline_limit": 100,
+ }
+ }
+ }
+ response_body, from_token = self.do_sync(sync_body, tok=user1_tok)
+
+ # Initial sync so we expect to see a bump stamp
+ self.assertIn("bump_stamp", response_body["rooms"][room_id1])
+
+ # Send an event that is not in the bump events list
+ self.helper.send_event(
+ room_id1, type="org.matrix.test", content={}, tok=user1_tok
+ )
+
+ response_body, from_token = self.do_sync(
+ sync_body, since=from_token, tok=user1_tok
+ )
+
+ # There hasn't been a change to the bump stamps, so we ignore it
+ self.assertNotIn("bump_stamp", response_body["rooms"][room_id1])
+
+ def test_rooms_bump_stamp_change_incremental(self) -> None:
+ """Test that the bump stamp is included if there has been a change, even
+ if its not in the timeline"""
+
+ user1_id = self.register_user("user1", "pass")
+ user1_tok = self.login(user1_id, "pass")
+
+ room_id1 = self.helper.create_room_as(
+ user1_id,
+ tok=user1_tok,
+ )
+
+ # Make the Sliding Sync request
+ sync_body = {
+ "lists": {
+ "foo-list": {
+ "ranges": [[0, 1]],
+ "required_state": [],
+ "timeline_limit": 2,
+ }
+ }
+ }
+ response_body, from_token = self.do_sync(sync_body, tok=user1_tok)
+
+ # Initial sync so we expect to see a bump stamp
+ self.assertIn("bump_stamp", response_body["rooms"][room_id1])
+ first_bump_stamp = response_body["rooms"][room_id1]["bump_stamp"]
+
+ # Send a bump event at the start.
+ self.helper.send(room_id1, "test", tok=user1_tok)
+
+ # Send events that are not in the bump events list to fill the timeline
+ for _ in range(5):
+ self.helper.send_event(
+ room_id1, type="org.matrix.test", content={}, tok=user1_tok
+ )
+
+ response_body, from_token = self.do_sync(
+ sync_body, since=from_token, tok=user1_tok
+ )
+
+ # There was a bump event in the timeline gap, so we should see the bump
+ # stamp be updated.
+ self.assertIn("bump_stamp", response_body["rooms"][room_id1])
+ second_bump_stamp = response_body["rooms"][room_id1]["bump_stamp"]
+
+ self.assertGreater(second_bump_stamp, first_bump_stamp)
+
def test_rooms_bump_stamp_invites(self) -> None:
"""
Test that `bump_stamp` is present and points to the membership event,
|
