From 46cd610f52618a51fd5f297834af8a3415f30e4d Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 19 Sep 2023 10:58:47 -0400
Subject: [PATCH 1/9] Add type hitns to synmark.

---
 mypy.ini                         |  4 +++
 synmark/__init__.py              |  9 ++++--
 synmark/__main__.py              | 33 ++++++++++++--------
 synmark/suites/logging.py        | 52 +++++++++++++++++++-------------
 synmark/suites/lrucache.py       |  5 +--
 synmark/suites/lrucache_evict.py |  5 +--
 6 files changed, 67 insertions(+), 41 deletions(-)

diff --git a/mypy.ini b/mypy.ini
index 88aea301b9d1..fdfe9432fcc7 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -32,6 +32,7 @@ files =
   docker/,
   scripts-dev/,
   synapse/,
+  synmark/,
   tests/,
   build_rust.py
 
@@ -80,6 +81,9 @@ ignore_missing_imports = True
 [mypy-pympler.*]
 ignore_missing_imports = True
 
+[mypy-pyperf.*]
+ignore_missing_imports = True
+
 [mypy-rust_python_jaeger_reporter.*]
 ignore_missing_imports = True
 
diff --git a/synmark/__init__.py b/synmark/__init__.py
index 2cc00b0f03d3..f21331954288 100644
--- a/synmark/__init__.py
+++ b/synmark/__init__.py
@@ -13,15 +13,18 @@
 # limitations under the License.
 
 import sys
+from typing import cast
+
+from synapse.types import ISynapseReactor
 
 try:
     from twisted.internet.epollreactor import EPollReactor as Reactor
 except ImportError:
-    from twisted.internet.pollreactor import PollReactor as Reactor
+    from twisted.internet.pollreactor import PollReactor as Reactor  # type: ignore[assignment]
 from twisted.internet.main import installReactor
 
 
-def make_reactor():
+def make_reactor() -> ISynapseReactor:
     """
     Instantiate and install a Twisted reactor suitable for testing (i.e. not the
     default global one).
@@ -32,4 +35,4 @@ def make_reactor():
         del sys.modules["twisted.internet.reactor"]
     installReactor(reactor)
 
-    return reactor
+    return cast(ISynapseReactor, reactor)
diff --git a/synmark/__main__.py b/synmark/__main__.py
index 19de6391877f..03c5087fce6c 100644
--- a/synmark/__main__.py
+++ b/synmark/__main__.py
@@ -12,9 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import sys
-from argparse import REMAINDER
+from argparse import REMAINDER, Namespace
 from contextlib import redirect_stderr
 from io import StringIO
+from typing import Any, Callable, Coroutine, List, TypeVar, cast
 
 import pyperf
 
@@ -22,44 +23,49 @@
 from twisted.logger import globalLogBeginner, textFileLogObserver
 from twisted.python.failure import Failure
 
+from synapse.types import ISynapseReactor
 from synmark import make_reactor
 from synmark.suites import SUITES
 
 from tests.utils import setupdb
 
+T = TypeVar("T")
 
-def make_test(main):
+
+def make_test(
+    main: Callable[[ISynapseReactor, int], Coroutine[float, Any, Any]]
+) -> Callable[[int], float]:
     """
     Take a benchmark function and wrap it in a reactor start and stop.
     """
 
-    def _main(loops):
+    def _main(loops: int) -> float:
         reactor = make_reactor()
 
         file_out = StringIO()
         with redirect_stderr(file_out):
-            d = Deferred()
-            d.addCallback(lambda _: ensureDeferred(main(reactor, loops)))
+            d: "Deferred[float]" = Deferred()
+            d.addCallback(lambda _: ensureDeferred(main(reactor, loops)))  # type: ignore
 
-            def on_done(_):
-                if isinstance(_, Failure):
-                    _.printTraceback()
+            def on_done(res: T) -> T:
+                if isinstance(res, Failure):
+                    res.printTraceback()
                     print(file_out.getvalue())
                 reactor.stop()
-                return _
+                return res
 
             d.addBoth(on_done)
             reactor.callWhenRunning(lambda: d.callback(True))
             reactor.run()
 
-        return d.result
+        return cast(float, d.result)
 
     return _main
 
 
 if __name__ == "__main__":
 
-    def add_cmdline_args(cmd, args):
+    def add_cmdline_args(cmd: List[str], args: Namespace) -> None:
         if args.log:
             cmd.extend(["--log"])
         cmd.extend(args.tests)
@@ -89,10 +95,11 @@ def add_cmdline_args(cmd, args):
     for suite, loops in SUITES:
         if loops:
             runner.args.loops = loops
+            loops_desc = str(loops)
         else:
             runner.args.loops = orig_loops
-            loops = "auto"
+            loops_desc = "auto"
         runner.bench_time_func(
-            suite.__name__ + "_" + str(loops),
+            suite.__name__ + "_" + loops_desc,
             make_test(suite.main),
         )
diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index 04e5b29dc95d..2cc6dde376d8 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -11,14 +11,19 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-
+import json
 import logging
+import logging.config
+import tempfile
 import warnings
 from io import StringIO
+from typing import Optional
 from unittest.mock import Mock
 
 from pyperf import perf_counter
+from synapse.synapse_rust import reset_logging_config
 
+from twisted.internet.address import IPv4Address, IPv6Address
 from twisted.internet.defer import Deferred
 from twisted.internet.protocol import ServerFactory
 from twisted.logger import LogBeginner, LogPublisher
@@ -26,45 +31,45 @@
 
 from synapse.config.logger import _setup_stdlib_logging
 from synapse.logging import RemoteHandler
+from synapse.types import ISynapseReactor
 from synapse.util import Clock
 
 
 class LineCounter(LineOnlyReceiver):
     delimiter = b"\n"
+    count = 0
 
-    def __init__(self, *args, **kwargs):
-        self.count = 0
-        super().__init__(*args, **kwargs)
-
-    def lineReceived(self, line):
+    def lineReceived(self, line: bytes) -> None:
         self.count += 1
 
+        assert isinstance(self.factory, Factory)
+
         if self.count >= self.factory.wait_for and self.factory.on_done:
             on_done = self.factory.on_done
             self.factory.on_done = None
             on_done.callback(True)
 
 
-async def main(reactor, loops):
+class Factory(ServerFactory):
+    protocol = LineCounter
+    wait_for: int
+    on_done: Optional[Deferred]
+
+
+async def main(reactor: ISynapseReactor, loops: int) -> float:
     """
     Benchmark how long it takes to send `loops` messages.
     """
-    servers = []
-
-    def protocol():
-        p = LineCounter()
-        servers.append(p)
-        return p
 
-    logger_factory = ServerFactory.forProtocol(protocol)
+    logger_factory = Factory()
     logger_factory.wait_for = loops
     logger_factory.on_done = Deferred()
-    port = reactor.listenTCP(0, logger_factory, interface="127.0.0.1")
+    port = reactor.listenTCP(0, logger_factory, backlog=50, interface="127.0.0.1")
 
     # A fake homeserver config.
     class Config:
-        server_name = "synmark-" + str(loops)
-        no_redirect_stdio = True
+        class server:
+            server_name = "synmark-" + str(loops)
 
     hs_config = Config()
 
@@ -78,6 +83,8 @@ class Config:
         publisher, errors, mock_sys, warnings, initialBufferSize=loops
     )
 
+    address = port.getHost()
+    assert isinstance(address, (IPv4Address, IPv6Address))
     log_config = {
         "version": 1,
         "loggers": {"synapse": {"level": "DEBUG", "handlers": ["tersejson"]}},
@@ -86,20 +93,23 @@ class Config:
             "tersejson": {
                 "class": "synapse.logging.RemoteHandler",
                 "host": "127.0.0.1",
-                "port": port.getHost().port,
+                "port": address.port,
                 "maximum_buffer": 100,
-                "_reactor": reactor,
             }
         },
     }
 
     logger = logging.getLogger("synapse.logging.test_terse_json")
     _setup_stdlib_logging(
-        hs_config,
-        log_config,
+        hs_config,  # type: ignore[arg-type]
+        None,
         logBeginner=beginner,
     )
 
+    # Force a new logging config without having to load it from a file.
+    logging.config.dictConfig(log_config)
+    reset_logging_config()
+
     # Wait for it to connect...
     for handler in logging.getLogger("synapse").handlers:
         if isinstance(handler, RemoteHandler):
diff --git a/synmark/suites/lrucache.py b/synmark/suites/lrucache.py
index 9b4a4241493f..cfa0163c6279 100644
--- a/synmark/suites/lrucache.py
+++ b/synmark/suites/lrucache.py
@@ -14,14 +14,15 @@
 
 from pyperf import perf_counter
 
+from synapse.types import ISynapseReactor
 from synapse.util.caches.lrucache import LruCache
 
 
-async def main(reactor, loops):
+async def main(reactor: ISynapseReactor, loops: int) -> float:
     """
     Benchmark `loops` number of insertions into LruCache without eviction.
     """
-    cache = LruCache(loops)
+    cache: LruCache[int, bool] = LruCache(loops)
 
     start = perf_counter()
 
diff --git a/synmark/suites/lrucache_evict.py b/synmark/suites/lrucache_evict.py
index 0ee202ed3626..02238c2627f5 100644
--- a/synmark/suites/lrucache_evict.py
+++ b/synmark/suites/lrucache_evict.py
@@ -14,15 +14,16 @@
 
 from pyperf import perf_counter
 
+from synapse.types import ISynapseReactor
 from synapse.util.caches.lrucache import LruCache
 
 
-async def main(reactor, loops):
+async def main(reactor: ISynapseReactor, loops: int) -> float:
     """
     Benchmark `loops` number of insertions into LruCache where half of them are
     evicted.
     """
-    cache = LruCache(loops // 2)
+    cache: LruCache[int, bool] = LruCache(loops // 2)
 
     start = perf_counter()
 

From 6acab51a59a11b2efeb322c67d5c5bab25221b71 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 2 Oct 2023 15:58:14 -0400
Subject: [PATCH 2/9] Newsfragment

---
 changelog.d/16421.misc | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changelog.d/16421.misc

diff --git a/changelog.d/16421.misc b/changelog.d/16421.misc
new file mode 100644
index 000000000000..93ceaeafc9b9
--- /dev/null
+++ b/changelog.d/16421.misc
@@ -0,0 +1 @@
+Improve type hints.

From 09f15ed8f59181e1a11fb0be4d6aad4a04f13667 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Mon, 2 Oct 2023 16:06:03 -0400
Subject: [PATCH 3/9] Fix-up imports.

---
 synmark/suites/logging.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index 2cc6dde376d8..4ecde504326c 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -21,7 +21,6 @@
 from unittest.mock import Mock
 
 from pyperf import perf_counter
-from synapse.synapse_rust import reset_logging_config
 
 from twisted.internet.address import IPv4Address, IPv6Address
 from twisted.internet.defer import Deferred
@@ -31,6 +30,7 @@
 
 from synapse.config.logger import _setup_stdlib_logging
 from synapse.logging import RemoteHandler
+from synapse.synapse_rust import reset_logging_config
 from synapse.types import ISynapseReactor
 from synapse.util import Clock
 

From ea136f41fab48f0e0804af36d20e67eac40fb19d Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Tue, 3 Oct 2023 06:46:09 -0400
Subject: [PATCH 4/9] Unused imports.

---
 synmark/suites/logging.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index 4ecde504326c..79625ba1f887 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -11,10 +11,8 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-import json
 import logging
 import logging.config
-import tempfile
 import warnings
 from io import StringIO
 from typing import Optional

From 38a6da677e1e979a4a4e88f2608a3ca455de07b6 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 4 Oct 2023 11:31:43 -0400
Subject: [PATCH 5/9] Review comments.

---
 synmark/suites/logging.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index 79625ba1f887..43853467fa2b 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -69,6 +69,9 @@ class Config:
         class server:
             server_name = "synmark-" + str(loops)
 
+        class logging:
+            no_redirect_stdio = True
+
     hs_config = Config()
 
     # To be able to sleep.
@@ -90,7 +93,7 @@ class server:
         "handlers": {
             "tersejson": {
                 "class": "synapse.logging.RemoteHandler",
-                "host": "127.0.0.1",
+                "host": address.host,
                 "port": address.port,
                 "maximum_buffer": 100,
             }

From eff474dd794f6ba97939b43e336f6365a9a75f2f Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 4 Oct 2023 12:28:17 -0400
Subject: [PATCH 6/9] Review comments.

---
 synmark/__main__.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/synmark/__main__.py b/synmark/__main__.py
index 03c5087fce6c..1c33144de017 100644
--- a/synmark/__main__.py
+++ b/synmark/__main__.py
@@ -33,7 +33,7 @@
 
 
 def make_test(
-    main: Callable[[ISynapseReactor, int], Coroutine[float, Any, Any]]
+    main: Callable[[ISynapseReactor, int], Coroutine[Any, Any, float]]
 ) -> Callable[[int], float]:
     """
     Take a benchmark function and wrap it in a reactor start and stop.
@@ -45,7 +45,7 @@ def _main(loops: int) -> float:
         file_out = StringIO()
         with redirect_stderr(file_out):
             d: "Deferred[float]" = Deferred()
-            d.addCallback(lambda _: ensureDeferred(main(reactor, loops)))  # type: ignore
+            d.addCallback(lambda _: ensureDeferred(main(reactor, loops)))
 
             def on_done(res: T) -> T:
                 if isinstance(res, Failure):
@@ -58,7 +58,8 @@ def on_done(res: T) -> T:
             reactor.callWhenRunning(lambda: d.callback(True))
             reactor.run()
 
-        return cast(float, d.result)
+        # mypy thinks this is an object for some reason.
+        return d.result  # type: ignore[return-value]
 
     return _main
 

From 2975f72d4a214e4e50cdaaa50603ea5c240c602a Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 4 Oct 2023 12:35:46 -0400
Subject: [PATCH 7/9] Log an error for a non-existant test suite.

---
 synmark/__main__.py | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/synmark/__main__.py b/synmark/__main__.py
index 1c33144de017..934aa4e51a56 100644
--- a/synmark/__main__.py
+++ b/synmark/__main__.py
@@ -89,11 +89,19 @@ def add_cmdline_args(cmd: List[str], args: Namespace) -> None:
         setupdb()
 
     if runner.args.tests:
-        SUITES = list(
-            filter(lambda x: x[0].__name__.split(".")[-1] in runner.args.tests, SUITES)
+        existing_suites = {s.__name__.split(".")[-1] for s, _ in SUITES}
+        for test in runner.args.tests:
+            if test not in existing_suites:
+                print(f"Test suite {test} does not exist.")
+                exit(-1)
+
+        suites = list(
+            filter(lambda t: t[0].__name__.split(".")[-1] in runner.args.tests, SUITES)
         )
+    else:
+        suites = SUITES
 
-    for suite, loops in SUITES:
+    for suite, loops in suites:
         if loops:
             runner.args.loops = loops
             loops_desc = str(loops)

From 4ab824054b7221bfbeb2b6fe222f68b63f7b50e2 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 4 Oct 2023 13:22:20 -0400
Subject: [PATCH 8/9] A few other fixes.

---
 synmark/suites/logging.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index 43853467fa2b..cb5cd5584d3d 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -88,11 +88,12 @@ class logging:
     assert isinstance(address, (IPv4Address, IPv6Address))
     log_config = {
         "version": 1,
-        "loggers": {"synapse": {"level": "DEBUG", "handlers": ["tersejson"]}},
+        "loggers": {"synapse": {"level": "DEBUG", "handlers": ["remote"]}},
         "formatters": {"tersejson": {"class": "synapse.logging.TerseJsonFormatter"}},
         "handlers": {
-            "tersejson": {
+            "remote": {
                 "class": "synapse.logging.RemoteHandler",
+                "formatter": "tersejson",
                 "host": address.host,
                 "port": address.port,
                 "maximum_buffer": 100,
@@ -100,7 +101,7 @@ class logging:
         },
     }
 
-    logger = logging.getLogger("synapse.logging.test_terse_json")
+    logger = logging.getLogger("synapse")
     _setup_stdlib_logging(
         hs_config,  # type: ignore[arg-type]
         None,
@@ -118,7 +119,7 @@ class logging:
     else:
         raise RuntimeError("Improperly configured: no RemoteHandler found.")
 
-    await handler._service.whenConnected()
+    await handler._service.whenConnected(failAfterFailures=10)
 
     start = perf_counter()
 

From 8df30486ee24479ddc4ec7a1f927aad7adedc4a2 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <patrickc@matrix.org>
Date: Wed, 4 Oct 2023 13:29:43 -0400
Subject: [PATCH 9/9] Lint

---
 synmark/__main__.py       | 2 +-
 synmark/suites/logging.py | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/synmark/__main__.py b/synmark/__main__.py
index 934aa4e51a56..397dd86576be 100644
--- a/synmark/__main__.py
+++ b/synmark/__main__.py
@@ -15,7 +15,7 @@
 from argparse import REMAINDER, Namespace
 from contextlib import redirect_stderr
 from io import StringIO
-from typing import Any, Callable, Coroutine, List, TypeVar, cast
+from typing import Any, Callable, Coroutine, List, TypeVar
 
 import pyperf
 
diff --git a/synmark/suites/logging.py b/synmark/suites/logging.py
index cb5cd5584d3d..e16044364363 100644
--- a/synmark/suites/logging.py
+++ b/synmark/suites/logging.py
@@ -69,9 +69,13 @@ class Config:
         class server:
             server_name = "synmark-" + str(loops)
 
-        class logging:
+        # This odd construct is to avoid mypy thinking that logging escapes the
+        # scope of Config.
+        class _logging:
             no_redirect_stdio = True
 
+        logging = _logging
+
     hs_config = Config()
 
     # To be able to sleep.