From 2ca66e0194d2b9077c63a9d1eace8f1083675fa9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Leandro=20L=C3=B3pez?= <leandro.lopez@grafana.com>
Date: Fri, 27 Sep 2024 16:21:43 -0300
Subject: [PATCH] fix(perf): Reduce allocations (#39)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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

* feat(perf): Precompile getPattern regexps

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)

* feat(perf): Replace regexp replace with strings replace

These functions are faster than the regexp ones.
---
 codeowners.go      | 21 +++++++++++++--------
 codeowners_test.go |  4 +++-
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/codeowners.go b/codeowners.go
index c0a9f6f..7111d55 100644
--- a/codeowners.go
+++ b/codeowners.go
@@ -223,21 +223,26 @@ func (c *Codeowners) Owners(path string) []string {
 	return nil
 }
 
+// precompile all regular expressions
+var (
+	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 regexp.MustCompile(`^(\\#|\\!)`).MatchString(line) {
+	if strings.HasPrefix(line, `\#`) || strings.HasPrefix(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 = strings.ReplaceAll(line, ".", `\.`)
 
 	magicStar := "#$~"
 
@@ -245,13 +250,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 = strings.ReplaceAll(line, `/**/`, `(/|/.+/)`)
+	line = strings.ReplaceAll(line, `**/`, `(|.`+magicStar+`/)`)
+	line = strings.ReplaceAll(line, `/**`, `(|/.`+magicStar+`)`)
 
 	// Handle escaping the "*" char
-	line = regexp.MustCompile(`\\\*`).ReplaceAllString(line, `\`+magicStar)
-	line = regexp.MustCompile(`\*`).ReplaceAllString(line, `([^/]*)`)
+	line = strings.ReplaceAll(line, `\*`, `\`+magicStar)
+	line = strings.ReplaceAll(line, `*`, `([^/]*)`)
 
 	// Handle escaping the "?" char
 	line = strings.ReplaceAll(line, "?", `\?`)
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)
 	}