Fix increased memory usage

changelog.d/20260731_163747_kurtmckee_rm_quadratic_behavior.rst

@@ -1,7 +1,9 @@
 Fixed
 -----
 
-*   Fix quadratic search behavior in ``SGMLParser.feed()``.
+*   Fix quadratic CPU usage in ``SGMLParser.feed()``.
 
     This can occur when a large document with an unfinished token
     (like ``<a href="...``) is fed into the parser in very small increments.
+
+    The new implementation doesn't significantly change peak memory usage.

src/feedparser_sgmllib/__init__.py

@@ -13,6 +13,8 @@
 import re
 import typing as t
 
+from ._compat import StringIO
+
 __all__ = ["SGMLParser", "SGMLParseError"]
 
 # Regular expressions used for parsing
@@ -66,20 +68,25 @@ def __init__(self, verbose: bool = False) -> None:
         """Initialize and reset this instance."""
         self.verbose = verbose
 
-        # These variables help guard against quadratic search behavior.
+        # These variables help guard against quadratic CPU usage behavior
+        # without significantly increasing memory usage.
+        #
         # Previously, each call to `.feed()` resulted in string-concatenation
-        # and a regular expression matching from the start of the raw data.
+        # and a regular expression matching from the start of the raw data
+        # even though it was known that the existing raw data wouldn't match.
         # For large unclosed tokens (like `<a href="...`) fed in small chunks,
-        # this could become quadratically expensive.
+        # this became quadratically expensive.
         #
-        # This quadratic behavior is addressed by:
+        # This quadratic CPU behavior is addressed by:
         #
         # * Gating failing calls to `.goahead()` behind an exponential backoff
         # * Avoiding string concatenation until it's needed
         #
+        # Memory usage is guarded by using an object with a resizeable buffer;
+        # a simple list of strings can increase memory usage significantly.
+        #
         self._rawdata = ""
-        self._pending: list[str] = []
-        self._pending_content_length = 0
+        self._pending = StringIO()
         self._retry_at = 0
 
         self.reset()
@@ -88,8 +95,7 @@ def reset(self) -> None:
         """Reset this instance. Loses all unprocessed data."""
         self.__starttag_text: str | None = None
         self._rawdata = ""
-        self._pending = []
-        self._pending_content_length = 0
+        self._pending = StringIO()
         self._retry_at = 0
         self.stack: list[str] = []
         self.lasttag = "???"
@@ -105,16 +111,14 @@ def rawdata(self) -> str:
     @rawdata.setter
     def rawdata(self, value: str) -> None:
         self._rawdata = value
-        self._pending = []
-        self._pending_content_length = 0
+        self._pending = StringIO()
 
     def _flush_pending(self) -> None:
         """Merge any chunks queued by `feed()` into `self._rawdata`."""
 
-        if self._pending:
-            self._rawdata += "".join(self._pending)
-            self._pending = []
-            self._pending_content_length = 0
+        if self._pending.tell():
+            self._rawdata += self._pending.getvalue()
+            self._pending = StringIO()
 
     def setnomoretags(self) -> None:
         """Enter literal mode (CDATA) till EOF.
@@ -138,9 +142,8 @@ def feed(self, data: str) -> None:
         all the processing is done by goahead().)
         """
 
-        self._pending.append(data)
-        self._pending_content_length += len(data)
-        if len(self._rawdata) + self._pending_content_length < self._retry_at:
+        self._pending.write(data)
+        if len(self._rawdata) + self._pending.tell() < self._retry_at:
             # The last attempt to resolve the pending token failed.
             # Skip re-scanning until enough new data has arrived
             # to make another attempt worthwhile.

src/feedparser_sgmllib/_compat.py

@@ -0,0 +1,38 @@
+# Manage compatibility across supported Python versions.
+
+import sys
+
+__all__ = [
+    "StringIO",
+]
+
+
+if sys.version_info >= (3, 12):
+    from io import StringIO
+else:
+    # In Python >=3.12, `io.StringIO` has good memory allocation performance,
+    # which makes it a good candidate for collecting unparsed input data
+    # (see the comment in `SGMLParser.__init__()` about CPU and memory usage).
+    #
+    # However, the performance of `io.StringIO` is poor in Python 3.11 and below.
+    # For these lower Python versions, bytearray exhibits good performance
+    # but requires manual tracking of the length of the decoded string
+    # as well as calls to `.encode()` and `.decode()`.
+    #
+    # Therefore, the `io.StringIO` interface is mimicked here
+    # and backed by a bytearray implementation.
+    class StringIO:
+        def __init__(self) -> None:
+            self._buffer = bytearray()
+            self._length = 0
+
+        def write(self, text: str) -> int:
+            self._buffer.extend(text.encode())
+            self._length += len(text)
+            return len(text)
+
+        def getvalue(self) -> str:
+            return self._buffer.decode()
+
+        def tell(self) -> int:
+            return self._length

tests/test_sgmllib.py

@@ -1,6 +1,7 @@
 import io
 import pathlib
 import time
+import tracemalloc
 
 import pytest
 
@@ -535,9 +536,9 @@ def test_endtag_with_no_valid_tag_name_does_not_close_open_tag(nested_tag_collec
         pytest.param("<![CDATA[", id="marked-section-missing-close"),
     ),
 )
-def test_feed_unresolved_token_scales_linearly(prefix):
+def test_feed_unresolved_token_scales_cpu_time_linearly(prefix):
     """
-    Verify that `.feed()` doesn't exhibit quadratic search behavior.
+    Verify `.feed()` doesn't exhibit quadratic CPU time usage.
 
     This is verified by creating a very large, incomplete token
     (like an unclosed start tag) and feeding it to the parser
@@ -580,6 +581,66 @@ def test_feed_unresolved_token_scales_linearly(prefix):
     )
 
 
+def test_feed_unresolved_token_peak_memory_scales_nicely():
+    """
+    Verify `.feed()` doesn't show a big peak memory usage constant multiplier.
+
+    This is verified by creating a very large, incomplete token
+    (like an unclosed start tag) and feeding it to the parser
+    one character at a time.
+
+    In the past, each character was concatenated each time `.feed()` is called.
+    For a large input, peak memory usage was always ~2x the current input size:
+    concatenation copied the existing string in memory to add a single character.
+
+    The initial fix for quadratic CPU usage replaced string concatenation
+    in favor of buffering incoming strings in a list.
+    However, this caused a jump in the peak memory usage constant multiplier:
+    now, in addition to the strings themselves, lots of references to the strings
+    had to be created and stored in the list buffer.
+
+    The solution was to replace the list of strings with something
+    which has a resizeable internal buffer and can store the incoming data
+    without the added cost of maintaining references or concatenating content.
+    """
+
+    # 200,000 should be big enough to dominate memory usage while the test runs
+    # so that peak memory usage is primarily influenced by the input payload
+    # and its final string-concatenation.
+    n = 200_000
+
+    prefix = '<a href="'
+    length = n - len(prefix)
+    payload = prefix + ("x" * length)
+    parser = sgmllib.SGMLParser()
+
+    tracemalloc.start()
+    try:
+        for ch in payload:
+            parser.feed(ch)
+        parser.close()
+        _, peak = tracemalloc.get_traced_memory()
+    finally:
+        tracemalloc.stop()
+
+    # The `max_allowed_ratio` reflects how large peak memory usage can be
+    # when compared to the size of the input payload.
+    # As long as the input payload dominates memory usage while the test runs,
+    # the string concatenation of the `SGMLParser._rawdata`
+    # with the buffered, unparsed data in `._pending` should double peak usage.
+    #
+    # The max allowed ratio is set to 3.0 to allow for other memory use,
+    # but is still lower than what was measured when a list of strings was used
+    # to accumulate unparsed input data.
+    max_allowed_ratio = 3.0
+    budget = n * max_allowed_ratio
+    assert peak <= budget, (
+        f"Peak traced memory ({peak:,} bytes) exceeded "
+        f"{max_allowed_ratio}x the input size ({len(payload):,} bytes) "
+        "while feeding an unresolved token byte-at-a-time."
+    )
+
+
 # XXX These tests have been disabled by prefixing their names with
 # an underscore.  The first two exercise outstanding bugs in the
 # sgmllib module, and the third exhibits questionable behavior