Explorar o código

util/multierr: optimize New for nil cases (#6750)

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 <[email protected]>
Joe Tsai %!s(int64=3) %!d(string=hai) anos
pai
achega
350aab05e5
Modificáronse 2 ficheiros con 43 adicións e 13 borrados
  1. 25 13
      util/multierr/multierr.go
  2. 18 0
      util/multierr/multierr_test.go

+ 25 - 13
util/multierr/multierr.go

@@ -53,28 +53,40 @@ 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:
-			dst = append(dst, e.errs...)
+			n += len(e.errs)
+			if errFirst == nil && len(e.errs) > 0 {
+				errFirst = e.errs[0]
+			}
 		default:
-			dst = append(dst, e)
+			n++
+			if errFirst == nil {
+				errFirst = e
+			}
 		}
 	}
-	// dst has been filtered and splatted.
-	switch len(dst) {
-	case 0:
-		return nil
-	case 1:
-		return dst[0]
+	if n <= 1 {
+		return errFirst // nil if n == 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
+
+	// 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:
+			continue
+		case Error:
+			dst = append(dst, e.errs...)
+		default:
+			dst = append(dst, e)
+		}
 	}
 	return Error{errs: dst}
 }

+ 18 - 0
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)
+	}
+}