perf: speed up _unescape_special_chars for large documents by SwastikTripathi · Pull Request #748 · trentm/python-markdown2

SwastikTripathi · 2026-10-25T03:16:50Z

Fixes #635.

Problem

_unescape_special_chars has two hidden O(n) costs that compound on large markdown documents:

Full-string comparison for loop termination. Each pass through the while True loop stores orig_text = text and then compares text == orig_text to detect whether anything changed. On a large document this is an O(n) string comparison even when the very first regex scan found nothing to replace.
Unnecessary intermediate allocations on every call. The replacements dict was built from three tuple(...) calls, converting _escape_table.items(), _code_table.items(), and a generator expression over html_blocks.items(), before iterating them into the dict. That's three extra list/tuple objects per call.

Fix

Early exit: if 'md5-' is not present anywhere in text, return immediately. This is the common case for plain paragraphs that contain no inline code, links, backslash escapes, or raw HTML, skipping both the dict build and the regex engine entirely.
Direct dict construction: iterate each source dict once, without allocating intermediate tuples.
Flag-based termination: a replaced flag (a one-element list so the inner function can mutate it) is set inside the re.sub callback whenever a match is found in the lookup table. The loop breaks when a full pass sets no flag, no full-string comparison needed.

All existing tests pass (test/test.py, test/api.doctests).

🤖 Generated with Claude Code

Two O(n) costs were paid per pass through the while loop: 1. The termination check compared the full text string (`text == orig_text`), which is O(n) even when nothing changed. 2. The replacements dict was built from three intermediate tuples before every call (escape_table + code_table + reversed html_blocks). Fix both: - Skip immediately if 'md5-' is not present in the text (the common case for plain paragraphs with no inline code, links, or escapes). - Build the replacements dict by iterating the source dicts directly, without allocating intermediate tuples. - Replace the string comparison with a boolean flag set inside the re.sub callback; the loop breaks as soon as a full pass produces no replacements. Fixes trentm#635 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Crozzers

Do you have any perf data that demonstrates an improvement? Any tests or profiling you've done, maybe a before and after?

Crozzers · 2026-10-27T00:16:48Z

-        for ch, hash in hashmap:
+        if 'md5-' not in text:
+            return text
+        # Build hash→original mapping directly without intermediate tuple.


Nitpick but I would remove the → char in the comment in favour of plain text like ->.
I don't think it will cause any issues (python source is default utf-8) but I just think it can't be typed from a keyboard so should it be in a comment?

It also displays oddly in VSCode, really small and almost looks like an underline:

As opposed to this which is bigger and more legible:

Done in 4bf3ddb, it's plain -> now.

Crozzers · 2026-10-27T00:21:43Z

-            text = re.sub(r'md5-[0-9a-f]{32}',
-                          lambda match: replacements.get(match[0], match[0]), text)
-            if text == orig_text:
+            replaced = [False]


Interesting but odd pattern. Would nonlocal work here? Bit cleaner?

Yes, switched to nonlocal in 4bf3ddb.

Crozzers · 2026-10-27T00:26:00Z

-                          lambda match: replacements.get(match[0], match[0]), text)
-            if text == orig_text:
+            replaced = [False]
+            def _sub(match: re.Match, _rep: dict = replacements) -> str:


Does _rep need defining as a kwarg with a default value? Can the function not just use replacements directly?

Also _sub is being re-defined every loop?

No, it doesn't. It uses replacements directly now (4bf3ddb).

Good catch. It's defined once before the loop now, and only the flag is reset each pass (4bf3ddb).

Use a nonlocal flag instead of a one-element list, define the re.sub callback once outside the loop, use replacements directly instead of a default kwarg, and replace the arrow character with plain ASCII. Add a CHANGES entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

SwastikTripathi · 2026-10-27T05:01:41Z

Thanks for the review. I hadn't measured it before, so I did now.

Per-call cost, after converting a doc with 2000 code spans and 2000 HTML blocks (so _code_table and html_blocks have 2000 entries each), calling _unescape_special_chars on a short string like a header or link text:

input	before	after
short text, no hash	346 us/call	0.1 us/call
short text, one hash	344 us/call	159 us/call

Most of the old cost was rebuilding the replacements dict on every call, which grows with the size of the tables, so on big documents the per-header and per-link calls get expensive. The early return skips that entirely when there's nothing to replace.

End to end the gain is smaller than that suggests. On a synthetic 420 KB doc (2000 sections, each with a header, inline code, a link, an escape and inline HTML, extras toc, header-ids, fenced-code-blocks, tables) cProfile shows the function's cumulative time going from 0.101s to 0.026s over 6 conversions, but total wall time is about the same (1.04s vs 1.02s median), because the function is only a few percent of runtime on that doc. So this mainly helps documents with many hashed blocks and many headers or links, which is the shape in #635. I don't have the reporter's file to confirm their case.

Output is byte-identical between before and after on the benchmark docs (compared by sha1 of html + toc_html), and test/test.py gives the same result as master (300 tests, only the 14 knownfailure ones fail).

The inline nits are done in 4bf3ddb: nonlocal flag, _sub defined once outside the loop and using replacements directly, ASCII -> in the comment. I also added a CHANGES entry.

# Conflicts: # CHANGES.md

nicholasserra · 2026-10-29T22:29:23Z

@Crozzers ill let you have final say

Crozzers · 2026-11-06T22:38:00Z

+        for hash, item in self.html_blocks.items():
+            replacements.setdefault(hash, item)
+
+        replaced = False


replaced defined twice? Gets defined in the loop before sub is called so this is unneeded?

Or, this could be done:

replaced=True while replaced: replaced=False text = re.sub(...) return text

Removes need for if/break branch

Done in be9bb4e, it uses while replaced: now, which also drops the extra replaced = False before _sub. I also merged master to clear the CHANGES.md conflict with #762. I could not run the suite locally this time, so CI is the check on this push.

Right, it was only there so the name existed before _sub closed over it. Gone in be9bb4e.

Follow-up: I got the suite running locally after all. On be9bb4e, python test.py runs 316 tests with the same 14 known failures as master and nothing new.

Crozzers · 2026-11-06T22:41:25Z

-        replacements = {}
-        for ch, hash in hashmap:
+        replacements: dict[str, str] = {}
+        for ch, hash in self._escape_table.items():


Would it be perhaps faster and simpler if instead of building a replacements dict, you just did this in the _sub function:

val = ( self._escape_table.get(match[0]) or self._code_table.get(match[0]) or self.html_blocks.get(match[0]) )

Maybe more memory efficient?

That works for html_blocks, which is keyed by hash, but _escape_table and _code_table go the other way ({original: hash}, e.g. self._code_table[text] = hashed), so .get(match[0]) on them would always miss. To skip the dict build we would need reverse tables kept in sync at every place those two are written (the escape table is also written per URL in _protect_url). The early md5- check already skips the build for most calls, so I kept the dict here. Happy to add the reverse tables if you prefer that.

Ah yes, forgot about that. In that case building the map is probably the best way

…al-chars

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Crozzers · 2026-11-08T21:30:48Z

LGTM

Crozzers reviewed Oct 27, 2026

View reviewed changes

Merge remote-tracking branch 'origin/master' into md2-748-merge

ed6330a

# Conflicts: # CHANGES.md

Crozzers reviewed Nov 6, 2026

View reviewed changes

SwastikTripathi and others added 2 commits November 7, 2026 08:32

Merge remote-tracking branch 'origin/master' into perf/unescape-speci…

58cd6cb

…al-chars

Simplify the unescape loop to while replaced

be9bb4e

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Merge branch 'master' into perf/unescape-special-chars

a13e621

nicholasserra merged commit 7975553 into trentm:master Nov 9, 2026
15 checks passed

Conversation

SwastikTripathi commented Oct 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Uh oh!

Crozzers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SwastikTripathi commented Oct 27, 2026

Uh oh!

nicholasserra commented Oct 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Crozzers commented Nov 8, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SwastikTripathi commented Oct 25, 2026 •

edited

Loading