When one has a []byte on hand, but desires to call the Parse function,
the conversion from []byte to string would allocate.
This occurs frequently through UnmarshalText and UnmarshalJSON.
This changes it such that the input string never escapes from
any of the Parse functions. Together with the compiler optimization
where the compiler stack allocates any string smaller than 32B
this makes most valid inputs for Parse(layout, string(input))
not require an allocation for the input string.
This optimization works well for most RFC3339 timestamps.
All timestamps with second resolution
(e.g., 2000-01-01T00:00:00Z or 2000-01-01T00:00:00+23:59)
or timestamps with nanosecond resolution in UTC
(e.g., 2000-01-01T00:00:00.123456789Z)
are less than 32B and benefit from this optimization.
Unfortunately, nanosecond timestamps with non-UTC timezones
(e.g., 2000-01-01T00:00:00.123456789+23:59)
do not benefit since they are 35B long.
Previously, this was not possible since the input leaked
to the error and calls to FixedZone with the zone name,
which causes the prover to give up and heap copy the []byte.
We fix this by copying the input string in both cases.
The advantage of this change is that you can now call Parse
with a []byte without allocating (most of the times).
The detriment is that the timezone and error path has an extra allocation.
Handling of timezones were already expensive (3 allocations and 160B allocated),
so the additional cost of another string allocation is relatively minor.
We should optimize for the common case, rather than the exceptional case.
Performance:
name old time/op new time/op delta
ParseRFC3339UTCBytes 54.4ns ± 1% 40.3ns ± 1% -25.91% (p=0.000 n=9+10)
Now that parsing of RFC3339 has been heavily optimized in CL 425197,
the performance gains by this optimization becomes relatively more notable.
Related to CL 345488.
Change-Id: I2a8a9cd6354b3bd46c2f57818ed2646a2e485f36
Reviewed-on: https://go-review.googlesource.com/c/go/+/429862
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Say "//go:build" instead of "// +build" in the package level godoc
comment.
Change-Id: I4700227a03197ffbe29e4de04d068b4c63bb5bf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/431856
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Replace custom append functions in the hash functions with the implementation of the encoding/binary package that do the same thing.
The binary bigendian functions are already used in other parts of the code in the crypto package.
Change-Id: I76d2dbe143fc72a3b4ac06be312caf72bd71378a
GitHub-Last-Rev: 1c6c68279e
GitHub-Pull-Request: golang/go#55085
Reviewed-on: https://go-review.googlesource.com/c/go/+/431035
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For #45557
Change-Id: I56824135d86452603dd4ed4bab0e24c201bb0683
Reviewed-on: https://go-review.googlesource.com/c/go/+/426257
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Use "%w" instead of "%v" as format verb for error value in the NextPart
method. This way it will be possible to use common go error utilities
from std library when parsing from custom io.Readers.
This issue was discovered during attempts to use
http.Request.ParseMultipartForm together with http.MaxBytesHandler.
Change-Id: Idb82510fb536b66b51ed1d943737c4828f07c2f2
GitHub-Last-Rev: 8bc49c945c
GitHub-Pull-Request: golang/go#55133
Reviewed-on: https://go-review.googlesource.com/c/go/+/431675
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
This CL adds some optimizaion rules:
1, Converts CMP to CMN, or vice versa, when comparing with a negative
number.
2, For equal and not equal comparisons, CMP can be converted to CMN in
some cases. In theory we could do the same optimization for LT, LE, GT
and GE, but need to account for overflow, this CL doesn't handle them.
There are no noticeable performance changes.
Change-Id: Ia49266c019ab7908ebc9510c2f02e121b1607869
Reviewed-on: https://go-review.googlesource.com/c/go/+/429795
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
get, at least, is called from typedmemclr which must not be interruptible.
These were previously nosplit by accident before CL 424395 (the only
call they had was an intrinsic, so they were leaf functions, so they had
no prologue). After CL 424395 they contained a call (in noinline builds),
thus had a prologue, thus had a suspension point.
I have no idea how we might test this.
This is another motivating use case for having a nosplitrec directive
in the runtime.
Fixes#55156Fixes#54779Fixes#54906Fixes#54907
Change-Id: I851d733d71bda7172c4c96e027657e22b499ee00
Reviewed-on: https://go-review.googlesource.com/c/go/+/431919
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Although I can't think of any reason to do this, it is possible for
a user-defined flag to implement IsBoolFlag but return "false".
This is nuts because checking the interface is satisfied should
obviously be sufficient, but the documentation kinda implies it's
not. And if you try this, you'll discover that the usage message
ignores the return value even though the rest of the package plays
nice. Bother.
So we fix it, as the fix is trivial: call the method when creating
the usage message.
Fixes#53473
Change-Id: I1ac80a876ad5626eebfc5ef6cb972cd3007afaad
Reviewed-on: https://go-review.googlesource.com/c/go/+/431102
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.
The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.
This CL is a minimally invasive solution to #54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.
Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.
Updates #54225.
Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Replace two immediate operations with one, where possible.
Change-Id: Idc00e868155c9ca1d872aaaf70ea1f73e9eac4d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/428497
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The use of SEQZ/SNEZ and SUB allows for other optimisations to be utilised,
particularly absorption into branch equality conditions.
Change-Id: I74e7d6a07a8decc1bdb651660c322bcc6eb6a10a
Reviewed-on: https://go-review.googlesource.com/c/go/+/428216
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The conversion T(x) is implemented as *(*T)(x). Accordingly, runtime
panic messages for (*T)(x) are made more general.
Fixes#46505.
Change-Id: I76317c0878b6a5908299506d392eed50d7ef6523
Reviewed-on: https://go-review.googlesource.com/c/go/+/430415
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
In unified IR, local defined types are promoted to a global defined
type with a "vargen" suffix. These shouldn't actually be exposed to
go/types users, because they're only relevant within function bodies,
which go/types doesn't support importing.
Moreover, in the case of defined types that were declared within a
generic function, they can have ambient type parameters, which the
go/types importer doesn't know how to handle (because they shouldn't
be needed for that use case).
While here, prune the gcimporter_test.go skip list, because some of
the listed failures have actually been fixed and all of them are
specific to the Go 1.18 (nounified) frontend. They all work correctly
with GOEXPERIMENT=unified.
Fixes#55110.
Change-Id: I7dd8b86355d910dfed1d47edbad7695144c3f84d
Reviewed-on: https://go-review.googlesource.com/c/go/+/431495
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This test case already works with GOEXPERIMENT=unified, and it never
worked with Go 1.18 or Go 1.19. So this CL simply adds a regress test
to make sure it continues working.
Fixes#55101.
Change-Id: I7e06bfdc136ce124f65cdcf02d20a1050b841d42
Reviewed-on: https://go-review.googlesource.com/c/go/+/431455
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Resend of CL 315281 which was partially reverted by CL 354249 after the
original CL was suspected to cause test failures as reported in #48789.
It seems that both wait4 and wait6 lead to that particular deadlock, so
let's use wait6. That way we at least don't hit #13987 on netbsd.
Updates #13987
For #48789
For #50138
Change-Id: Iadc4a771217b7e9e821502e89afa07036e0dcb6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/431855
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The type of the source and destination of a memmove call isn't
always accurate. It will always be a pointer (or an unsafe.Pointer), but
the base type might not be accurate. This comes about because multiple
copies of a pointer with different base types are coalesced into a single value.
In the failing example, the IData selector of the input argument is a
*[32]byte in one branch of the type switch, and a *[]byte in the other branch.
During the expand_calls pass both IDatas become just copies of the input
register. Those copies are deduped and an arbitrary one wins (in this case,
*[]byte is the unfortunate winner).
Generally an op v can rely on v.Type during rewrite rules. But relying
on v.Args[i].Type is discouraged.
Fixes#55122
Change-Id: I348fd9accf2058a87cd191eec01d39cda612f120
Reviewed-on: https://go-review.googlesource.com/c/go/+/431496
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Change-Id: If36e9fd7d6b1993ca2d0d382e7fa52212170c798
Reviewed-on: https://go-review.googlesource.com/c/go/+/425481
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
TestReadRequest_Bad's tests for leading whitespace in the first header
were also exercising the test verifying that a HEAD request has no
Content-Length. Also, the test intended to test a leading tab was
actually testing for a leading \t (literal backslash, literal t).
Change-Id: I05b46d05851b49bf75f1d1257c421b953b66ea9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/428134
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#50770.
Change-Id: If0104883bb409ec85827fa5b570f68099ad4fd1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/382734
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: hopehook <hopehook@golangcn.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Changes:
1. When e_shstrndx holds the value SHN_UNDEF (0), the file has no section
name string table. In this case, do not try to set section names .
2. e_shstrndx should point to an SHT_STRTAB section. If it does not, returns
an error.
Reference:
https://refspecs.linuxfoundation.org/elf/gabi4+/ch4.eheader.html
Updates #54967.
Change-Id: Ic8f228061d996fd7845dfa630719a1ba12d2bb60
GitHub-Last-Rev: aeb70ca8a0
GitHub-Pull-Request: golang/go#55001
Reviewed-on: https://go-review.googlesource.com/c/go/+/430155
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In module mode, we shouldn't handle packages under GOPATH any
differently from other packages. Clear GOPATH from the build context
before Importing to ensure that.
Fixes#37015.
Change-Id: I0203e25013716bca346fd4a67d80f1d05bbaea77
Reviewed-on: https://go-review.googlesource.com/c/go/+/412476
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Change-Id: If09094e72161f2c5da9102706781524e32f87782
GitHub-Last-Rev: 89949bc6ee
GitHub-Pull-Request: golang/go#54855
Reviewed-on: https://go-review.googlesource.com/c/go/+/428234
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
For #45557
Change-Id: I25be5b437fa1c9b0e0c46802a9b37efc2d47bca0
Reviewed-on: https://go-review.googlesource.com/c/go/+/431097
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Since os.SEEK_SET was deprecated, use io.SeekStart instead.
Change-Id: I11ae496b071ab35412403ff73e52f3da73d5b120
Reviewed-on: https://go-review.googlesource.com/c/go/+/431096
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: hopehook <hopehook@golangcn.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Now we have 8-byte alignment types on 32-bit system, so in some rare
case, e.g, generated wrapper for embedded interface, the function
argument may need more than 4 byte alignment. We could pad somehow, but
this is a rare case which makes it hard to ensure that we've got it right.
So relaxing the check for argument and return value region of the stack.
Fixes#54991
Change-Id: I34986e17a920254392a39439ad3dcb323da2ea8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/431098
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
When SLTI/SLTIU is used with ANDI/ORI, it may be possible to determine the
outcome based on the values of the immediates. Resolve these cases.
Improves code generation for various shift operations.
While here, sort tests by architecture to improve readability and ease
future maintenance.
Change-Id: I87e71e016a0e396a928e7d6389a2df61583dfd8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/428217
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Jenny Rakoczy <jenny@golang.org>
CL 427615 causes failures when Len64 can't be inlined. It's unclear to
me why this wasn't a problem before, but it is used in sensitive
contexts and therefore really should be marked. Confirmed that the
failures in question reproduce without this change, and don't reproduce
with it.
Fixes#55117.
Change-Id: Ic3aa96af1420cc0c39551908d83f954725c712f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/431058
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If one (accidentally) uses Go 1.16 as bootstrap toolchain, we
want it to print the error
found packages main (build.go) and building_Go_requires_Go_1_17_or_later (notgo117.go)
But because some files lack old style build tags, Go 1.16 instead
prints
//go:build comment without // +build comment
Add the build tags to make the error message work.
Change-Id: Iaa9b3c12e71842bb40f8687b2fda2cc4cb15b113
Reviewed-on: https://go-review.googlesource.com/c/go/+/431057
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
On Linux a signal sent using tgkill will have si_code == SI_TKILL,
not SI_USER. Treat the two cases the same. Add a Linux-specific test.
Change the test to use the C pause function rather than sleeping
for a second, as that achieves the same effect.
Change-Id: I2a36646aecabcab9ec42ed9a048b07c2ff0a3987
Reviewed-on: https://go-review.googlesource.com/c/go/+/431255
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Return errors instead of calling t.Fatalf for timing-sensitive failure
modes. Scale the Sleep call to the timeout (so that it isn't too short
for the longer durations).
Fixes#54891.
Change-Id: I574e85e121becdda9ab8ee6bfd37c18a549d366d
Reviewed-on: https://go-review.googlesource.com/c/go/+/430955
Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fixes#51152
Change-Id: Ib366e733434b4bb60ac93e6e622d9ba50bfe4e26
GitHub-Last-Rev: e98220d62f
GitHub-Pull-Request: golang/go#54921
Reviewed-on: https://go-review.googlesource.com/c/go/+/428955
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently bgsweep attempts to be a low-priority background goroutine
that runs mainly when the application is mostly idle. To avoid
complicating the scheduler further, it achieves this with a simple
heuristic: call Gosched after each span swept. While this is somewhat
inefficient as there's scheduling overhead on each iteration, it's
mostly fine because it tends to just come out of idle time anyway. In a
busy system, the call to Gosched quickly puts bgsweep at the back of
scheduler queues.
However, what's problematic about this heuristic is the number of
tracing events it produces. Average span sweeping latencies have been
measured as low as 30 ns, so every 30 ns in the sweep phase, with
available idle time, there would be a few trace events emitted. This
could result in an overwhelming number, making traces much larger than
they need to be. It also pollutes other observability tools, like the
scheduling latencies runtime metric, because bgsweep stays runnable the
whole time.
This change fixes these problems with two modifications to the
heursitic:
1. Check if there are any idle Ps before yielding. If there are, don't
yield.
2. Sweep at least 10 spans before trying to yield.
(1) is doing most of the work here. This change assumes that the
presence of idle Ps means that there is available CPU time, so bgsweep
is already making use of idle time and there's no reason it should stop.
This will have the biggest impact on the aforementioned issues.
(2) is a mitigation for the case where GOMAXPROCS=1, because we won't
ever observe a zero idle P count. It does mean that bgsweep is a little
bit higher priority than before because it yields its time less often,
so it could interfere with goroutine scheduling latencies more. However,
by sweeping 10 spans before volunteering time, we directly reduce trace
event production by 90% in all cases. The impact on scheduling latencies
should be fairly minimal, as sweeping a span is already so fast, that
sweeping 10 is unlikely to make a dent in any meaningful end-to-end
latency. In fact, it may even improve application latencies overall by
freeing up spans and sweep work from goroutines allocating memory. It
may be worth considering pushing this number higher in the future.
Another reason to do (2) is to reduce contention on npidle, which will
be checked as part of (1), but this is a fairly minor concern. The main
reason is to capture the GOMAXPROCS=1 case.
Fixes#54767.
Change-Id: I4361400f17197b8ab84c01f56203f20575b29fc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/429615
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change adds a metric to the runtime/metrics package which tracks
total mutex wait time for sync.Mutex and sync.RWMutex. The purpose of
this metric is to be able to quickly get an idea of the total mutex wait
time.
The implementation of this metric piggybacks off of the existing G
runnable tracking infrastructure, as well as the wait reason set on a G
when it goes into _Gwaiting.
Fixes#49881.
Change-Id: I4691abf64ac3574bec69b4d7d4428b1573130517
Reviewed-on: https://go-review.googlesource.com/c/go/+/427618
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, wait reasons are set somewhat inconsistently. In a follow-up
CL, we're going to want to rely on the wait reason being there for
casgstatus, so the status quo isn't really going to work for that. Plus
this inconsistency means there are a whole bunch of cases where we could
be more specific about the G's status but aren't.
So, this change adds a new function, casGToWaiting which is like
casgstatus but also sets the wait reason. The goal is that by using this
API it'll be harder to forget to set a wait reason (or the lack thereof
will at least be explicit). This change then updates all casgstatus(gp,
..., _Gwaiting) calls to casGToWaiting(gp, ..., waitReasonX) instead.
For a number of these cases, we're missing a wait reason, and it
wouldn't hurt to add a wait reason for them, so this change also adds
those wait reasons.
For #49881.
Change-Id: Ia95e06ecb74ed17bb7bb94f1a362ebfe6bec1518
Reviewed-on: https://go-review.googlesource.com/c/go/+/427617
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This change adds 3 new waitReasons that correspond to sync.Mutex.Lock,
sync.RWMutex.RLock, and sync.RWMutex.Lock that are plumbed down into
semacquire1 by exporting new functions to the sync package from the
runtime.
Currently these three functions show up as "semacquire" in backtraces
which isn't very clear, though the stack trace itself should reveal
what's really going on. This represents a minor improvement to backtrace
readability, though blocking on an RWMutex.w.Lock will still show up as
blocking on a regular mutex (I suppose technically it is).
This is a step toward helping the runtime identify when a goroutine is
blocked on a mutex of some kind.
For #49881.
Change-Id: Ia409b4d27e117fe4bfdc25fa541e9c58d6d587b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/427616
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This changes adds a breakdown for estimated CPU usage by time. These
estimates are not based on real on-CPU counters, so each metric has a
disclaimer explaining so. They can, however, be more reasonably
compared to a total CPU time metric that this change also adds.
Fixes#47216.
Change-Id: I125006526be9f8e0d609200e193da5a78d9935be
Reviewed-on: https://go-review.googlesource.com/c/go/+/404307
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Josh MacDonald <jmacd@lightstep.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
There are lots of useless buckets with too much precision. Introduce a
minimum level of precision with a minimum bucket bit. This cuts down on
the size of a time histogram dramatically (~3x). Also, pick a smaller
sub bucket count; we don't need 6% precision.
Also, rename super-buckets to buckets to more closely line up with HDR
histogram literature.
Change-Id: I199449650e4b34f2a6dca3cf1d8edb071c6655c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/427615
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Includes cases where the core type of the variadic parameter is
a slice or bytestring. Permits a client to create the signature
for various instantiations of append.
Fixes#55030.
Change-Id: I0f4983eb00c088cbe1d87954ee0b2df0ccc3bc49
Reviewed-on: https://go-review.googlesource.com/c/go/+/430455
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
