| Age | Commit message (Collapse) | Author |
|---|
|
For unformatted strings, it comes up periodically that there are
more allocations using fmt.Errorf("x") compared to errors.New("x").
People cite it as a reason to switch code using fmt.Errorf to
use errors.New instead.
Three examples from the last few weeks essentially made
this suggestion: #75235, CL 708496, and CL 708618. Prior to that,
it is periodically suggested as a vet check (e.g., proposals #17173
and #52696) or in various CLs to change the standard library
(e.g., CL 403938 and CL 588776).
On the other hand, I believe the position of the core Go team
is that it is usually not worthwhile to make such a change. For example,
in #52696, Russ wrote:
Thanks for raising the issue, but please don't do this. Using
fmt.Errorf("foo") is completely fine, especially in a program where
all the errors are constructed with fmt.Errorf. Having to
mentally switch between two functions based on the argument
is unnecessary noise.
This CL attempts to mostly take performance out of the discussion.
We drop from 2 allocations to 0 allocations for a non-escaping error,
and drop from 2 allocations to 1 allocation for an escaping error:
_ = fmt.Errorf("foo") // non-escaping error
sink = fmt.Errorf("foo") // escaping error
This now matches the allocations for errors.New("foo") in both cases.
The CPU cost difference is greatly reduced, though there is still
a small ~4ns difference measurable in these microbenchmarks. Previously,
it was ~64ns vs. ~21ns for fmt.Errorf("x") vs. errors.New("x")
for escaping errors, whereas with this CL it is now ~25ns vs. ~21ns.
When fmt.Errorf("foo") executes with this CL, there are essentially
three optimizations now, in rough order of usefulness:
(1) we always avoid an allocation inside the doPrintf machinery;
(2) if the error does not otherwise escape, we can stack allocate
the errors.errorString struct by virtue of mid-stack inlining
of fmt.Errorf and the resulting inlining of errors.New, which
also can be more effective via PGO;
(3) stringslite.IndexByte is a tiny bit faster than going through the
for loops looking for '%' inside doPrintf.
See https://blog.filippo.io/efficient-go-apis-with-the-inliner/ for
background on avoiding heap allocations via mid-stack inlining.
The common case here is likely that the string format argument is a
constant when there are no other arguments.
However, one concern could be that by not allocating a copy, we could
now keep a string argument alive longer with this change, which could
be a pessimization if for example that string argument is a
slice of a much bigger string:
s := bigString[m:n]
longLivedErr := fmt.Errorf(s)
Aside from that being perhaps unusual code, vet will complain about
s there as a "non-constant format string in call to fmt.Errorf", so that
particular example seems unlikely to occur frequently in practice.
The main benchmark results are below. "old" is prior to this CL, "new"
is with this CL. The non-escaping case is "local", the escaping case is
"sink". In practice, I suspect errors escape the majority of the time.
Benchmark code at https://go.dev/play/p/rlRSO1ehx8O
goos: linux
goarch: amd64
pkg: fmt
cpu: AMD EPYC 7B13
│ old-7bd6fac4.txt │ new-dcd2a72f0.txt │
│ sec/op │ sec/op vs base │
Errorf/no-args/local-16 63.76n ± 1% 4.874n ± 0% -92.36% (n=120)
Errorf/no-args/sink-16 64.25n ± 1% 25.81n ± 0% -59.83% (n=120)
Errorf/int-arg/local-16 90.86n ± 1% 90.97n ± 1% ~ (p=0.713 n=120)
Errorf/int-arg/sink-16 91.81n ± 1% 91.10n ± 1% -0.76% (p=0.036 n=120)
geomean 76.46n 31.95n -58.20%
│ old-7bd6fac4.txt │ new-dcd2a72f0.txt │
│ B/op │ B/op vs base │
Errorf/no-args/local-16 19.00 ± 0% 0.00 ± 0% -100.00% (n=120)
Errorf/no-args/sink-16 19.00 ± 0% 16.00 ± 0% -15.79% (n=120)
Errorf/int-arg/local-16 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=120) ¹
Errorf/int-arg/sink-16 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=120) ¹
geomean 21.35 ? ² ³
¹ all samples are equal
│ old-7bd6fac4.txt │ new-dcd2a72f0.txt │
│ allocs/op │ allocs/op vs base │
Errorf/no-args/local-16 2.000 ± 0% 0.000 ± 0% -100.00% (n=120)
Errorf/no-args/sink-16 2.000 ± 0% 1.000 ± 0% -50.00% (n=120)
Errorf/int-arg/local-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=120) ¹
Errorf/int-arg/sink-16 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=120) ¹
geomean 2.000 ? ² ³
¹ all samples are equal
Change-Id: Ib27c52933bec5c2236624c577fbb1741052e792f
Reviewed-on: https://go-review.googlesource.com/c/go/+/708836
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: t hepudds <thepudds1460@gmail.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
|
|
An error which implements an "Unwrap() []error" method wraps all the
non-nil errors in the returned []error.
We replace the concept of the "error chain" inspected by errors.Is
and errors.As with the "error tree". Is and As perform a pre-order,
depth-first traversal of an error's tree. As returns the first
matching result, if any.
The new errors.Join function returns an error wrapping a list of errors.
The fmt.Errorf function now supports multiple instances of the %w verb.
For #53435.
Change-Id: Ib7402e70b68e28af8f201d2b66bd8e87ccfb5283
Reviewed-on: https://go-review.googlesource.com/c/go/+/432898
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
|
|
As mentioned in https://golang.org/issue/34062#issuecomment-529692313
src/cmd refers to older version of golang.org/x/tools.
Hence, not checking if multiple errors are used in the same fmt.Errorf.
Updating golang.org/x/tools version to latest in src/cmd.
Fixes #34062
Change-Id: I358dec2c3d3af2b19add766b8488b919109b81d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/196843
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
|
|
When fmt.Errorf is provided with a %w verb with an error operand,
return an error implementing an Unwrap method returning that operand.
It is invalid to use %w with other formatting functions, to use %w
multiple times in a format string, or to use %w with a non-error
operand. When the Errorf format string contains an invalid use of %w,
the returned error does not implement Unwrap.
Change-Id: I534e20d3b163ab22c2b137b1c9095906dc243221
Reviewed-on: https://go-review.googlesource.com/c/go/+/176998
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
|
|
Reverts the following changes:
https://go.googlesource.com/go/+/1f90d081391d4f5911960fd28d81d7ea5e554a8f
https://go.googlesource.com/go/+/8bf18b56a47a98b9dd2fa03beb358312237a8c76
https://go.googlesource.com/go/+/5402854c3557f87fa2741a52ffc15dfb1ef333cc
https://go.googlesource.com/go/+/37f84817247d3b8e687a701ccb0d6bc7ffe3cb78
https://go.googlesource.com/go/+/6be6f114e0d483a233101a67c9644cd72bd3ae7a
Partially reverts the followinng change, removing the errors.Opaque
function and the errors.Wrapper type definition:
https://go.googlesource.com/go/+/62f5e8156ef56fa61e6af56f4ccc633bde1a9120
Updates documentation referencing the Wrapper type.
Change-Id: Ia622883e39cafb06809853e3fd90b21441124534
Reviewed-on: https://go-review.googlesource.com/c/go/+/176997
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
|
|
As proposed in Issue #29934, update errors produced by the os package to
work with errors.Is sentinel tests. For example,
errors.Is(err, os.ErrPermission) is equivalent to os.IsPermission(err)
with added unwrapping support.
Move the definition for os.ErrPermission and others into the syscall
package. Add an Is method to syscall.Errno and others. Add an Unwrap
method to os.PathError and others.
Updates #30322
Updates #29934
Change-Id: I95727d26c18a5354c720de316dff0bffc04dd926
Reviewed-on: https://go-review.googlesource.com/c/go/+/163058
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
|
|
The name change init -> init.ializers was initially required for
initialization code.
With CL 161337 there's no wrapper code any more, there's a data
structure instead (named .inittask). So we can go back to just
plain init appearing in tracebacks.
RELNOTE=yes
Update #29919. Followon to CL 161337.
Change-Id: I5a4a49d286df24b53b2baa193dfda482f3ea82a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/167780
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
This applies only for cases where %w is not used.
The purpose of this change is to reduce test failures where tests
depend on these two being the same type, as they previously were.
Change-Id: I2dd28b93fe1d59f3cfbb4eb0875d1fb8ee699746
Reviewed-on: https://go-review.googlesource.com/c/go/+/167402
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
|
|
This is a refinement of CL 164557.
Make it explicit in tests that using a non-string verb with
fmtError does not result in falling back to using fmt.Formatter.
Change-Id: I6d090f31818eb7cc7668d7565b1449c91cd03a23
Reviewed-on: https://go-review.googlesource.com/c/go/+/164701
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
|
|
When formatting an error with a non-string formatting verb such as %d,
use the default formatting behavior rather than treating this as a bad
verb.
For example, this should print 42, not %!d(main.E=42):
var E int
func (E) Error() string { return "error" }
fmt.Printf("%d", E(42))
Fixes #30472
Change-Id: I62fd309c8ee9839a69052b0ec7f1808449dcee8e
Reviewed-on: https://go-review.googlesource.com/c/164557
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Partly implements proposal Issue #29934.
Change-Id: Ibcf12f383158dcfbc313ab29c417a710571d1acb
Reviewed-on: https://go-review.googlesource.com/c/163559
Run-TryBot: Marcel van Lohuizen <mpvl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Damien Neil <dneil@google.com>
|
