-
-
Notifications
You must be signed in to change notification settings - Fork 78
Use frozensets for some membership tests #607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: adamchainz <[email protected]>
Co-authored-by: adamchainz <[email protected]>
Co-authored-by: adamchainz <[email protected]>
adamchainz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # Compiled regex patterns for performance | ||
| PATH_ALLOWED_CHARS_RE = re.compile(r"[a-zA-Z0-9_\-./<>:]*") | ||
| LITERAL_DOT_RE = re.compile(r"\\\.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo - fixing URLs is not on the hot path, as it only occurs on the rare case we have a match. i'd rather have fast import of this module and rely on the re module's internal caching when required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7b2ebb4. Reverted the regex pre-compilation since URL fixing is not on the hot path.
src/django_upgrade/data.py
Outdated
| or node.module in ("django", "unittest") | ||
| or node.module in DJANGO_MODULES_TO_TRACK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo, for a tuple of length 2, an inline co_const tuple is gonna be faster
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7b2ebb4. Reverted to inline tuple ("django", "unittest").
src/django_upgrade/main.py
Outdated
| |+----UNIMPORTANT_WS | ||
| """ | ||
| for i, token in enumerate(tokens): | ||
| for i, token in enumerate(tokens[:-1]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7b2ebb4. Reverted to enumerate(tokens).
src/django_upgrade/tokens.py
Outdated
| # Frequently used token name sets | ||
| NL_NEWLINE = frozenset({"NL", "NEWLINE"}) | ||
| INDENT_TYPES = frozenset({"INDENT", UNIMPORTANT_WS}) | ||
| DEDENT_NL_NEWLINE = frozenset({"DEDENT", "NL", "NEWLINE"}) | ||
| NEWLINE_ENDMARKER = frozenset({"NEWLINE", "ENDMARKER"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undo all these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7b2ebb4. Removed all frozenset constants and reverted to inline tuples/sets.
src/django_upgrade/tokens.py
Outdated
| DEDENT_NL_NEWLINE = frozenset({"DEDENT", "NL", "NEWLINE"}) | ||
| NEWLINE_ENDMARKER = frozenset({"NEWLINE", "ENDMARKER"}) | ||
|
|
||
| # Compiled regex patterns for performance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete comment
src/django_upgrade/tokens.py
Outdated
| NEWLINE_ENDMARKER = frozenset({"NEWLINE", "ENDMARKER"}) | ||
|
|
||
| # Compiled regex patterns for performance | ||
| QUOTE_RE = re.compile(r'[\'"]') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move next to relevant function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 7b2ebb4. Reverted to inline re.search(r'[\'"]', match_quotes) within the function.
- Remove regex pre-compilation in django_urls.py (not on hot path) - Remove DJANGO_MODULES_TO_TRACK frozenset (inline tuple is faster) - Remove tokens[:-1] slice optimization in fixup_dedent_tokens - Remove token name frozensets, use inline tuples/sets instead - Revert QUOTE_RE to inline re.search call Co-authored-by: adamchainz <[email protected]>
for more information, see https://pre-commit.ci
Applied micro-optimizations to hot paths in the codebase to reduce overhead in membership testing by converting larger constant sets to frozensets.
Changes
SPECIAL_HEADERSin request_headers.pyRELATION_FIELD_NAMESin on_delete.pyMESSAGE_MODULE_NAMESin mail_api_kwargs.pyDJANGO_CHOICES_TYPESin model_field_choices.pyMODULESin postgres_float_range_field.pyAll changes maintain backward compatibility. 978 tests pass.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.