From dd1d796da17f97eb605b0204a007dbdb8e64b3c7 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 2 Mar 2023 12:39:07 -0800 Subject: [PATCH 1/8] don't try and fetch auth events if we already have them (they're not in db anyway) --- synapse/event_auth.py | 22 ++++++++++++++++------ synapse/handlers/event_auth.py | 18 ++++++++++++++++-- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 4d6d1b8ebd58..8c7d7d9aea50 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -168,13 +168,23 @@ async def check_state_independent_auth_rules( return # 2. Reject if event has auth_events that: ... - auth_events = await store.get_events( - event.auth_event_ids(), - redact_behaviour=EventRedactBehaviour.as_is, - allow_rejected=True, - ) if batched_auth_events: - auth_events.update(batched_auth_events) + auth_event_ids = event.auth_event_ids() + auth_events = dict(batched_auth_events) + if set(auth_event_ids) - batched_auth_events.keys(): + auth_events.update( + await store.get_events( + set(auth_event_ids) - batched_auth_events.keys(), + redact_behaviour=EventRedactBehaviour.as_is, + allow_rejected=True, + ) + ) + else: + auth_events = await store.get_events( + event.auth_event_ids(), + redact_behaviour=EventRedactBehaviour.as_is, + allow_rejected=True, + ) room_id = event.room_id auth_dict: MutableStateMap[str] = {} diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index c508861b6a7b..d7aa8bb2fcd7 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -63,9 +63,23 @@ async def check_auth_rules_from_context( self._store, event, batched_auth_events ) auth_event_ids = event.auth_event_ids() - auth_events_by_id = await self._store.get_events(auth_event_ids) + logger.info("auth events %s", auth_event_ids) + if batched_auth_events: - auth_events_by_id.update(batched_auth_events) + logger.info("Batched auth events %s", list(batched_auth_events.keys())) + auth_events_by_id = dict(batched_auth_events) + if set(auth_event_ids) - set(batched_auth_events): + logger.info( + "fetching %s", set(auth_event_ids) - set(batched_auth_events) + ) + auth_events_by_id.update( + await self._store.get_events( + set(auth_event_ids) - set(batched_auth_events) + ) + ) + else: + auth_events_by_id = await self._store.get_events(auth_event_ids) + check_state_dependent_auth_rules(event, auth_events_by_id.values()) def compute_auth_events( From 05e7bdb9191a9d7a777d0db8c838fe14cc1e1f47 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 2 Mar 2023 12:40:07 -0800 Subject: [PATCH 2/8] add a state_map_before_event field to EventContext --- synapse/events/snapshot.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py index a91a5d1e3cbe..c5d98e76145e 100644 --- a/synapse/events/snapshot.py +++ b/synapse/events/snapshot.py @@ -135,6 +135,8 @@ class EventContext(UnpersistedEventContextBase): delta_ids: Optional[StateMap[str]] = None app_service: Optional[ApplicationService] = None + _state_map_before_event: Optional[StateMap[str]] = None + partial_state: bool = False @staticmethod @@ -293,6 +295,11 @@ async def get_prev_state_ids( Maps a (type, state_key) to the event ID of the state event matching this tuple. """ + if self._state_map_before_event is not None: + if state_filter is not None: + return state_filter.filter_state(self._state_map_before_event) + return self._state_map_before_event + assert self.state_group_before_event is not None return await self._storage.state.get_state_ids_for_group( self.state_group_before_event, state_filter From 423f13cdd4282c3d37dcb66981b3438727716f09 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 2 Mar 2023 12:40:19 -0800 Subject: [PATCH 3/8] copy dict rather than mutating it --- synapse/handlers/room.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index b1784638f4ba..c018d24e2efb 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1123,7 +1123,7 @@ async def create_event( event_dict, prev_event_ids=prev_event, depth=depth, - state_map=state_map, + state_map=dict(state_map), for_batch=for_batch, ) From 81df09b3f9dc9606276f1a8c3275955d7bc38060 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 2 Mar 2023 12:44:43 -0800 Subject: [PATCH 4/8] newsfragement --- changelog.d/15195.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15195.misc diff --git a/changelog.d/15195.misc b/changelog.d/15195.misc new file mode 100644 index 000000000000..b187c3258260 --- /dev/null +++ b/changelog.d/15195.misc @@ -0,0 +1 @@ +Misc speedups to creating and authing events. \ No newline at end of file From db30a9f32fad2fc07d98d19fba4fe5a5fbd44240 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 2 Mar 2023 16:42:52 -0800 Subject: [PATCH 5/8] requested cahnges --- synapse/event_auth.py | 6 +++--- synapse/handlers/event_auth.py | 12 +++--------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 8c7d7d9aea50..1d03f001070b 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -169,12 +169,12 @@ async def check_state_independent_auth_rules( # 2. Reject if event has auth_events that: ... if batched_auth_events: - auth_event_ids = event.auth_event_ids() auth_events = dict(batched_auth_events) - if set(auth_event_ids) - batched_auth_events.keys(): + needed_auth_event_ids = set(event.auth_event_ids()) - batched_auth_events.keys() + if needed_auth_event_ids: auth_events.update( await store.get_events( - set(auth_event_ids) - batched_auth_events.keys(), + needed_auth_event_ids, redact_behaviour=EventRedactBehaviour.as_is, allow_rejected=True, ) diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index d7aa8bb2fcd7..93cd4617bd9f 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -63,19 +63,13 @@ async def check_auth_rules_from_context( self._store, event, batched_auth_events ) auth_event_ids = event.auth_event_ids() - logger.info("auth events %s", auth_event_ids) if batched_auth_events: - logger.info("Batched auth events %s", list(batched_auth_events.keys())) auth_events_by_id = dict(batched_auth_events) - if set(auth_event_ids) - set(batched_auth_events): - logger.info( - "fetching %s", set(auth_event_ids) - set(batched_auth_events) - ) + needed_auth_event_ids = set(auth_event_ids) - set(batched_auth_events) + if needed_auth_event_ids: auth_events_by_id.update( - await self._store.get_events( - set(auth_event_ids) - set(batched_auth_events) - ) + await self._store.get_events(needed_auth_event_ids) ) else: auth_events_by_id = await self._store.get_events(auth_event_ids) From cfaf33f72923a99e7bfc39ffe82fc0f95bed4ae3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Mon, 6 Mar 2023 12:09:28 -0800 Subject: [PATCH 6/8] clarifying comments --- synapse/event_auth.py | 1 + synapse/handlers/event_auth.py | 1 + synapse/handlers/room.py | 2 ++ 3 files changed, 4 insertions(+) diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 1d03f001070b..af55874b5c47 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -169,6 +169,7 @@ async def check_state_independent_auth_rules( # 2. Reject if event has auth_events that: ... if batched_auth_events: + # Copy the batched auth events to avoid mutating them. auth_events = dict(batched_auth_events) needed_auth_event_ids = set(event.auth_event_ids()) - batched_auth_events.keys() if needed_auth_event_ids: diff --git a/synapse/handlers/event_auth.py b/synapse/handlers/event_auth.py index 93cd4617bd9f..0db0bd7304ce 100644 --- a/synapse/handlers/event_auth.py +++ b/synapse/handlers/event_auth.py @@ -65,6 +65,7 @@ async def check_auth_rules_from_context( auth_event_ids = event.auth_event_ids() if batched_auth_events: + # Copy the batched auth events to avoid mutating them. auth_events_by_id = dict(batched_auth_events) needed_auth_event_ids = set(auth_event_ids) - set(batched_auth_events) if needed_auth_event_ids: diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index c018d24e2efb..32451670f326 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1123,6 +1123,8 @@ async def create_event( event_dict, prev_event_ids=prev_event, depth=depth, + # Take a copy to ensure each event gets a unique copy of + # state_map since it is modified below. state_map=dict(state_map), for_batch=for_batch, ) From 5fe44f0ebf19ded6f9c0b126863ef4ae6c950196 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Mar 2023 09:27:19 -0800 Subject: [PATCH 7/8] requested changes --- synapse/events/snapshot.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py index c5d98e76145e..c04ad08cbb61 100644 --- a/synapse/events/snapshot.py +++ b/synapse/events/snapshot.py @@ -135,8 +135,6 @@ class EventContext(UnpersistedEventContextBase): delta_ids: Optional[StateMap[str]] = None app_service: Optional[ApplicationService] = None - _state_map_before_event: Optional[StateMap[str]] = None - partial_state: bool = False @staticmethod @@ -295,10 +293,6 @@ async def get_prev_state_ids( Maps a (type, state_key) to the event ID of the state event matching this tuple. """ - if self._state_map_before_event is not None: - if state_filter is not None: - return state_filter.filter_state(self._state_map_before_event) - return self._state_map_before_event assert self.state_group_before_event is not None return await self._storage.state.get_state_ids_for_group( From d3d9d73c877e3d5a3d0ceafa62f55cc519a5798c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 7 Mar 2023 09:28:33 -0800 Subject: [PATCH 8/8] improve changelog message --- changelog.d/15195.misc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/15195.misc b/changelog.d/15195.misc index b187c3258260..d8beea917d8b 100644 --- a/changelog.d/15195.misc +++ b/changelog.d/15195.misc @@ -1 +1 @@ -Misc speedups to creating and authing events. \ No newline at end of file +Improve performance of creating and authenticating events. \ No newline at end of file