Skip to content

Commit

Permalink
fix(perf): Reduce allocations (#39)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
inkel authored and hairyhenderson committed Sep 27, 2024
1 parent d659b73 commit 2ca66e0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
21 changes: 13 additions & 8 deletions codeowners.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,35 +223,40 @@ 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 := "#$~"

// Handle "/**/" usage
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, "?", `\?`)
Expand Down
4 changes: 3 additions & 1 deletion codeowners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 2ca66e0

Please sign in to comment.