Skip to content

Commit

Permalink
Combine: Optimize for all nil (#55)
Browse files Browse the repository at this point in the history
The current implementation of Combine returns the input slice in the
returned multierr. This causes it to escape to the heap unconditionally.
Optimizing for the no-errors case, we can copy the slice in that case
and keep it on the stack.

```
name                                old time/op    new time/op    delta
Combine/inline_1-8                    17.7ns ± 0%     2.1ns ± 0%   -88.20%  (p=0.008 n=5+5)
Combine/inline_2-8                    21.0ns ± 0%     4.4ns ± 1%   -79.03%  (p=0.008 n=5+5)
Combine/inline_3_no_error-8           24.4ns ± 0%     5.1ns ± 1%   -79.22%  (p=0.016 n=4+5)
Combine/inline_3_one_error-8          24.8ns ± 0%     5.4ns ± 1%   -78.42%  (p=0.008 n=5+5)
Combine/inline_3_multiple_errors-8    44.3ns ± 0%    54.9ns ± 0%   +23.80%  (p=0.008 n=5+5)
Combine/slice_100_no_errors-8         72.9ns ± 0%    73.4ns ± 1%    +0.68%  (p=0.008 n=5+5)
Combine/slice_100_one_error-8         74.5ns ± 0%    74.8ns ± 0%      ~     (p=0.056 n=5+5)
Combine/slice_100_multi_error-8        193ns ± 0%     193ns ± 0%      ~     (p=0.127 n=5+5)

name                                old alloc/op   new alloc/op   delta
Combine/inline_1-8                     16.0B ± 0%      0.0B       -100.00%  (p=0.008 n=5+5)
Combine/inline_2-8                     32.0B ± 0%      0.0B       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_no_error-8            48.0B ± 0%      0.0B       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_one_error-8           48.0B ± 0%      0.0B       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_multiple_errors-8     80.0B ± 0%     80.0B ± 0%      ~     (all equal)
Combine/slice_100_no_errors-8          0.00B          0.00B           ~     (all equal)
Combine/slice_100_one_error-8          0.00B          0.00B           ~     (all equal)
Combine/slice_100_multi_error-8        64.0B ± 0%     64.0B ± 0%      ~     (all equal)

name                                old allocs/op  new allocs/op  delta
Combine/inline_1-8                      1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Combine/inline_2-8                      1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_no_error-8             1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_one_error-8            1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
Combine/inline_3_multiple_errors-8      2.00 ± 0%      2.00 ± 0%      ~     (all equal)
Combine/slice_100_no_errors-8           0.00           0.00           ~     (all equal)
Combine/slice_100_one_error-8           0.00           0.00           ~     (all equal)
Combine/slice_100_multi_error-8         2.00 ± 0%      2.00 ± 0%      ~     (all equal)
```

Co-authored-by: Abhinav Gupta <abg@uber.com>
  • Loading branch information
jstroem and abhinav committed Feb 28, 2022
1 parent d49c2ba commit cea7d2e
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 2 deletions.
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

0 comments on commit cea7d2e

Please sign in to comment.