From 350aab05e547fb5e0d5bbda8b4c906b23d0df14f Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Wed, 14 Dec 2022 18:17:38 -0800 Subject: [PATCH] util/multierr: optimize New for nil cases (#6750) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consider the following pattern: err1 := foo() err2 := bar() err3 := baz() return multierr.New(err1, err2, err3) If err1, err2, and err3 are all nil, then multierr.New should not allocate. Thus, modify the logic of New to count the number of distinct error values and allocate the exactly needed slice. This also speeds up non-empty error situation since repeatedly growing with append is slow. Performance: name old time/op new time/op delta Empty-24 41.8ns ± 2% 6.4ns ± 1% -84.73% (p=0.000 n=10+10) NonEmpty-24 120ns ± 3% 69ns ± 1% -42.01% (p=0.000 n=9+10) name old alloc/op new alloc/op delta Empty-24 64.0B ± 0% 0.0B -100.00% (p=0.000 n=10+10) NonEmpty-24 168B ± 0% 88B ± 0% -47.62% (p=0.000 n=10+10) name old allocs/op new allocs/op delta Empty-24 1.00 ± 0% 0.00 -100.00% (p=0.000 n=10+10) NonEmpty-24 3.00 ± 0% 2.00 ± 0% -33.33% (p=0.000 n=10+10) Signed-off-by: Joe Tsai --- util/multierr/multierr.go | 38 ++++++++++++++++++++++------------ util/multierr/multierr_test.go | 18 ++++++++++++++++ 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/util/multierr/multierr.go b/util/multierr/multierr.go index 83a5cb17f..b4ae0afe5 100644 --- a/util/multierr/multierr.go +++ b/util/multierr/multierr.go @@ -53,7 +53,31 @@ func (e Error) Unwrap() []error { // If the resulting slice has length 1, New returns that error. // If the resulting slice has length > 1, New returns that slice as an Error. func New(errs ...error) error { - dst := make([]error, 0, len(errs)) + // First count the number of errors to avoid allocating. + var n int + var errFirst error + for _, e := range errs { + switch e := e.(type) { + case nil: + continue + case Error: + n += len(e.errs) + if errFirst == nil && len(e.errs) > 0 { + errFirst = e.errs[0] + } + default: + n++ + if errFirst == nil { + errFirst = e + } + } + } + if n <= 1 { + return errFirst // nil if n == 0 + } + + // More than one error, allocate slice and construct the multi-error. + dst := make([]error, 0, n) for _, e := range errs { switch e := e.(type) { case nil: @@ -64,18 +88,6 @@ func New(errs ...error) error { dst = append(dst, e) } } - // dst has been filtered and splatted. - switch len(dst) { - case 0: - return nil - case 1: - return dst[0] - } - // Zero any remaining elements of dst left over from errs, for GC. - tail := dst[len(dst):cap(dst)] - for i := range tail { - tail[i] = nil - } return Error{errs: dst} } diff --git a/util/multierr/multierr_test.go b/util/multierr/multierr_test.go index f8cb729ba..28b4efdb9 100644 --- a/util/multierr/multierr_test.go +++ b/util/multierr/multierr_test.go @@ -7,6 +7,7 @@ package multierr_test import ( "errors" "fmt" + "io" "testing" qt "github.com/frankban/quicktest" @@ -106,3 +107,20 @@ func TestRange(t *testing.T) { return x.Error() == y.Error() })), want) } + +var sink error + +func BenchmarkEmpty(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + sink = multierr.New(nil, nil, nil, multierr.Error{}) + } +} + +func BenchmarkNonEmpty(b *testing.B) { + merr := multierr.New(io.ErrShortBuffer, io.ErrNoProgress) + b.ReportAllocs() + for i := 0; i < b.N; i++ { + sink = multierr.New(io.ErrUnexpectedEOF, merr, io.ErrClosedPipe) + } +}