Skip to content

move regexp.MustCompile close to call #3280

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

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

alingse
Copy link
Contributor

@alingse alingse commented Feb 18, 2025

I'm validating an idea for my linter. I checked the top 2500 projects on github and found this.

All project result can be found here -> alingse/sundrylint#7

I think move the regexp.MustCompile out has two advantages

  1. Performance benefits
  2. Fail early with faulty regexp string

@ndyakov
Copy link
Member

ndyakov commented Feb 19, 2025

Hello @alingse thank you for the contribution. You are right that this may increase performance (if info modules is used often) and also will fail early if the regex is incorrect. At the same time time it introduces a global variable that can be changed from anywhere in the package. Since this method is not expected to be called that often I do not see the benefit outweighing the possible issues this may introduce. What I can suggest is to compile the regex only when used (i.e. if the section is modules) which will speed things a little.

@ndyakov ndyakov self-requested a review February 19, 2025 08:36
@alingse alingse changed the title move regexp.MustCompile out of func move regexp.MustCompile close to call Feb 19, 2025
@ndyakov ndyakov self-requested a review February 20, 2025 14:22
@ndyakov ndyakov merged commit f3c2711 into redis:master Feb 20, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants