Repository navigation
Conversation
This branch has not been deployed
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.
is_emergency_number() and connects_to_emergency_number() hand their argument to _extract_possible_number() with no length limit; parse() rejects anything over _MAX_INPUT_STRING_LENGTH before that call, these two don't. _extract_possible_number() strips trailing punctuation with _UNWANTED_END_CHAR_PATTERN.search(), and
(?:_|[^#\w])+$starts a match at every position of a separator run that isn't at the end of the string, consumes the rest of the run and then fails on the$. That makes the call quadratic in the run length: is_emergency_number("1" + "-" * n + "1", "US") takes about 0.28s at n=2000, 4.3s at 8000, 15s at 16000 and 90s at 32000 on CPython 3.9, and these functions are usually fed a raw dialled string. A one-character negative lookbehind on the same class means a match can only start at the beginning of a run, so each run is scanned once: the n=32000 case now takes 13 ms and 1 MB takes about 1s. The leftmost match is always the start of the trailing run, so the match span is unchanged for every input (I compared old and new search() spans on ~636k random and exhaustive short strings). The fix is in the pattern rather than another length cap because PhoneNumberMatcher shares it (its candidates are already bounded, so it isn't affected in practice).testEmergencyNumberLongSeparatorRun fails on dev (50s, hits the timing assert) and passes in 8 ms with the change. make -C tools/python lint and test pass (309 tests), and the metaclean regeneration leaves no diff.