-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Do not compile regexes on each run #3949
Conversation
- Keep a cache of already compiled regexes (they will always be the same) - Alphanumeric strings an be simply matched for `==` instead of using RE.
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.
LGTM, particularly interesting regarding your insights into how re
caches the regexes.
except Exception: | ||
self.warning("Cannot compile regex: %s" % regex) | ||
return [] | ||
if key.isalnum(): |
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.
👌
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.
Thanks, the isalnum trick makes a big performance difference, if not a memory one. Some minor comments though.
309ab83
to
407d786
Compare
Codecov Report
@@ Coverage Diff @@
## master #3949 +/- ##
==========================================
+ Coverage 79.47% 88.88% +9.41%
==========================================
Files 158 7 -151
Lines 8250 270 -7980
Branches 1007 43 -964
==========================================
- Hits 6557 240 -6317
+ Misses 1460 19 -1441
+ Partials 233 11 -222 |
* Do not compile regexes on each run - Keep a cache of already compiled regexes (they will always be the same) - Alphanumeric strings an be simply matched for `==` instead of using RE. * Deduplicate loop as per CR
==
instead of using RE.