From 3a36d2e66f6dfb771981ddbe08cc8cc6695ee737 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez=20=28inkel=29?= Date: Thu, 27 Jun 2024 09:40:40 -0300 Subject: [PATCH 1/3] fix(perf): Fix benchmark The reader was being read only once in the first iteration, and afterwards it was always empty, thus `parseCodeowners` was parsing an empty file. These were the metrics before the change: $ go test -benchmem -bench=. goos: darwin goarch: arm64 pkg: github.com/hairyhenderson/go-codeowners BenchmarkParseCodeowners-10 2677503 465.3 ns/op 4096 B/op 1 allocs/op PASS ok github.com/hairyhenderson/go-codeowners 2.255s After the change it's not showing more realistic data: $ go test -benchmem -bench=. goos: darwin goarch: arm64 pkg: github.com/hairyhenderson/go-codeowners BenchmarkParseCodeowners-10 31054 38910 ns/op 61424 B/op 641 allocs/op PASS ok github.com/hairyhenderson/go-codeowners 3.529s --- codeowners_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codeowners_test.go b/codeowners_test.go index 6eba1a0..e99843d 100644 --- a/codeowners_test.go +++ b/codeowners_test.go @@ -145,10 +145,12 @@ func TestParseCodeownersSections(t *testing.T) { } func BenchmarkParseCodeowners(b *testing.B) { - r := bytes.NewBufferString(sample) var c []Codeowner for n := 0; n < b.N; n++ { + b.StopTimer() + r := bytes.NewBufferString(sample) + b.StartTimer() c, _ = parseCodeowners(r) } From fa2c30d9dab184166f21ef94214761a886fe7bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez=20=28inkel=29?= Date: Thu, 27 Jun 2024 10:02:06 -0300 Subject: [PATCH 2/3] feat(perf): Precompile getPattern regexps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This greatly improves the allocations: $ go test -benchmem -bench=. goos: darwin goarch: arm64 pkg: github.com/hairyhenderson/go-codeowners BenchmarkParseCodeowners-10 70927 16857 ns/op 27211 B/op 242 allocs/op PASS ok github.com/hairyhenderson/go-codeowners 5.297s This reduces the allocations in 62%: $ benchstat main.log precompile.log goos: darwin goarch: arm64 pkg: github.com/hairyhenderson/go-codeowners │ main.log │ precompile.log │ │ sec/op │ sec/op vs base │ ParseCodeowners-10 38.72µ ± 2% 17.63µ ± 6% -54.46% (p=0.000 n=10) │ main.log │ precompile.log │ │ B/op │ B/op vs base │ ParseCodeowners-10 59.95Ki ± 0% 26.56Ki ± 0% -55.70% (p=0.000 n=10) │ main.log │ precompile.log │ │ allocs/op │ allocs/op vs base │ ParseCodeowners-10 641.0 ± 0% 242.0 ± 0% -62.25% (p=0.000 n=10) --- codeowners.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/codeowners.go b/codeowners.go index 3b383db..a0f8ff6 100644 --- a/codeowners.go +++ b/codeowners.go @@ -220,21 +220,33 @@ func (c *Codeowners) Owners(path string) []string { return nil } +// precompile all regular expressions +var ( + reCommentIgnore = regexp.MustCompile(`^(\\#|\\!)`) + rePrependSlash = regexp.MustCompile(`([^\/+])/.*\*\.`) + reEscapeDot = regexp.MustCompile(`\.`) + reDoubleStar1 = regexp.MustCompile(`/\*\*/`) + reDoubleStar2 = regexp.MustCompile(`\*\*/`) + reDoubleStar3 = regexp.MustCompile(`/\*\*`) + reEscapeStar1 = regexp.MustCompile(`\\\*`) + reEscapeStar2 = regexp.MustCompile(`\*`) +) + // based on github.com/sabhiram/go-gitignore // but modified so that 'dir/*' only matches files in 'dir/' func getPattern(line string) (*regexp.Regexp, error) { // when # or ! is escaped with a \ - if regexp.MustCompile(`^(\\#|\\!)`).MatchString(line) { + if reCommentIgnore.MatchString(line) { line = line[1:] } // If we encounter a foo/*.blah in a folder, prepend the / char - if regexp.MustCompile(`([^\/+])/.*\*\.`).MatchString(line) && line[0] != '/' { + if rePrependSlash.MatchString(line) && line[0] != '/' { line = "/" + line } // Handle escaping the "." char - line = regexp.MustCompile(`\.`).ReplaceAllString(line, `\.`) + line = reEscapeDot.ReplaceAllString(line, `\.`) magicStar := "#$~" @@ -242,13 +254,13 @@ func getPattern(line string) (*regexp.Regexp, error) { if strings.HasPrefix(line, "/**/") { line = line[1:] } - line = regexp.MustCompile(`/\*\*/`).ReplaceAllString(line, `(/|/.+/)`) - line = regexp.MustCompile(`\*\*/`).ReplaceAllString(line, `(|.`+magicStar+`/)`) - line = regexp.MustCompile(`/\*\*`).ReplaceAllString(line, `(|/.`+magicStar+`)`) + line = reDoubleStar1.ReplaceAllString(line, `(/|/.+/)`) + line = reDoubleStar2.ReplaceAllString(line, `(|.`+magicStar+`/)`) + line = reDoubleStar3.ReplaceAllString(line, `(|/.`+magicStar+`)`) // Handle escaping the "*" char - line = regexp.MustCompile(`\\\*`).ReplaceAllString(line, `\`+magicStar) - line = regexp.MustCompile(`\*`).ReplaceAllString(line, `([^/]*)`) + line = reEscapeStar1.ReplaceAllString(line, `\`+magicStar) + line = reEscapeStar2.ReplaceAllString(line, `([^/]*)`) // Handle escaping the "?" char line = strings.ReplaceAll(line, "?", `\?`) From 483c183b528641c76d0929f9f2ef309c6e131f10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Leandro=20L=C3=B3pez=20=28inkel=29?= Date: Thu, 27 Jun 2024 10:32:57 -0300 Subject: [PATCH 3/3] feat(perf): Replace regexp replace with strings replace These functions are faster than the regexp ones. --- codeowners.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/codeowners.go b/codeowners.go index a0f8ff6..7b27a7e 100644 --- a/codeowners.go +++ b/codeowners.go @@ -222,21 +222,14 @@ func (c *Codeowners) Owners(path string) []string { // precompile all regular expressions var ( - reCommentIgnore = regexp.MustCompile(`^(\\#|\\!)`) - rePrependSlash = regexp.MustCompile(`([^\/+])/.*\*\.`) - reEscapeDot = regexp.MustCompile(`\.`) - reDoubleStar1 = regexp.MustCompile(`/\*\*/`) - reDoubleStar2 = regexp.MustCompile(`\*\*/`) - reDoubleStar3 = regexp.MustCompile(`/\*\*`) - reEscapeStar1 = regexp.MustCompile(`\\\*`) - reEscapeStar2 = regexp.MustCompile(`\*`) + rePrependSlash = regexp.MustCompile(`([^\/+])/.*\*\.`) ) // based on github.com/sabhiram/go-gitignore // but modified so that 'dir/*' only matches files in 'dir/' func getPattern(line string) (*regexp.Regexp, error) { // when # or ! is escaped with a \ - if reCommentIgnore.MatchString(line) { + if strings.HasPrefix(line, `\#`) || strings.HasPrefix(line, `\!`) { line = line[1:] } @@ -246,7 +239,7 @@ func getPattern(line string) (*regexp.Regexp, error) { } // Handle escaping the "." char - line = reEscapeDot.ReplaceAllString(line, `\.`) + line = strings.ReplaceAll(line, ".", `\.`) magicStar := "#$~" @@ -254,13 +247,13 @@ func getPattern(line string) (*regexp.Regexp, error) { if strings.HasPrefix(line, "/**/") { line = line[1:] } - line = reDoubleStar1.ReplaceAllString(line, `(/|/.+/)`) - line = reDoubleStar2.ReplaceAllString(line, `(|.`+magicStar+`/)`) - line = reDoubleStar3.ReplaceAllString(line, `(|/.`+magicStar+`)`) + line = strings.ReplaceAll(line, `/**/`, `(/|/.+/)`) + line = strings.ReplaceAll(line, `**/`, `(|.`+magicStar+`/)`) + line = strings.ReplaceAll(line, `/**`, `(|/.`+magicStar+`)`) // Handle escaping the "*" char - line = reEscapeStar1.ReplaceAllString(line, `\`+magicStar) - line = reEscapeStar2.ReplaceAllString(line, `([^/]*)`) + line = strings.ReplaceAll(line, `\*`, `\`+magicStar) + line = strings.ReplaceAll(line, `*`, `([^/]*)`) // Handle escaping the "?" char line = strings.ReplaceAll(line, "?", `\?`)