Repository navigation
Skip regex passes that cannot match (~29% faster slugify) - #211
Closed
deadstrobe5 wants to merge 1 commit into
Closed
deadstrobe5 wants to merge 1 commit into
deadstrobe5 wants to merge 1 commit into
Conversation
Owner
|
Thank you. This is careful, well-argued work, and I want to say that plainly before the no. I verified it rather than taking the writeup on trust: 9,872 differential cases came out byte-identical to master (both algorithms, entities, quotes, commas, dash runs, combining marks, custom regex patterns), all 133 tests pass, and I reproduced the speedup here — about 52% faster on ordinary titles, a little over a microsecond saved per call. The nested-entity reasoning is correct too. I'm closing it for two reasons, neither of them about quality:
If you ever hit an actual slugify bug in real use, please do file it as an issue — this level of rigor would be very welcome there. 🚀 Generated with Dojo ⛩️ |
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.
Why
Every
slugify()call runs five or six regex passes, even when a pass can't match. On a typical ASCII title the no-match passes cost about 1.8 µs of about 5.6 µs. The worst isNUMBERS_PATTERN: its lookbehind scans the whole string even when there is no comma (~870 ns per call).Change
Each pass now runs only when the literal it needs is present:
&for entity decoding,'for both quote passes,,for the numbers pass, and--for the duplicate-dash pass. The same guards are in_legacy.pyandslugify.py. Without that literal,re.subwould have returned its input unchanged, so output can't change.Results
CPython 3.11, Apple M5, 15 alternating in-process rounds, median ns per call over 12 realistic inputs (titles, accents, pinyin, CJK, emoji,
&):max_length+word_boundarystopwordsallow_unicodealgorithm='modern'Risks and how they are covered
tests/test_legacy.pypasses (unchanged).&still decodes, because the&check runs once at entry and the original sequential passes follow.Tests
133 passed (143 subtests). flake8, pycodestyle and mypy are clean.
Re-measured on CPython 3.14.7 (7 alternating processes per side, median): default slugify() on 10 typical titles x300 went from 27.3 ms to 24.0 ms (1.14x), smaller than the 3.11 numbers above.