Skip to content

Avoid per-token full-string scans when unescaping Markdown - #724

Merged
nicholasserra merged 1 commit into
trentm:masterfrom
codeofwxz:perf/batch-unescape-tokens-635
Sep 13, 2026
Merged

nicholasserra merged 1 commit into
trentm:masterfrom
codeofwxz:perf/batch-unescape-tokens-635

Conversation

@codeofwxz

Copy link
Copy Markdown
Contributor

_unescape_special_chars repeatedly scans the full output once for every stored escape/code/HTML token. Documents with many distinct code spans spend an increasing share of conversion time in these replacements.

Scan for generated tokens once per expansion layer and look them up in a reverse mapping. Keep expanding until the text is stable, preserving nested code/HTML/escape content. Keep the first mapping for duplicate tokens to match the original replacement priority, and leave unknown tokens untouched.

Related to #635. Add six semantic regression tests and perf/issue635.py, which generates 500/1,000/2,000/4,000 distinct code spans, takes three-run medians, and checks exact HTML output without timing assertions. On Windows/Python 3.9.13, the 4,000-span unescape median changed from 431.7 ms to 5.6 ms; total conversion from 725.8 ms to 209.8 ms. Output hashes match at all sizes. These are local synthetic measurements, not a general speed guarantee.

Validation: the project's main suite passes 275 tests on Python 3.9.13 and 3.14.4. ReDoS results on 3.14 are mixed: one modified run passed all seven checks (issue493: 3.781 s), while a later review run timed out on issue493 at 4.015 s and passed the other six. One clean-baseline comparison with the same interpreter passed all seven (issue493: 3.779 s). These non-simultaneous measurements do not establish the cause of the timeout; stable ReDoS success is not claimed. On 3.9, issue493 hits the existing four-second limit both before and after the change; the other six pass. No timeout or unrelated algorithm was changed. The standard runner's known-failure and Pygments-version exclusions remain unchanged.

The lookup pattern follows the existing salted md5- + 32 lowercase hex token format. Directly injecting cyclic or nonstandard tokens into private tables is not supported by this optimization; ordinary generated tokens and current extras were checked.

A separate bounded issue493 diagnostic ran exactly three alternating baseline/modified pairs in one Python 3.14 process with a shared salt. Unescaping was faster in every pair; total conversion varied in both directions, with medians of 5.577 s baseline and 5.532 s modified. All six output hashes matched. This did not show consistent overall degradation, but is not an official ReDoS pass or a guarantee of staying below its four-second limit.

Copilot AI lite review requested due to automatic review settings September 12, 2026 13:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking issues were identified, and all supplied assessments support approval.

Pull request overview

Optimizes Markdown token unescaping by using layered reverse token lookup while preserving nested content.

Changes:

  • Adds efficient reverse token mapping.
  • Adds six semantic regression tests.
  • Adds an issue #635 performance benchmark.
File summaries
File Summary
test/test_markdown2.py Adds regression coverage for token behavior.
perf/issue635.py Benchmarks conversion and unescaping performance.
lib/markdown2.py Optimizes special-character unescaping.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@nicholasserra

Copy link
Copy Markdown
Collaborator

Thanks!

@nicholasserra
nicholasserra merged commit 2962ae7 into trentm:master Sep 13, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants