From 992d01060c858734dd49c9b47a5937309a7f3a47 Mon Sep 17 00:00:00 2001 From: myukitty Date: Sun, 16 Aug 2026 05:33:57 +0900 Subject: [PATCH] perf: compile regex patterns once instead of per-call in hot paths Signed-off-by: myukitty --- internal/digest/digest.go | 11 ++- internal/digest/digest_test.go | 8 ++ internal/filters/resourceFilter.go | 72 ++++++++++++--- internal/filters/resourceFilter_test.go | 115 ++++++++++++++++++++++++ internal/gitview/gitView.go | 10 ++- internal/gitview/gitView_test.go | 25 ++++++ internal/jira/jira.go | 30 +++++-- internal/jira/jira_test.go | 24 +++++ 8 files changed, 273 insertions(+), 22 deletions(-) diff --git a/internal/digest/digest.go b/internal/digest/digest.go index d065ea4f6..469ece5f4 100644 --- a/internal/digest/digest.go +++ b/internal/digest/digest.go @@ -343,14 +343,13 @@ func RemoteDockerImageSha256(client *requests.Client, imageName, imageTag, regis return strings.TrimPrefix(digestHeader, "sha256:"), nil } +const validSha256regex = "^([a-f0-9]{64})$" + +var validSha256CompiledRegex = regexp.MustCompile(validSha256regex) + // ValidateDigest checks if a digest matches the sha256 regex func ValidateDigest(sha256ToCheck string) error { - validSha256regex := "^([a-f0-9]{64})$" - r, err := regexp.Compile(validSha256regex) - if err != nil { - return fmt.Errorf("failed to validate the provided SHA256 fingerprint") - } - if !r.MatchString(sha256ToCheck) { + if !validSha256CompiledRegex.MatchString(sha256ToCheck) { return fmt.Errorf("%s is not a valid SHA256 fingerprint. It should match the pattern %v", sha256ToCheck, validSha256regex) } return nil diff --git a/internal/digest/digest_test.go b/internal/digest/digest_test.go index fcb471b6c..7f07ff913 100644 --- a/internal/digest/digest_test.go +++ b/internal/digest/digest_test.go @@ -973,3 +973,11 @@ func (suite *DigestTestSuite) TestGetExcludePathsFromIgnoreFile() { func TestDigestTestSuite(t *testing.T) { suite.Run(t, new(DigestTestSuite)) } + +func BenchmarkValidateDigest(b *testing.B) { + sha := "db40d79b3a15b17ee9fcc2f49aa73736e0073de6b5a35c459268bb9a31e55139" + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = ValidateDigest(sha) + } +} diff --git a/internal/filters/resourceFilter.go b/internal/filters/resourceFilter.go index 39668fbd5..a40e2d326 100644 --- a/internal/filters/resourceFilter.go +++ b/internal/filters/resourceFilter.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "slices" + "sync" ) type ResourceFilterOptions struct { @@ -11,6 +12,14 @@ type ResourceFilterOptions struct { IncludeNamesRegex []string ExcludeNames []string ExcludeNamesRegex []string + + excludeOnce sync.Once + excludeErr error + compiledExclude []*regexp.Regexp + + includeOnce sync.Once + includeErr error + compiledInclude []*regexp.Regexp } // IsSet checks if the filter options are set @@ -18,6 +27,49 @@ func (filter *ResourceFilterOptions) IsSet() bool { return len(filter.IncludeNames) > 0 || len(filter.IncludeNamesRegex) > 0 || len(filter.ExcludeNames) > 0 || len(filter.ExcludeNamesRegex) > 0 } +// CompilePatterns pre-compiles all include and exclude regex patterns +func (filter *ResourceFilterOptions) CompilePatterns() error { + if _, err := filter.compileExcludeRegexes(); err != nil { + return err + } + if _, err := filter.compileIncludeRegexes(); err != nil { + return err + } + return nil +} + +func (filter *ResourceFilterOptions) compileExcludeRegexes() ([]*regexp.Regexp, error) { + filter.excludeOnce.Do(func() { + compiled := make([]*regexp.Regexp, 0, len(filter.ExcludeNamesRegex)) + for _, pattern := range filter.ExcludeNamesRegex { + re, err := regexp.Compile(pattern) + if err != nil { + filter.excludeErr = fmt.Errorf("invalid exclude name regex pattern %s: %v", pattern, err) + return + } + compiled = append(compiled, re) + } + filter.compiledExclude = compiled + }) + return filter.compiledExclude, filter.excludeErr +} + +func (filter *ResourceFilterOptions) compileIncludeRegexes() ([]*regexp.Regexp, error) { + filter.includeOnce.Do(func() { + compiled := make([]*regexp.Regexp, 0, len(filter.IncludeNamesRegex)) + for _, pattern := range filter.IncludeNamesRegex { + re, err := regexp.Compile(pattern) + if err != nil { + filter.includeErr = fmt.Errorf("invalid include name regex pattern %s: %v", pattern, err) + return + } + compiled = append(compiled, re) + } + filter.compiledInclude = compiled + }) + return filter.compiledInclude, filter.includeErr +} + // ShouldInclude checks if a name should be included or not according to the filter options // the filter should only be used for one operation (include, exclude) func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { @@ -25,11 +77,11 @@ func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { if slices.Contains(filter.ExcludeNames, name) { return false, nil } - for _, pattern := range filter.ExcludeNamesRegex { - re, err := regexp.Compile(pattern) - if err != nil { - return false, fmt.Errorf("invalid exclude name regex pattern %s: %v", pattern, err) - } + excludeRegexes, err := filter.compileExcludeRegexes() + if err != nil { + return false, err + } + for _, re := range excludeRegexes { if re.MatchString(name) { return false, nil } @@ -41,11 +93,11 @@ func (filter *ResourceFilterOptions) ShouldInclude(name string) (bool, error) { return true, nil } - for _, pattern := range filter.IncludeNamesRegex { - re, err := regexp.Compile(pattern) - if err != nil { - return false, fmt.Errorf("invalid include name regex pattern %s: %v", pattern, err) - } + includeRegexes, err := filter.compileIncludeRegexes() + if err != nil { + return false, err + } + for _, re := range includeRegexes { if re.MatchString(name) { return true, nil } diff --git a/internal/filters/resourceFilter_test.go b/internal/filters/resourceFilter_test.go index f11ad5362..e4b921660 100644 --- a/internal/filters/resourceFilter_test.go +++ b/internal/filters/resourceFilter_test.go @@ -1,6 +1,8 @@ package filters import ( + "fmt" + "sync" "testing" "github.com/stretchr/testify/require" @@ -112,8 +114,121 @@ func (suite *FiltersSuite) TestShouldInclude() { } } +func (suite *FiltersSuite) TestCompilePatterns() { + validFilter := &ResourceFilterOptions{ + ExcludeNamesRegex: []string{"^baz.*$", "^qux.*$"}, + } + err := validFilter.CompilePatterns() + require.NoError(suite.T(), err) + require.Len(suite.T(), validFilter.compiledExclude, 2) + + // Subsequent ShouldInclude calls reuse compiled regexes + inc, err := validFilter.ShouldInclude("baz123") + require.NoError(suite.T(), err) + require.False(suite.T(), inc) + + inc, err = validFilter.ShouldInclude("qux456") + require.NoError(suite.T(), err) + require.False(suite.T(), inc) + + inc, err = validFilter.ShouldInclude("other") + require.NoError(suite.T(), err) + require.True(suite.T(), inc) + + includeFilter := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"^foo.*$", "^bar.*$"}, + } + err = includeFilter.CompilePatterns() + require.NoError(suite.T(), err) + require.Len(suite.T(), includeFilter.compiledInclude, 2) + + inc, err = includeFilter.ShouldInclude("foo123") + require.NoError(suite.T(), err) + require.True(suite.T(), inc) + + inc, err = includeFilter.ShouldInclude("bar456") + require.NoError(suite.T(), err) + require.True(suite.T(), inc) + + inc, err = includeFilter.ShouldInclude("other") + require.NoError(suite.T(), err) + require.False(suite.T(), inc) + + invalidInclude := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"[invalid"}, + } + require.Error(suite.T(), invalidInclude.CompilePatterns()) + + invalidExclude := &ResourceFilterOptions{ + ExcludeNamesRegex: []string{"[invalid"}, + } + require.Error(suite.T(), invalidExclude.CompilePatterns()) +} + +func (suite *FiltersSuite) TestConcurrentShouldInclude() { + filter := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"^include-.*$"}, + } + + type result struct { + included bool + err error + } + + const workerCount = 20 + results := make([]result, workerCount) + var wg sync.WaitGroup + + for i := 0; i < workerCount; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + inc, err := filter.ShouldInclude(fmt.Sprintf("include-%d", idx)) + results[idx] = result{included: inc, err: err} + }(i) + } + wg.Wait() + + for _, res := range results { + require.NoError(suite.T(), res.err) + require.True(suite.T(), res.included) + } + + // Concurrent exclude test + excludeFilter := &ResourceFilterOptions{ + ExcludeNamesRegex: []string{"^exclude-.*$"}, + } + excludeResults := make([]result, workerCount) + for i := 0; i < workerCount; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + inc, err := excludeFilter.ShouldInclude(fmt.Sprintf("exclude-%d", idx)) + excludeResults[idx] = result{included: inc, err: err} + }(i) + } + wg.Wait() + + for _, res := range excludeResults { + require.NoError(suite.T(), res.err) + require.False(suite.T(), res.included) + } +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestFiltersSuite(t *testing.T) { suite.Run(t, new(FiltersSuite)) } + +func BenchmarkShouldInclude_Regex(b *testing.B) { + filter := &ResourceFilterOptions{ + IncludeNamesRegex: []string{"^prod-.*$", "^staging-.*$"}, + } + _ = filter.CompilePatterns() + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = filter.ShouldInclude("prod-namespace-app") + } +} diff --git a/internal/gitview/gitView.go b/internal/gitview/gitView.go index e9365817e..e6c95a2cf 100644 --- a/internal/gitview/gitView.go +++ b/internal/gitview/gitView.go @@ -282,12 +282,20 @@ func getCommitURL(repoURL, commitHash string) string { // matches lookup happens in the commit message first, and if none is found, matching against the branch name is done // if no matches are found in both the commit message and the branch name, an empty slice is returned func (gv *GitView) MatchPatternInCommitMessageORBranchName(pattern, commitSHA, secondarySource string, ignoreBranchMatch bool) ([]string, *CommitInfo, error) { + re, err := regexp.Compile(pattern) + if err != nil { + return []string{}, nil, fmt.Errorf("invalid pattern regex %s: %w", pattern, err) + } + return gv.MatchRegexpInCommitMessageORBranchName(re, commitSHA, secondarySource, ignoreBranchMatch) +} + +// MatchRegexpInCommitMessageORBranchName returns a slice of strings matching a pre-compiled regular expression in a commit message or branch name +func (gv *GitView) MatchRegexpInCommitMessageORBranchName(re *regexp.Regexp, commitSHA, secondarySource string, ignoreBranchMatch bool) ([]string, *CommitInfo, error) { commitInfo, err := gv.GetCommitInfoFromCommitSHA(commitSHA, true, []string{}) if err != nil { return []string{}, nil, err } - re := regexp.MustCompile(pattern) commitMatches := re.FindAllString(commitInfo.Message, -1) branchMatches := re.FindAllString(commitInfo.Branch, -1) secondaryMatches := re.FindAllString(secondarySource, -1) diff --git a/internal/gitview/gitView_test.go b/internal/gitview/gitView_test.go index 2ff79040f..3227fdf88 100644 --- a/internal/gitview/gitView_test.go +++ b/internal/gitview/gitView_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "testing" "github.com/kosli-dev/cli/internal/jira" @@ -513,6 +514,13 @@ func (suite *GitViewTestSuite) TestMatchPatternInCommitMessageORBranchName() { want: []string{"#324"}, wantError: false, }, + { + name: "Invalid regex pattern returns an error", + pattern: "[invalid", + commitMessage: "test commit", + want: []string{}, + wantError: true, + }, } { suite.Run(t.name, func() { @@ -538,6 +546,23 @@ func (suite *GitViewTestSuite) TestMatchPatternInCommitMessageORBranchName() { } } +func (suite *GitViewTestSuite) TestMatchRegexpInCommitMessageORBranchName() { + _, workTree, fs, err := testHelpers.InitializeGitRepo(suite.tmpDir) + require.NoError(suite.T(), err) + + commitSha, err := testHelpers.CommitToRepo(workTree, fs, "Resolves JIRA-100 and JIRA-200") + require.NoError(suite.T(), err) + + gitView, err := New(suite.tmpDir) + require.NoError(suite.T(), err) + + re := regexp.MustCompile(`JIRA-[0-9]+`) + matches, commitInfo, err := gitView.MatchRegexpInCommitMessageORBranchName(re, commitSha, "", false) + require.NoError(suite.T(), err) + require.NotNil(suite.T(), commitInfo) + require.ElementsMatch(suite.T(), []string{"JIRA-100", "JIRA-200"}, matches) +} + func (suite *GitViewTestSuite) TestResolveRevision() { _, workTree, fs, err := testHelpers.InitializeGitRepo(suite.tmpDir) require.NoError(suite.T(), err) diff --git a/internal/jira/jira.go b/internal/jira/jira.go index 2ba96445f..a5c78cf07 100644 --- a/internal/jira/jira.go +++ b/internal/jira/jira.go @@ -7,6 +7,7 @@ import ( "regexp" "sort" "strings" + "sync" jira "github.com/andygrunwald/go-jira" ) @@ -107,6 +108,27 @@ func (jc *JiraConfig) GetJiraIssueInfo(issueID string, issueFields string) (*Jir return result, nil } +const defaultJiraIssueKeyPattern = `\b[A-Z][A-Z0-9]{1,9}-[0-9]+` + +var ( + defaultJiraKeyRegex = regexp.MustCompile(defaultJiraIssueKeyPattern) + dashDigitRegex = regexp.MustCompile(`^-\d`) + jiraKeyRegexCache sync.Map +) + +func getJiraKeyRegex(projectKeys []string) *regexp.Regexp { + if len(projectKeys) == 0 { + return defaultJiraKeyRegex + } + pattern := MakeJiraIssueKeyPattern(projectKeys) + if val, ok := jiraKeyRegexCache.Load(pattern); ok { + return val.(*regexp.Regexp) + } + re := regexp.MustCompile(pattern) + jiraKeyRegexCache.Store(pattern, re) + return re +} + func MakeJiraIssueKeyPattern(projectKeys []string) string { // Jira issue keys consist of [project-key]-[sequential-number]. // FindJiraIssueKeys uppercases the text before applying this pattern, so the @@ -114,7 +136,7 @@ func MakeJiraIssueKeyPattern(projectKeys []string) string { // are also uppercased here for the same reason. // more info: https://support.atlassian.com/jira-software-cloud/docs/what-is-an-issue/#Workingwithissues-Projectandissuekeys if len(projectKeys) == 0 { - return `\b[A-Z][A-Z0-9]{1,9}-[0-9]+` + return defaultJiraIssueKeyPattern } upper := make([]string, len(projectKeys)) for i, k := range projectKeys { @@ -131,8 +153,7 @@ func MakeJiraIssueKeyPattern(projectKeys []string) string { // immediately followed by a hyphen and a digit. func FindJiraIssueKeys(text string, projectKeys []string) []string { upperText := strings.ToUpper(text) - pattern := MakeJiraIssueKeyPattern(projectKeys) - re := regexp.MustCompile(pattern) + re := getJiraKeyRegex(projectKeys) candidates := re.FindAllString(upperText, -1) // Deduplicate (all candidates are already uppercase). @@ -146,10 +167,9 @@ func FindJiraIssueKeys(text string, projectKeys []string) []string { } // Filter out matches that are always followed by - in the uppercased text. - dashDigit := regexp.MustCompile(`^-\d`) var result []string for _, m := range unique { - if isPartialMultiSegment(upperText, m, dashDigit) { + if isPartialMultiSegment(upperText, m, dashDigitRegex) { continue } result = append(result, m) diff --git a/internal/jira/jira_test.go b/internal/jira/jira_test.go index 84acefa48..91007a8fc 100644 --- a/internal/jira/jira_test.go +++ b/internal/jira/jira_test.go @@ -2,6 +2,7 @@ package jira import ( "regexp" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -223,3 +224,26 @@ func TestFindJiraIssueKeys(t *testing.T) { }) } } + +func TestFindJiraIssueKeys_ConcurrentCache(t *testing.T) { + const workerCount = 20 + var wg sync.WaitGroup + for i := 0; i < workerCount; i++ { + wg.Add(1) + go func(idx int) { + defer wg.Done() + keys := FindJiraIssueKeys("fixes PROJ-42 and OTHER-99", []string{"PROJ", "OTHER"}) + assert.ElementsMatch(t, []string{"PROJ-42", "OTHER-99"}, keys) + }(i) + } + wg.Wait() +} + +func BenchmarkFindJiraIssueKeys(b *testing.B) { + text := "fixes PROJ-42 and OTHER-99 in commit message" + projectKeys := []string{"PROJ", "OTHER"} + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = FindJiraIssueKeys(text, projectKeys) + } +}