From 7e6e1277b82f85229f6ce5a75e4c276a3eab777f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 23 Jan 2023 11:43:54 -0500 Subject: [PATCH 01/16] Hoist whether to calculate device lists to caller. --- synapse/handlers/sync.py | 164 +++++++++++++++++++-------------------- 1 file changed, 81 insertions(+), 83 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index ee117645673f..2c4457268fb2 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1463,13 +1463,17 @@ async def generate_sync_result( logger.debug("Fetching to-device data") await self._generate_sync_entry_for_to_device(sync_result_builder) - device_lists = await self._generate_sync_entry_for_device_list( - sync_result_builder, - newly_joined_rooms=newly_joined_rooms, - newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, - newly_left_rooms=newly_left_rooms, - newly_left_users=newly_left_users, - ) + include_device_list_updates = since_token and since_token.device_list_key + if include_device_list_updates: + device_lists = await self._generate_sync_entry_for_device_list( + sync_result_builder, + newly_joined_rooms=newly_joined_rooms, + newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, + newly_left_rooms=newly_left_rooms, + newly_left_users=newly_left_users, + ) + else: + device_lists = DeviceListUpdates() logger.debug("Fetching OTK data") device_id = sync_config.device_id @@ -1539,6 +1543,7 @@ async def _generate_sync_entry_for_device_list( user_id = sync_result_builder.sync_config.user.to_string() since_token = sync_result_builder.since_token + assert since_token is not None # Take a copy since these fields will be mutated later. newly_joined_or_invited_or_knocked_users = set( @@ -1546,92 +1551,85 @@ async def _generate_sync_entry_for_device_list( ) newly_left_users = set(newly_left_users) - if since_token and since_token.device_list_key: - # We want to figure out what user IDs the client should refetch - # device keys for, and which users we aren't going to track changes - # for anymore. - # - # For the first step we check: - # a. if any users we share a room with have updated their devices, - # and - # b. we also check if we've joined any new rooms, or if a user has - # joined a room we're in. - # - # For the second step we just find any users we no longer share a - # room with by looking at all users that have left a room plus users - # that were in a room we've left. - - users_that_have_changed = set() + # We want to figure out what user IDs the client should refetch + # device keys for, and which users we aren't going to track changes + # for anymore. + # + # For the first step we check: + # a. if any users we share a room with have updated their devices, + # and + # b. we also check if we've joined any new rooms, or if a user has + # joined a room we're in. + # + # For the second step we just find any users we no longer share a + # room with by looking at all users that have left a room plus users + # that were in a room we've left. - joined_rooms = sync_result_builder.joined_room_ids + users_that_have_changed = set() - # Step 1a, check for changes in devices of users we share a room - # with - # - # We do this in two different ways depending on what we have cached. - # If we already have a list of all the user that have changed since - # the last sync then it's likely more efficient to compare the rooms - # they're in with the rooms the syncing user is in. - # - # If we don't have that info cached then we get all the users that - # share a room with our user and check if those users have changed. - cache_result = self.store.get_cached_device_list_changes( - since_token.device_list_key - ) - if cache_result.hit: - changed_users = cache_result.entities - - result = await self.store.get_rooms_for_users(changed_users) - - for changed_user_id, entries in result.items(): - # Check if the changed user shares any rooms with the user, - # or if the changed user is the syncing user (as we always - # want to include device list updates of their own devices). - if user_id == changed_user_id or any( - rid in joined_rooms for rid in entries - ): - users_that_have_changed.add(changed_user_id) - else: - users_that_have_changed = ( - await self._device_handler.get_device_changes_in_shared_rooms( - user_id, - sync_result_builder.joined_room_ids, - from_token=since_token, - ) - ) + joined_rooms = sync_result_builder.joined_room_ids - # Step 1b, check for newly joined rooms - for room_id in newly_joined_rooms: - joined_users = await self.store.get_users_in_room(room_id) - newly_joined_or_invited_or_knocked_users.update(joined_users) + # Step 1a, check for changes in devices of users we share a room + # with + # + # We do this in two different ways depending on what we have cached. + # If we already have a list of all the user that have changed since + # the last sync then it's likely more efficient to compare the rooms + # they're in with the rooms the syncing user is in. + # + # If we don't have that info cached then we get all the users that + # share a room with our user and check if those users have changed. + cache_result = self.store.get_cached_device_list_changes( + since_token.device_list_key + ) + if cache_result.hit: + changed_users = cache_result.entities - # TODO: Check that these users are actually new, i.e. either they - # weren't in the previous sync *or* they left and rejoined. - users_that_have_changed.update(newly_joined_or_invited_or_knocked_users) + result = await self.store.get_rooms_for_users(changed_users) - user_signatures_changed = ( - await self.store.get_users_whose_signatures_changed( - user_id, since_token.device_list_key + for changed_user_id, entries in result.items(): + # Check if the changed user shares any rooms with the user, + # or if the changed user is the syncing user (as we always + # want to include device list updates of their own devices). + if user_id == changed_user_id or any( + rid in joined_rooms for rid in entries + ): + users_that_have_changed.add(changed_user_id) + else: + users_that_have_changed = ( + await self._device_handler.get_device_changes_in_shared_rooms( + user_id, + sync_result_builder.joined_room_ids, + from_token=since_token, ) ) - users_that_have_changed.update(user_signatures_changed) - # Now find users that we no longer track - for room_id in newly_left_rooms: - left_users = await self.store.get_users_in_room(room_id) - newly_left_users.update(left_users) + # Step 1b, check for newly joined rooms + for room_id in newly_joined_rooms: + joined_users = await self.store.get_users_in_room(room_id) + newly_joined_or_invited_or_knocked_users.update(joined_users) - # Remove any users that we still share a room with. - left_users_rooms = await self.store.get_rooms_for_users(newly_left_users) - for user_id, entries in left_users_rooms.items(): - if any(rid in joined_rooms for rid in entries): - newly_left_users.discard(user_id) + # TODO: Check that these users are actually new, i.e. either they + # weren't in the previous sync *or* they left and rejoined. + users_that_have_changed.update(newly_joined_or_invited_or_knocked_users) - return DeviceListUpdates( - changed=users_that_have_changed, left=newly_left_users - ) - else: - return DeviceListUpdates() + user_signatures_changed = await self.store.get_users_whose_signatures_changed( + user_id, since_token.device_list_key + ) + users_that_have_changed.update(user_signatures_changed) + + # Now find users that we no longer track + for room_id in newly_left_rooms: + left_users = await self.store.get_users_in_room(room_id) + newly_left_users.update(left_users) + + # Remove any users that we still share a room with. + left_users_rooms = await self.store.get_rooms_for_users(newly_left_users) + for user_id, entries in left_users_rooms.items(): + if any(rid in joined_rooms for rid in entries): + newly_left_users.discard(user_id) + + return DeviceListUpdates(changed=users_that_have_changed, left=newly_left_users) @trace async def _generate_sync_entry_for_to_device( From dda8a2659940e1aaa394c66276db6e72a1448091 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 23 Jan 2023 13:22:00 -0500 Subject: [PATCH 02/16] Clarify some comments. --- synapse/handlers/sync.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 2c4457268fb2..ff4127af2060 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1888,6 +1888,7 @@ async def _generate_sync_entry_for_rooms( # joined or archived). async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: logger.debug("Generating room entry for %s", room_entry.room_id) + # Note that this mutates sync_result_builder.{joined,archived}. await self._generate_room_entry( sync_result_builder, room_entry, @@ -1905,7 +1906,7 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: sync_result_builder.knocked.extend(knocked) # 5. Work out which users have joined or left rooms we're in. We use this - # to build the device_list part of the sync response in + # to build the presence and device_list parts of the sync response in # `_generate_sync_entry_for_device_list`. ( newly_joined_or_invited_or_knocked_users, From f7e3a3e83d1bad10aef32cb48f1ffb4de9653442 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 23 Jan 2023 13:26:54 -0500 Subject: [PATCH 03/16] Skip potentially unneeded calculation. --- synapse/handlers/sync.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index ff4127af2060..87fd24a551b6 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1440,18 +1440,22 @@ async def generate_sync_result( logger.debug("Fetching room data") + block_all_presence_data = ( + since_token is None and sync_config.filter_collection.blocks_all_presence() + ) + include_device_list_updates = bool(since_token and since_token.device_list_key) ( newly_joined_rooms, newly_joined_or_invited_or_knocked_users, newly_left_rooms, newly_left_users, ) = await self._generate_sync_entry_for_rooms( - sync_result_builder, account_data_by_room + sync_result_builder, + account_data_by_room, + block_all_presence_data, + include_device_list_updates, ) - block_all_presence_data = ( - since_token is None and sync_config.filter_collection.blocks_all_presence() - ) if self.hs_config.server.use_presence and not block_all_presence_data: logger.debug("Fetching presence data") await self._generate_sync_entry_for_presence( @@ -1463,7 +1467,6 @@ async def generate_sync_result( logger.debug("Fetching to-device data") await self._generate_sync_entry_for_to_device(sync_result_builder) - include_device_list_updates = since_token and since_token.device_list_key if include_device_list_updates: device_lists = await self._generate_sync_entry_for_device_list( sync_result_builder, @@ -1804,6 +1807,8 @@ async def _generate_sync_entry_for_rooms( self, sync_result_builder: "SyncResultBuilder", account_data_by_room: Dict[str, Dict[str, JsonDict]], + block_all_presence_data: bool, + include_device_list_updates: bool, ) -> Tuple[AbstractSet[str], AbstractSet[str], AbstractSet[str], AbstractSet[str]]: """Generates the rooms portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1815,6 +1820,8 @@ async def _generate_sync_entry_for_rooms( Args: sync_result_builder account_data_by_room: Dictionary of per room account data + block_all_presence_data: True if presence data will not be returned. + include_device_list_updates: True if device list updates will be returned. Returns: Returns a 4-tuple describing rooms the user has joined or left, and users who've @@ -1908,10 +1915,14 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: # 5. Work out which users have joined or left rooms we're in. We use this # to build the presence and device_list parts of the sync response in # `_generate_sync_entry_for_device_list`. - ( - newly_joined_or_invited_or_knocked_users, - newly_left_users, - ) = sync_result_builder.calculate_user_changes() + if not block_all_presence_data or include_device_list_updates: + ( + newly_joined_or_invited_or_knocked_users, + newly_left_users, + ) = sync_result_builder.calculate_user_changes() + else: + newly_joined_or_invited_or_knocked_users = set() + newly_left_users = set() return ( set(newly_joined_rooms), From 99c5ceaeb3526f08c6a1c072d28d17a6f4a1ef17 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 23 Jan 2023 14:03:53 -0500 Subject: [PATCH 04/16] Skip some processing if we do not care about any rooms. --- synapse/api/filtering.py | 3 +++ synapse/handlers/sync.py | 19 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/synapse/api/filtering.py b/synapse/api/filtering.py index 4cf8f0cc8ef9..2b5af264b43d 100644 --- a/synapse/api/filtering.py +++ b/synapse/api/filtering.py @@ -283,6 +283,9 @@ async def filter_room_account_data( await self._room_filter.filter(events) ) + def blocks_all_rooms(self) -> bool: + return self._room_filter.filters_all_rooms() + def blocks_all_presence(self) -> bool: return ( self._presence_filter.filters_all_types() diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 87fd24a551b6..679a61d9258c 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1709,6 +1709,7 @@ async def _generate_sync_entry_for_account_data( since_token = sync_result_builder.since_token if since_token and not sync_result_builder.full_state: + # TODO Do not fetch room account data if it will be unused. ( global_account_data, account_data_by_room, @@ -1725,6 +1726,7 @@ async def _generate_sync_entry_for_account_data( sync_config.user ) else: + # TODO Do not fetch room account data if it will be unused. ( global_account_data, account_data_by_room, @@ -1837,10 +1839,25 @@ async def _generate_sync_entry_for_rooms( since_token = sync_result_builder.since_token + # If all rooms are blocked, we can skip bits of processing. + block_all_rooms = ( + sync_result_builder.sync_config.filter_collection.blocks_all_rooms() + ) + + # 0. If there are no rooms to return *and* we don't care about presence + # or device list updates, there's nothing to do. + if ( + block_all_rooms + and block_all_presence_data + and not include_device_list_updates + ): + return set(), set(), set(), set() + # 1. Start by fetching all ephemeral events in rooms we've joined (if required). user_id = sync_result_builder.sync_config.user.to_string() block_all_room_ephemeral = ( - since_token is None + block_all_rooms + or since_token is None and sync_result_builder.sync_config.filter_collection.blocks_all_room_ephemeral() ) From a0ed8deb089fd8dbcc369c86bb9e8f40061e4baa Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 Jan 2023 10:08:48 -0500 Subject: [PATCH 05/16] Newsfragment --- changelog.d/14908.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14908.misc diff --git a/changelog.d/14908.misc b/changelog.d/14908.misc new file mode 100644 index 000000000000..365762360285 --- /dev/null +++ b/changelog.d/14908.misc @@ -0,0 +1 @@ +Improve performance of `/sync` in a few situations. From a4092fbe8934a1f25d2ecbc5b3bde0c9adec997c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 07:37:59 -0500 Subject: [PATCH 06/16] Clarify comment. --- synapse/handlers/sync.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 0adc2073f4ce..ec253e41a054 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1941,7 +1941,8 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: # 5. Work out which users have joined or left rooms we're in. We use this # to build the presence and device_list parts of the sync response in - # `_generate_sync_entry_for_device_list`. + # `_generate_sync_entry_for_presence` and + # `_generate_sync_entry_for_device_list` respectively.. if not block_all_presence_data or include_device_list_updates: ( newly_joined_or_invited_or_knocked_users, From bf44840343c75d8630b3e47f7d55f7a7ef5c647e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:19:13 -0500 Subject: [PATCH 07/16] Add comments. --- synapse/handlers/sync.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 5235e294608d..dcd31d6d2ed5 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1459,6 +1459,9 @@ async def generate_sync_result( sync_result_builder, account_data_by_room ) + # Presence data is included if the server has it enabled and: + # - There is a sync token, or + # - Is not filtered out. block_all_presence_data = ( since_token is None and sync_config.filter_collection.blocks_all_presence() ) From 1a8f515271b50c8fd1a6f575ffefe1bcf340f1ea Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:19:50 -0500 Subject: [PATCH 08/16] Swap logic to positive. --- synapse/handlers/sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index dcd31d6d2ed5..8491e24f3db8 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1462,10 +1462,10 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and: # - There is a sync token, or # - Is not filtered out. - block_all_presence_data = ( + include_presence_data = not ( since_token is None and sync_config.filter_collection.blocks_all_presence() ) - if self.hs_config.server.use_presence and not block_all_presence_data: + if self.hs_config.server.use_presence and include_presence_data: logger.debug("Fetching presence data") await self._generate_sync_entry_for_presence( sync_result_builder, From d303310e7f2ba1544009cc79f070f6cc14f88ea4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:20:25 -0500 Subject: [PATCH 09/16] Move config flag into boolean. --- synapse/handlers/sync.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 8491e24f3db8..131514316bdd 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1462,10 +1462,10 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and: # - There is a sync token, or # - Is not filtered out. - include_presence_data = not ( + include_presence_data = self.hs_config.server.use_presence and not ( since_token is None and sync_config.filter_collection.blocks_all_presence() ) - if self.hs_config.server.use_presence and include_presence_data: + if include_presence_data: logger.debug("Fetching presence data") await self._generate_sync_entry_for_presence( sync_result_builder, From a9ced4d5a62748870e1d1e869d3258aedb816433 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:21:04 -0500 Subject: [PATCH 10/16] Apply De Morgan's Theorem. --- synapse/handlers/sync.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 131514316bdd..86f8bf876869 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1462,8 +1462,9 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and: # - There is a sync token, or # - Is not filtered out. - include_presence_data = self.hs_config.server.use_presence and not ( - since_token is None and sync_config.filter_collection.blocks_all_presence() + include_presence_data = self.hs_config.server.use_presence and ( + since_token is not None + or not sync_config.filter_collection.blocks_all_presence() ) if include_presence_data: logger.debug("Fetching presence data") From 63fa12c590b702f1149705890a157ea392b0a5b3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:25:50 -0500 Subject: [PATCH 11/16] Newsfragment --- changelog.d/14970.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14970.misc diff --git a/changelog.d/14970.misc b/changelog.d/14970.misc new file mode 100644 index 000000000000..365762360285 --- /dev/null +++ b/changelog.d/14970.misc @@ -0,0 +1 @@ +Improve performance of `/sync` in a few situations. From 9b970d6c5bf4fcc9509d93bc2f5e3ad22c0f09be Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 08:56:41 -0500 Subject: [PATCH 12/16] Clarify comment. Co-authored-by: David Robertson --- synapse/handlers/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 86f8bf876869..cc3c2de513f1 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1461,7 +1461,7 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and: # - There is a sync token, or - # - Is not filtered out. + # - Presence is not filtered out. include_presence_data = self.hs_config.server.use_presence and ( since_token is not None or not sync_config.filter_collection.blocks_all_presence() From b02780bb1367f5cb569dfb970f2a099da8b7aaec Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 09:17:30 -0500 Subject: [PATCH 13/16] More simplification. --- synapse/handlers/sync.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index cc3c2de513f1..0cb8d5ef4b66 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1459,12 +1459,10 @@ async def generate_sync_result( sync_result_builder, account_data_by_room ) - # Presence data is included if the server has it enabled and: - # - There is a sync token, or - # - Presence is not filtered out. - include_presence_data = self.hs_config.server.use_presence and ( - since_token is not None - or not sync_config.filter_collection.blocks_all_presence() + # Presence data is included if the server has it enabled and not filtered out. + include_presence_data = ( + self.hs_config.server.use_presence + and not sync_config.filter_collection.blocks_all_presence() ) if include_presence_data: logger.debug("Fetching presence data") @@ -1845,15 +1843,12 @@ async def _generate_sync_entry_for_rooms( """ since_token = sync_result_builder.since_token - - # 1. Start by fetching all ephemeral events in rooms we've joined (if required). user_id = sync_result_builder.sync_config.user.to_string() - block_all_room_ephemeral = ( - since_token is None - and sync_result_builder.sync_config.filter_collection.blocks_all_room_ephemeral() - ) - if block_all_room_ephemeral: + # 1. Start by fetching all ephemeral events in rooms we've joined (if required). + if ( + sync_result_builder.sync_config.filter_collection.blocks_all_room_ephemeral() + ): ephemeral_by_room: Dict[str, List[JsonDict]] = {} else: now_token, ephemeral_by_room = await self.ephemeral_by_room( From 101e09b827a1200f9cfc58a6c3fb12b66903f19a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 09:59:38 -0500 Subject: [PATCH 14/16] Hoist the calculate_user_changes call up a level. --- synapse/handlers/sync.py | 78 +++++++++++++++------------------------- 1 file changed, 28 insertions(+), 50 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 63dd18c1dcd0..866595cd99b9 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1450,23 +1450,36 @@ async def generate_sync_result( logger.debug("Fetching room data") + ( + newly_joined_rooms, + newly_left_rooms, + ) = await self._generate_sync_entry_for_rooms( + sync_result_builder, account_data_by_room + ) + # Presence data is included if the server has it enabled and not filtered out. include_presence_data = bool( self.hs_config.server.use_presence and not sync_config.filter_collection.blocks_all_presence() ) + # Device list updates are sent if a since token is provided. include_device_list_updates = bool(since_token and since_token.device_list_key) - ( - newly_joined_rooms, - newly_joined_or_invited_or_knocked_users, - newly_left_rooms, - newly_left_users, - ) = await self._generate_sync_entry_for_rooms( - sync_result_builder, - account_data_by_room, - include_presence_data, - include_device_list_updates, - ) + + # Work out which users have joined or left rooms we're in. We use this + # to build the presence and device_list parts of the sync response in + # `_generate_sync_entry_for_presence` and + # `_generate_sync_entry_for_device_list` respectively. + if include_presence_data or include_device_list_updates: + # This uses the sync_result_builder.joined which is set in + # `_generate_sync_entry_for_rooms`, if that didn't find any joined + # rooms for some reason it is a no-op. + ( + newly_joined_or_invited_or_knocked_users, + newly_left_users, + ) = sync_result_builder.calculate_user_changes() + else: + newly_joined_or_invited_or_knocked_users = set() + newly_left_users = set() if include_presence_data: logger.debug("Fetching presence data") @@ -1821,9 +1834,7 @@ async def _generate_sync_entry_for_rooms( self, sync_result_builder: "SyncResultBuilder", account_data_by_room: Dict[str, Dict[str, JsonDict]], - include_presence_data: bool, - include_device_list_updates: bool, - ) -> Tuple[AbstractSet[str], AbstractSet[str], AbstractSet[str], AbstractSet[str]]: + ) -> Tuple[AbstractSet[str], AbstractSet[str]]: """Generates the rooms portion of the sync response. Populates the `sync_result_builder` with the result. @@ -1834,19 +1845,13 @@ async def _generate_sync_entry_for_rooms( Args: sync_result_builder account_data_by_room: Dictionary of per room account data - include_presence_data: True if presence data will be returned. - include_device_list_updates: True if device list updates will be returned. Returns: - Returns a 4-tuple describing rooms the user has joined or left, and users who've - joined or left rooms any rooms the user is in. This gets used later in - `_generate_sync_entry_for_device_list`. + Returns a 2-tuple describing rooms the user has joined or left. Its entries are: - newly_joined_rooms - - newly_joined_or_invited_or_knocked_users - newly_left_rooms - - newly_left_users """ since_token = sync_result_builder.since_token @@ -1857,15 +1862,6 @@ async def _generate_sync_entry_for_rooms( sync_result_builder.sync_config.filter_collection.blocks_all_rooms() ) - # 0. If there are no rooms to return *and* we don't care about presence - # or device list updates, there's nothing to do. - if ( - block_all_rooms - and not include_presence_data - and not include_device_list_updates - ): - return set(), set(), set(), set() - # 1. Start by fetching all ephemeral events in rooms we've joined (if required). block_all_room_ephemeral = ( block_all_rooms @@ -1893,7 +1889,7 @@ async def _generate_sync_entry_for_rooms( ) if not tags_by_room: logger.debug("no-oping sync") - return set(), set(), set(), set() + return set(), set() # 3. Work out which rooms need reporting in the sync response. ignored_users = await self.store.ignored_users(user_id) @@ -1939,25 +1935,7 @@ async def handle_room_entries(room_entry: "RoomSyncResultBuilder") -> None: sync_result_builder.invited.extend(invited) sync_result_builder.knocked.extend(knocked) - # 5. Work out which users have joined or left rooms we're in. We use this - # to build the presence and device_list parts of the sync response in - # `_generate_sync_entry_for_presence` and - # `_generate_sync_entry_for_device_list` respectively.. - if include_presence_data or include_device_list_updates: - ( - newly_joined_or_invited_or_knocked_users, - newly_left_users, - ) = sync_result_builder.calculate_user_changes() - else: - newly_joined_or_invited_or_knocked_users = set() - newly_left_users = set() - - return ( - set(newly_joined_rooms), - newly_joined_or_invited_or_knocked_users, - set(newly_left_rooms), - newly_left_users, - ) + return set(newly_joined_rooms), set(newly_left_rooms) async def _have_rooms_changed( self, sync_result_builder: "SyncResultBuilder" From 01c6dd8339dd824782de37e253f490f9394a6b5c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 13:00:23 -0500 Subject: [PATCH 15/16] Move to-device calculation. --- synapse/handlers/sync.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 4147a1a3c639..dc6c1a4b5770 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1489,9 +1489,6 @@ async def generate_sync_result( newly_joined_or_invited_or_knocked_users, ) - logger.debug("Fetching to-device data") - await self._generate_sync_entry_for_to_device(sync_result_builder) - if include_device_list_updates: device_lists = await self._generate_sync_entry_for_device_list( sync_result_builder, @@ -1503,6 +1500,9 @@ async def generate_sync_result( else: device_lists = DeviceListUpdates() + logger.debug("Fetching to-device data") + await self._generate_sync_entry_for_to_device(sync_result_builder) + logger.debug("Fetching OTK data") device_id = sync_config.device_id one_time_keys_count: JsonDict = {} From e9b51679b05cf9a534b8d7feee20c78a4931cd17 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Feb 2023 13:01:11 -0500 Subject: [PATCH 16/16] Refactor to skip the calculation of room information if it is not needed. --- synapse/handlers/sync.py | 84 +++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index dc6c1a4b5770..3566537894e6 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1448,15 +1448,6 @@ async def generate_sync_result( sync_result_builder ) - logger.debug("Fetching room data") - - ( - newly_joined_rooms, - newly_left_rooms, - ) = await self._generate_sync_entry_for_rooms( - sync_result_builder, account_data_by_room - ) - # Presence data is included if the server has it enabled and not filtered out. include_presence_data = bool( self.hs_config.server.use_presence @@ -1465,40 +1456,55 @@ async def generate_sync_result( # Device list updates are sent if a since token is provided. include_device_list_updates = bool(since_token and since_token.device_list_key) - # Work out which users have joined or left rooms we're in. We use this - # to build the presence and device_list parts of the sync response in - # `_generate_sync_entry_for_presence` and - # `_generate_sync_entry_for_device_list` respectively. - if include_presence_data or include_device_list_updates: - # This uses the sync_result_builder.joined which is set in - # `_generate_sync_entry_for_rooms`, if that didn't find any joined - # rooms for some reason it is a no-op. - ( - newly_joined_or_invited_or_knocked_users, - newly_left_users, - ) = sync_result_builder.calculate_user_changes() - else: - newly_joined_or_invited_or_knocked_users = set() - newly_left_users = set() + # If we do not care about the rooms or things which depend on the room + # data (namely presence and device list updates), then we can skip + # this process completely. + device_lists = DeviceListUpdates() + if ( + not sync_result_builder.sync_config.filter_collection.blocks_all_rooms() + or include_presence_data + or include_device_list_updates + ): + logger.debug("Fetching room data") - if include_presence_data: - logger.debug("Fetching presence data") - await self._generate_sync_entry_for_presence( - sync_result_builder, + # Note that _generate_sync_entry_for_rooms sets sync_result_builder.joined, which + # is used in calculate_user_changes below. + ( newly_joined_rooms, - newly_joined_or_invited_or_knocked_users, + newly_left_rooms, + ) = await self._generate_sync_entry_for_rooms( + sync_result_builder, account_data_by_room ) - if include_device_list_updates: - device_lists = await self._generate_sync_entry_for_device_list( - sync_result_builder, - newly_joined_rooms=newly_joined_rooms, - newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, - newly_left_rooms=newly_left_rooms, - newly_left_users=newly_left_users, - ) - else: - device_lists = DeviceListUpdates() + # Work out which users have joined or left rooms we're in. We use this + # to build the presence and device_list parts of the sync response in + # `_generate_sync_entry_for_presence` and + # `_generate_sync_entry_for_device_list` respectively. + if include_presence_data or include_device_list_updates: + # This uses the sync_result_builder.joined which is set in + # `_generate_sync_entry_for_rooms`, if that didn't find any joined + # rooms for some reason it is a no-op. + ( + newly_joined_or_invited_or_knocked_users, + newly_left_users, + ) = sync_result_builder.calculate_user_changes() + + if include_presence_data: + logger.debug("Fetching presence data") + await self._generate_sync_entry_for_presence( + sync_result_builder, + newly_joined_rooms, + newly_joined_or_invited_or_knocked_users, + ) + + if include_device_list_updates: + device_lists = await self._generate_sync_entry_for_device_list( + sync_result_builder, + newly_joined_rooms=newly_joined_rooms, + newly_joined_or_invited_or_knocked_users=newly_joined_or_invited_or_knocked_users, + newly_left_rooms=newly_left_rooms, + newly_left_users=newly_left_users, + ) logger.debug("Fetching to-device data") await self._generate_sync_entry_for_to_device(sync_result_builder)