twisted#12011 Speed up small TLS writes (twisted#12090)

eevelweezel · Jan 29, 2024 · 12c8a5c · 12c8a5c
2 parents dd96b6f + 0d60d5e
commit 12c8a5c
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 13 deletions.
diff --git a/src/twisted/newsfragments/12011.bugfix b/src/twisted/newsfragments/12011.bugfix
@@ -0,0 +1 @@
+twisted.protocols.tls.BufferingTLSTransport, used by default by twisted.protocols.tls.TLSMemoryBIOFactory, was refactored for improved performance when doing a high number of small writes.
diff --git a/src/twisted/protocols/test/test_tls.py b/src/twisted/protocols/test/test_tls.py
@@ -15,7 +15,8 @@
 
 from hypothesis import given, strategies as st
 
-from twisted.internet.task import Clock
+from twisted.internet import reactor
+from twisted.internet.task import Clock, deferLater
 from twisted.python.compat import iterbytes
 
 try:
@@ -698,13 +699,19 @@ class SimpleSendingProtocol(Protocol):
             def connectionMade(self):
                 try:
                     self.transport.write(notBytes)
+                    self.transport.write(b"bytes")
+                    self.transport.loseConnection()
                 except TypeError:
                     result.append(True)
-                self.transport.write(b"bytes")
-                self.transport.loseConnection()
+                    self.transport.abortConnection()
+
+        def flush_logged_errors():
+            self.assertEqual(len(self.flushLoggedErrors(ConnectionLost, TypeError)), 2)
 
         d = self.writeBeforeHandshakeTest(SimpleSendingProtocol, b"bytes")
-        return d.addCallback(lambda ign: self.assertEqual(result, [True]))
+        d.addBoth(lambda ign: self.assertEqual(result, [True]))
+        d.addBoth(lambda ign: deferLater(reactor, 0, flush_logged_errors))
+        return d
 
     def test_multipleWrites(self):
         """

diff --git a/src/twisted/protocols/tls.py b/src/twisted/protocols/tls.py
@@ -708,7 +708,7 @@ def __init__(self, write: Callable[[bytes], object], clock: IReactorTime):
         self._write = write
         self._clock = clock
         self._buffer: list[bytes] = []
-        self._bufferLen = 0
+        self._bufferLeft = self.MAX_BUFFER_SIZE
         self._scheduled: Optional[IDelayedCall] = None
 
     def write(self, data: bytes) -> None:
@@ -719,9 +719,9 @@ def write(self, data: bytes) -> None:
         Accumulating too much data can result in higher memory usage.
         """
         self._buffer.append(data)
-        self._bufferLen += len(data)
+        self._bufferLeft -= len(data)
 
-        if self._bufferLen > self.MAX_BUFFER_SIZE:
+        if self._bufferLeft < 0:
             # We've accumulated enough we should just write it out. No need to
             # schedule a flush, since we just flushed everything.
             self.flush()
@@ -744,7 +744,7 @@ def _scheduledFlush(self) -> None:
     def flush(self) -> None:
         """Flush any buffered writes."""
         if self._buffer:
-            self._bufferLen = 0
+            self._bufferLeft = self.MAX_BUFFER_SIZE
             self._write(b"".join(self._buffer))
             del self._buffer[:]
 
@@ -788,11 +788,10 @@ def __init__(
         super().__init__(factory, wrappedProtocol, _connectWrapped)
         actual_write = super().write
         self._aggregator = _AggregateSmallWrites(actual_write, factory._clock)
-
-    def write(self, data: bytes) -> None:
-        if isinstance(data, str):  # type: ignore[unreachable]
-            raise TypeError("Must write bytes to a TLS transport, not str.")
-        self._aggregator.write(data)
+        # This is kinda ugly, but speeds things up a lot in a hot path with
+        # lots of small TLS writes. May become unnecessary in Python 3.13 or
+        # later if JIT and/or inlining becomes a thing.
+        self.write = self._aggregator.write  # type: ignore[method-assign]
 
     def writeSequence(self, sequence: Iterable[bytes]) -> None:
         self._aggregator.write(b"".join(sequence))
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		twisted.protocols.tls.BufferingTLSTransport, used by default by twisted.protocols.tls.TLSMemoryBIOFactory, was refactored for improved performance when doing a high number of small writes.