-
Notifications
You must be signed in to change notification settings - Fork 611
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
pkg: move regexp compilation out of loops #2331
pkg: move regexp compilation out of loops #2331
Conversation
All these regexes are only used once, or very few times, so the time spent recompiling them is immaterial.1 On the other hand, you now introduce additional global variables with longer names in a wider scope, and the actual text of the regex is now further away in the source code from the location where is being used. Making the code even slightly harder to understand/maintain is not worth it to me to get a non-noticeable performance gain. I don't feel strongly about this particular instance, so I'll leave it to other maintainers to decide if they want to accept this change. Footnotes
|
a74edea
to
e558941
Compare
Moving it out of the loops sounds good (if loops are long), not sure about making it global (as mentioned above) |
What's the current status? |
@alexandear (Off-topic: what's your CNCF slack ID?) |
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.
As I wrote before, I'm not sure if creating global variables for regular expressions that are likely only compiled once per program run is an improvement, but at least the definition is still reasonably close to the location where it is used.
If other @lima-vm/maintainers want to merge, then I'm fine with it.
e558941
to
93a8a3a
Compare
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, LGTM
|
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
Signed-off-by: Oleksandr Redko <oleksandr.red+github@gmail.com>
93a8a3a
to
bfb90fa
Compare
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
This PR moves regexps out of
for
loops to recompile only once per function call.