From 743af2e5ac0892270d7e0accbce293059c34d1a9 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Sat, 18 Mar 2023 20:16:31 +0100 Subject: [PATCH 1/7] Speedup tests by caching HomeServerConfig instances These two lines: ``` config_obj = HomeServerConfig() config_obj.parse_config_dict(config, "", "") ``` are called many times with the exact same value for `config`. As the test suite is CPU-bound and non-negligeably time is spent in `parse_config_dict`, this saves ~5% on the overall runtime of the Trial test suite (tested with both `-j2` and `-j12` on a 12t CPU). This is sadly rather limited, as the cache cannot be shared between processes (it contains at least jinja2.Template and RLock objects which aren't pickleable), and Trial tends to run close tests in different processes. --- changelog.d/15283.misc | 1 + tests/unittest.py | 69 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 changelog.d/15283.misc diff --git a/changelog.d/15283.misc b/changelog.d/15283.misc new file mode 100644 index 000000000000..99d753f8f051 --- /dev/null +++ b/changelog.d/15283.misc @@ -0,0 +1 @@ +Speedup tests by caching HomeServerConfig instances. diff --git a/tests/unittest.py b/tests/unittest.py index f9160faa1d0c..3cb96df8b21d 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -16,6 +16,7 @@ import gc import hashlib import hmac +import json import logging import secrets import time @@ -28,12 +29,14 @@ Generic, Iterable, List, + Literal, NoReturn, Optional, Tuple, Type, TypeVar, Union, + overload, ) from unittest.mock import Mock, patch @@ -53,6 +56,7 @@ from synapse import events from synapse.api.constants import EventTypes from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion +from synapse.config._base import Config, RootConfig from synapse.config.homeserver import HomeServerConfig from synapse.config.server import DEFAULT_ROOM_VERSION from synapse.crypto.event_signing import add_hashes_and_signatures @@ -124,6 +128,68 @@ def new(*args: P.args, **kwargs: P.kwargs) -> R: return _around +@overload +def deepcopy_config(config: RootConfig, root: Literal[True]) -> RootConfig: + ... + + +@overload +def deepcopy_config(config: Config, root: Literal[False]) -> Config: + ... + + +def deepcopy_config(config, root): + if root: + new_config = config.__class__(config.config_files) + else: + new_config = config.__class__(config.root) + + for attr_name in config.__dict__: + if attr_name.startswith("__") or attr_name == "root": + continue + attr = getattr(config, attr_name) + if isinstance(attr, Config): + new_attr = deepcopy_config(attr, root=False) + else: + new_attr = attr + + setattr(new_config, attr_name, new_attr) + + return new_config + + +_make_homeserver_config_obj_cache: Dict[str, Union[RootConfig, Config]] = {} + + +def make_homeserver_config_obj(config: Dict[str, Any]) -> RootConfig: + """Creates a :class:`HomeServerConfig` instance with the given configuration dict. + + This is equivalent to:: + + config_obj = HomeServerConfig() + config_obj.parse_config_dict(config, "", "") + + but it keeps a cache of `HomeServerConfig` instances and deepcopies them as needed, + to avoid validating the whole configuration every time. + """ + cache_key = json.dumps(config) + + if cache_key in _make_homeserver_config_obj_cache: + # Cache hit: reuse the existing instance + config_obj = _make_homeserver_config_obj_cache[cache_key] + else: + # Cache miss; create the actual instance + config_obj = HomeServerConfig() + config_obj.parse_config_dict(config, "", "") + + # Add to the cache + _make_homeserver_config_obj_cache[cache_key] = config_obj + + assert isinstance(config_obj, RootConfig) + + return deepcopy_config(config_obj, root=True) + + class TestCase(unittest.TestCase): """A subclass of twisted.trial's TestCase which looks for 'loglevel' attributes on both itself and its individual test methods, to override the @@ -518,8 +584,7 @@ def setup_test_homeserver(self, *args: Any, **kwargs: Any) -> HomeServer: config = kwargs["config"] # Parse the config from a config dict into a HomeServerConfig - config_obj = HomeServerConfig() - config_obj.parse_config_dict(config, "", "") + config_obj = make_homeserver_config_obj(config) kwargs["config"] = config_obj async def run_bg_updates() -> None: From 22865fa030f130fd325b1f13503dec43c1c2b315 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Sun, 19 Mar 2023 11:31:18 +0100 Subject: [PATCH 2/7] Help mypy figure the types out --- tests/unittest.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/unittest.py b/tests/unittest.py index 3cb96df8b21d..f87b236ef34c 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -36,6 +36,7 @@ Type, TypeVar, Union, + cast, overload, ) from unittest.mock import Mock, patch @@ -138,11 +139,15 @@ def deepcopy_config(config: Config, root: Literal[False]) -> Config: ... -def deepcopy_config(config, root): +def deepcopy_config(config: Union[Config, RootConfig], root: bool) -> Union[RootConfig, Config]: + new_config: Union[Config, RootConfig] + if root: - new_config = config.__class__(config.config_files) + typed_rootconfig = cast(RootConfig, config) + new_config = typed_rootconfig.__class__(typed_rootconfig.config_files) else: - new_config = config.__class__(config.root) + typed_config = cast(Config, config) + new_config = typed_config.__class__(typed_config.root) for attr_name in config.__dict__: if attr_name.startswith("__") or attr_name == "root": From a4027d4b0344fb79601b8b590c0bc33dcecd2acf Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Sun, 19 Mar 2023 11:33:24 +0100 Subject: [PATCH 3/7] Fix changelog id --- changelog.d/{15283.misc => 15284.misc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{15283.misc => 15284.misc} (100%) diff --git a/changelog.d/15283.misc b/changelog.d/15284.misc similarity index 100% rename from changelog.d/15283.misc rename to changelog.d/15284.misc From 64e9d5d53cd202b961fba7d1f03e043a9ee72b98 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Sun, 19 Mar 2023 13:14:03 +0100 Subject: [PATCH 4/7] Re-run linters --- tests/unittest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unittest.py b/tests/unittest.py index f87b236ef34c..ad3560eb5470 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -139,7 +139,9 @@ def deepcopy_config(config: Config, root: Literal[False]) -> Config: ... -def deepcopy_config(config: Union[Config, RootConfig], root: bool) -> Union[RootConfig, Config]: +def deepcopy_config( + config: Union[Config, RootConfig], root: bool +) -> Union[RootConfig, Config]: new_config: Union[Config, RootConfig] if root: From 02bcd46dc89e1bd1b1edef3b2ca23d4458e1a23a Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 28 Mar 2023 19:46:57 +0200 Subject: [PATCH 5/7] Fix py <3.7 support --- tests/unittest.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/unittest.py b/tests/unittest.py index ad3560eb5470..515d872d37c4 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -29,7 +29,6 @@ Generic, Iterable, List, - Literal, NoReturn, Optional, Tuple, @@ -44,7 +43,7 @@ import canonicaljson import signedjson.key import unpaddedbase64 -from typing_extensions import Concatenate, ParamSpec, Protocol +from typing_extensions import Concatenate, Literal, ParamSpec, Protocol from twisted.internet.defer import Deferred, ensureDeferred from twisted.python.failure import Failure From 11f78e829721f1bbd774b472a217a08c9fd9d403 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 28 Mar 2023 19:55:42 +0200 Subject: [PATCH 6/7] Simplify typing --- tests/unittest.py | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/tests/unittest.py b/tests/unittest.py index 515d872d37c4..54472719a2b3 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -128,34 +128,23 @@ def new(*args: P.args, **kwargs: P.kwargs) -> R: return _around -@overload -def deepcopy_config(config: RootConfig, root: Literal[True]) -> RootConfig: - ... +_TConfig = TypeVar("_TConfig", Config, RootConfig) -@overload -def deepcopy_config(config: Config, root: Literal[False]) -> Config: - ... +def deepcopy_config(config: _TConfig) -> _TConfig: + new_config: _TConfig - -def deepcopy_config( - config: Union[Config, RootConfig], root: bool -) -> Union[RootConfig, Config]: - new_config: Union[Config, RootConfig] - - if root: - typed_rootconfig = cast(RootConfig, config) - new_config = typed_rootconfig.__class__(typed_rootconfig.config_files) + if isinstance(config, RootConfig): + new_config = config.__class__(config.config_files) # type: ignore[arg-type] else: - typed_config = cast(Config, config) - new_config = typed_config.__class__(typed_config.root) + new_config = config.__class__(config.root) for attr_name in config.__dict__: if attr_name.startswith("__") or attr_name == "root": continue attr = getattr(config, attr_name) if isinstance(attr, Config): - new_attr = deepcopy_config(attr, root=False) + new_attr = deepcopy_config(attr) else: new_attr = attr @@ -193,7 +182,7 @@ def make_homeserver_config_obj(config: Dict[str, Any]) -> RootConfig: assert isinstance(config_obj, RootConfig) - return deepcopy_config(config_obj, root=True) + return deepcopy_config(config_obj) class TestCase(unittest.TestCase): From 4e589d34714f790ddd3cdd79309aa2e963cd0d81 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 28 Mar 2023 19:58:47 +0200 Subject: [PATCH 7/7] Remove unused imports --- tests/unittest.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unittest.py b/tests/unittest.py index 54472719a2b3..e93cbc4b7dbe 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -35,15 +35,13 @@ Type, TypeVar, Union, - cast, - overload, ) from unittest.mock import Mock, patch import canonicaljson import signedjson.key import unpaddedbase64 -from typing_extensions import Concatenate, Literal, ParamSpec, Protocol +from typing_extensions import Concatenate, ParamSpec, Protocol from twisted.internet.defer import Deferred, ensureDeferred from twisted.python.failure import Failure