parse_lines: avoid O(n^2) string rebuild on line continuations - #77
Closed
choudhryfrompak wants to merge 1 commit into
Closed
choudhryfrompak wants to merge 1 commit into
choudhryfrompak wants to merge 1 commit into
Conversation
Each continuation line was merged into the accumulated value with
f"{last.value}\n{data}", which rebuilds and copies the entire
accumulated string on every continuation line. For a value spread
across N continuation lines that's O(N) work per line, O(N^2) total -
confirmed locally: 50k lines ~1.6s, 100k lines ~7.7s, 200k lines
~32.6s, consistent with quadratic scaling. A single pathological
ini file in the low tens of MB can cost tens of seconds to minutes
of CPU.
Accumulate continuation fragments in a list per in-progress entry
and join once when the entry is finalized (a new value/section line
starts, or parsing ends), instead of rebuilding the string each line.
Preserves the original's exact falsy-value semantics (an empty/falsy
accumulated value gets replaced by the next fragment rather than
newline-joined, matching the original's "if last.value: concat,
else: replace" check) - verified with a differential test that runs
both the original algorithm (kept as a test oracle) and the new one
across 10 edge cases (empty values, falsy continuations interleaved
with real ones, continuations right after a section header, etc.)
and asserts identical results, plus a scaling test confirming the
fix (linear, not quadratic). Full existing suite (71 tests total)
passes unchanged. mypy --strict and ruff clean.
RonnyPfannschmidt
pushed a commit
to RonnyPfannschmidt/iniconfig
that referenced
this pull request
Oct 5, 2026
Each continuation line rebuilt the whole accumulated value string, which is quadratic in the number of continuation lines of a value. Collect continuation lines per entry and join them once at the end. Alternative to pytest-dev#77 with a smaller change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012yrarEe7i67yuTE8GC22ey
RonnyPfannschmidt
pushed a commit
to RonnyPfannschmidt/iniconfig
that referenced
this pull request
Oct 5, 2026
Each continuation line rebuilt the whole accumulated value string, which is quadratic in the number of continuation lines of a value. ParsedLine.value is now the list of value lines; continuations append to it and parse_ini_data joins it once. Alternative to pytest-dev#77 with a smaller change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012yrarEe7i67yuTE8GC22ey
Member
|
closing as ai slop - the chosen implementation is horrible and overcomplicated, the attack vector is ludicrously contrived #78 does it properly |
RonnyPfannschmidt
pushed a commit
to RonnyPfannschmidt/iniconfig
that referenced
this pull request
Oct 5, 2026
Each continuation line rebuilt the whole accumulated value string, which is quadratic in the number of continuation lines of a value. ParsedLine.value is now the list of value lines; continuations append to it and parse_ini_data joins it once. Alternative to pytest-dev#77 with a smaller change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012yrarEe7i67yuTE8GC22ey
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each continuation line was merged into the accumulated value with
f"{last.value}\n{data}", which rebuilds and copies the entire accumulated string on every continuation line. For a value spread across N continuation lines that's O(N) work per line, O(N^2) total — confirmed locally: 50k lines ~1.6s, 100k lines ~7.7s, 200k lines ~32.6s, consistent with quadratic scaling. A single pathological ini file in the low tens of MB can cost tens of seconds to minutes of CPU.Fix: accumulate continuation fragments in a list per in-progress entry and join once when the entry is finalized (a new value/section line starts, or parsing ends), instead of rebuilding the string on every line.
The original had a subtle falsy-value quirk (an empty/falsy accumulated value gets replaced by the next fragment rather than newline-joined, matching its
if last.value: concat, else: replacecheck) that I wanted to preserve exactly rather than risk changing. Verified with a differential test (test_parse_lines_perf.py) that keeps the original algorithm as a test oracle and runs both implementations across 10 edge cases (empty values, falsy continuations interleaved with real ones, continuations right after a section header, error cases) asserting identical results — plus a scaling test confirming linear rather than quadratic growth.Full existing suite (71 tests total) passes unchanged.
mypy --strictandruffboth clean.