Skip to content

perf: compile regex patterns once instead of per-call in hot paths - #1098

Closed
myukitty wants to merge 1 commit into
kosli-dev:mainfrom
myukitty:perf/compile-regex-once
Closed

perf: compile regex patterns once instead of per-call in hot paths#1098
myukitty wants to merge 1 commit into
kosli-dev:mainfrom
myukitty:perf/compile-regex-once

Conversation

@myukitty

@myukittymyukitty commented Aug 15, 2026

Copy link
Copy Markdown

When processing filters, Git commits, and Jira issue keys in loops or hot paths, compiling regular expressions on every iteration introduced unnecessary CPU and memory allocation overhead.

This PR:

  1. ResourceFilterOptions: Lazily compiles and caches include/exclude regexes using sync.Once, making concurrent ShouldInclude calls thread-safe and fast.
  2. GitView: Adds MatchRegexpInCommitMessageORBranchName(re *regexp.Regexp, ...) accepting a precompiled regex while maintaining backward-compatible MatchPatternInCommitMessageORBranchName(pattern, ...).
  3. Jira: Pre-compiles and caches Jira key patterns in a thread-safe sync.Map (getJiraKeyRegex) so both default and custom project key patterns avoid per-call compilation in FindJiraIssueKeys.
  4. Adds unit and concurrent tests in internal/filters/resourceFilter_test.go, internal/jira/jira_test.go, and internal/gitview/gitView_test.go.

Fixes#826.

Signed-off-by: myukitty <myukittyy@gmail.com>
@myukitty
myukittyforce-pushed the perf/compile-regex-once branch from d65036d to 992d010CompareAugust 17, 2026 17:45
@mbevc1

mbevc1 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Hi @myukitty and thanks for your contribution. Checking this PR and comparing to the issue intent it has some shortcomings, which we'll address holistically with a fresh PR: #1114

Summary:

5c ValidateDigest ✅ Correct, and thoughtfully done
5a ShouldInclude filters ⚠️ Fixed, but changes behavior
5b MatchPatternInCommitMessageORBranchName ❌ Not actually fixed - hot path untouched

@mbevc1mbevc1 closed this Aug 20, 2026
@myukitty

Copy link
Copy Markdown
Author

Thanks for writing that up rather than just closing it, @mbevc1 — the assessment is fair on both counts.

On 5b you're right, and it's the more useful correction: I added MatchRegexpInCommitMessageORBranchName as a pre-compiled variant but left every caller on the pattern-compiling wrapper, so nothing on the hot path actually got faster. The PR title claimed a result the diff did not deliver.

On 5a, hoisting the compile out of the per-name loop also moves when an invalid pattern surfaces. That's a behaviour change, and it belonged in the description as one instead of riding along inside a perf change — #1114's note on preserving the original error timing is exactly the part I skipped.

Handling it holistically in #1114 makes sense. Thanks for the review.

Sign up for freeto 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.

perf: compile regex patterns once instead of per-call in 3 hot paths

2 participants

@myukitty@mbevc1