Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Combine for all nil scenarios #55

Merged
merged 9 commits into from Feb 28, 2022
Merged
65 changes: 65 additions & 0 deletions benchmarks_test.go
Expand Up @@ -60,3 +60,68 @@ func BenchmarkAppend(b *testing.B) {
}
}
}

func BenchmarkCombine(b *testing.B) {
b.Run("inline 1", func(b *testing.B) {
var x error
for i := 0; i < b.N; i++ {
Combine(x)
}
})

b.Run("inline 2", func(b *testing.B) {
var x, y error
for i := 0; i < b.N; i++ {
Combine(x, y)
}
})

b.Run("inline 3 no error", func(b *testing.B) {
var x, y, z error
for i := 0; i < b.N; i++ {
Combine(x, y, z)
}
})

b.Run("inline 3 one error", func(b *testing.B) {
var x, y, z error
z = fmt.Errorf("failed")
for i := 0; i < b.N; i++ {
Combine(x, y, z)
}
})

b.Run("inline 3 multiple errors", func(b *testing.B) {
var x, y, z error
z = fmt.Errorf("failed3")
y = fmt.Errorf("failed2")
x = fmt.Errorf("failed")
for i := 0; i < b.N; i++ {
Combine(x, y, z)
}
})

b.Run("slice 100 no errors", func(b *testing.B) {
errs := make([]error, 100)
for i := 0; i < b.N; i++ {
Combine(errs...)
}
})

b.Run("slice 100 one error", func(b *testing.B) {
errs := make([]error, 100)
errs[len(errs)-1] = fmt.Errorf("failed")
for i := 0; i < b.N; i++ {
Combine(errs...)
}
})

b.Run("slice 100 multi error", func(b *testing.B) {
errs := make([]error, 100)
errs[0] = fmt.Errorf("failed1")
errs[len(errs)-1] = fmt.Errorf("failed2")
for i := 0; i < b.N; i++ {
Combine(errs...)
}
})
}
17 changes: 15 additions & 2 deletions error.go
Expand Up @@ -372,6 +372,14 @@ func inspect(errors []error) (res inspectResult) {

// fromSlice converts the given list of errors into a single error.
func fromSlice(errors []error) error {
// Don't pay to inspect small slices.
switch len(errors) {
case 0:
return nil
case 1:
return errors[0]
}

res := inspect(errors)
switch res.Count {
case 0:
Expand All @@ -381,8 +389,13 @@ func fromSlice(errors []error) error {
return errors[res.FirstErrorIdx]
case len(errors):
if !res.ContainsMultiError {
// already flat
return &multiError{errors: errors}
// Error list is flat. Make a copy of it
// Otherwise "errors" escapes to the heap
// unconditionally for all other cases.
// This lets us optimize for the "no errors" case.
out := make([]error, len(errors))
copy(out, errors)
return &multiError{errors: out}
}
}

Expand Down
14 changes: 14 additions & 0 deletions error_test.go
Expand Up @@ -97,6 +97,12 @@ func TestCombine(t *testing.T) {
" - bar",
wantSingleline: "foo; bar",
},
{
giveErrors: []error{nil, nil, errors.New("great sadness"), nil},
wantError: errors.New("great sadness"),
wantMultiline: "great sadness",
wantSingleline: "great sadness",
},
{
giveErrors: []error{
errors.New("foo"),
Expand Down Expand Up @@ -273,6 +279,14 @@ func TestCombineDoesNotModifySlice(t *testing.T) {
assert.Nil(t, errors[1], 3)
}

func TestCombineGoodCaseNoAlloc(t *testing.T) {
errs := make([]error, 10)
allocs := testing.AllocsPerRun(100, func() {
Combine(errs...)
})
assert.Equal(t, 0.0, allocs)
}

func TestAppend(t *testing.T) {
tests := []struct {
left error
Expand Down