Cosmos Nicolaou [Wed, 29 Nov 2023 19:04:57 +0000 (11:04 -0800)]
runtime/pprof: retry vmmap invocation if it failed due to a reported temporary resource shortage
As per #62352 the invocation of vmmap may fail (very rarely) due to
a temporary lack of resources on the test runner machine. This PR
allows for retrying the invocation a fixed number of times before
giving up. This is because we suspect the failure is due to
sensible to retry.
Dmitri Shuralyov [Wed, 29 Nov 2023 20:42:53 +0000 (15:42 -0500)]
all: update vendored dependencies
The Go 1.22 code freeze has recently started. This is a time to update
all golang.org/x/... module versions that contribute packages to the
std and cmd modules in the standard library to latest master versions.
Generated with:
go install golang.org/x/build/cmd/updatestd@latest
go install golang.org/x/tools/cmd/bundle@latest
updatestd -goroot=$(pwd) -branch=master
For #36905.
Change-Id: I76525261b9a954ed21a3bd3cb6c4a12e6c031d80
Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-linux-amd64-longtest,gotip-linux-386-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/546055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Keith Randall [Tue, 28 Nov 2023 21:09:42 +0000 (13:09 -0800)]
doc: release notes for GOARM hardfloat/softfloat change
See CL 514907
Change-Id: Ieba2d7737115c66990b0ea7629033e787a99be93
Reviewed-on: https://go-review.googlesource.com/c/go/+/545655 Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ludi Rehak <ludi317@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Michael Anthony Knyszek [Tue, 28 Nov 2023 17:10:46 +0000 (17:10 +0000)]
internal/trace/v2: tolerate having a P in GoCreateSyscall
On non-pthread platforms, it's totally possible for the same M to
GoCreateSyscall/GoDestroySyscall on the same thread multiple times. That
same thread may hold onto its P through all those calls.
For #64060.
Change-Id: Ib968bfd439ecd5bc24fc98d78c06145b0d4b7802
Reviewed-on: https://go-review.googlesource.com/c/go/+/545515 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Mon, 27 Nov 2023 22:27:32 +0000 (22:27 +0000)]
runtime: put ReadMemStats debug assertions behind a double-check mode
ReadMemStats has a few assertions it makes about the consistency of the
stats it's about to produce. Specifically, how those stats line up with
runtime-internal stats. These checks are generally useful, but crashing
just because some stats are wrong is a heavy price to pay.
For a long time this wasn't a problem, but very recently it became a
real problem. It turns out that there's real benign skew that can happen
wherein sysmon (which doesn't synchronize with a STW) generates a trace
event when tracing is enabled, and may mutate some stats while
ReadMemStats is running its checks.
Fix this by synchronizing with both sysmon and the tracer. This is a bit
heavy-handed, but better that than false positives.
Also, put the checks behind a debug mode. We want to reduce the risk of
backporting this change, and again, it's not great to crash just because
user-facing stats are off. Still, enable this debug mode during the
runtime tests so we don't lose quite as much coverage from disabling
these checks by default.
Fixes #64401.
Change-Id: I9adb3e5c7161d207648d07373a11da8a5f0fda9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/545277
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Rhys Hiltner [Tue, 21 Nov 2023 23:31:26 +0000 (15:31 -0800)]
runtime: test for contention in both semaphore paths
Most contention on the runtime locks inside semaphores is observed in
runtime.semrelease1, but it can also appear in runtime.semacquire1. When
examining contention profiles in TestRuntimeLockMetricsAndProfile, allow
call stacks that include either.
For #64253
Change-Id: Id4f16af5e9a28615ab5032a3197e8df90f7e382f
Reviewed-on: https://go-review.googlesource.com/c/go/+/544375 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Rhys Hiltner <rhys@justin.tv>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Keith Randall [Tue, 28 Nov 2023 01:21:08 +0000 (17:21 -0800)]
cmd/compile: use correct type for slice pointer
The type of the data pointer field of a slice should be a pointer
to the element type, not a *uint8.
This ensures that the SSA value representing the slice's data pointer
can be spilled to the stack slot for the corresponding argument.
Before this change the types didn't match so we ended up spilling the
argument to an autotmp instead of to the dedicated argument slot.
Fixes #64414
Change-Id: I09ee39e93f05aee07e3eceb14e39736d7fd70a33
Reviewed-on: https://go-review.googlesource.com/c/go/+/545357
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: David Chase <drchase@google.com>
Cherry Mui [Mon, 27 Nov 2023 20:17:11 +0000 (15:17 -0500)]
syscall: remove ptrace1 on darwin
On Darwin, the ptrace syscall is called in ptrace1, which then be
called in ptrace. This allows ptrace1 be disabled on iOS (by
implementing ptrace differently). But we can also achieve this by
adding a conditional directly in ptrace. This reduces stack usage
with -N -l, while keeping ptrace disabled on iOS.
For #64113.
Change-Id: I89d8e317e77352fffdbb5a25ba21ee9cdf2e1e20
Reviewed-on: https://go-review.googlesource.com/c/go/+/545276 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Wed, 22 Nov 2023 20:37:13 +0000 (20:37 +0000)]
doc: update release notes from the relnote tool
For #61422.
Change-Id: I0e091c30a445dbc55054c31837c6f051fea3c07d
Reviewed-on: https://go-review.googlesource.com/c/go/+/544537
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Sean Liao [Thu, 23 Nov 2023 18:43:12 +0000 (18:43 +0000)]
.github: clean up issue forms
bugs:
* drop unused labels
* drop the reproduce checkbox:
it's not a strong signal and introduces clutter in github as a task list
* link go.dev/play
govuln:
* use correct label
* ask for version of the tool
* link go.dev/play
telemetry:
* align title with purpose
Change-Id: Id7dd876e518c75dc22e9aec43d9af6e18af088fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/544775 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
cui fliter [Fri, 3 Nov 2023 11:18:00 +0000 (19:18 +0800)]
path: add available godoc link
Change-Id: I6d40a59cde4c3f1d5094f5126fdbc1195285195f
Reviewed-on: https://go-review.googlesource.com/c/go/+/539577 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Dmitri Shuralyov [Wed, 22 Nov 2023 19:31:37 +0000 (14:31 -0500)]
.github: try "import/path: issue title" instead of "affected/package: "
The multiple issue templates pre-populate the issue title with a prefix
that Go issues customarily have. The "affected/package" phrase is short
for "the import path of the affected package". Let's try simplifying it
to just "import/path", and also include "issue title" to make the title
a more representative template of what the final title should look like.
Updates #29839.
Change-Id: I9736d24cf3d0a51536ac13dd07dd189fb51da021
Reviewed-on: https://go-review.googlesource.com/c/go/+/544556
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Sean Liao <sean@liao.dev>
Michael Anthony Knyszek [Wed, 22 Nov 2023 16:36:23 +0000 (16:36 +0000)]
runtime: throw from the systemstack in wirep
The exitsyscall path, since the introduction of the new execution
tracer, stores a just little bit more data in the exitsyscall stack
frame, causing a build failure from exceeding the nosplit limit with
'-N -l' set on all packages (like Delve does).
One of the paths through which this fails is "throw" from wirep, called
by a callee of exitsyscall. By switching to the systemstack on this
path, we can avoid hitting the nosplit limit, fixing the build. It's
also not totally unreasonable to switch to the systemstack for the
throws in this function, since the function has to be nosplit anyway. It
gives the throw path a bit more wiggle room to dump information than it
otherwise would have.
Fixes #64113.
Change-Id: I56e94e40614a202b8ac2fdc8b8b731493b74e5d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/544535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:57:36 +0000 (23:57 +0000)]
runtime: hold sched.lock over traceThreadDestroy in dropm
This is required by traceThreadDestroy, though it's not strictly
necessary in this case. The requirement to hold sched.lock comes from
the assumption that traceThreadDestroy is getting called when the thread
leaves the tracer's view, but in this case the extra m that dropm is
dropping never leaves the allm list. Nevertheless, traceThreadDestroy
requires it just as a safety measure, and that's reasonable. dropm is
generally rare on pthread platforms, so the extra lock acquire over this
short critical section (and only when tracing is enabled) is fine.
Change-Id: Ib631820963c74f2f087d14a0067d0441d75d6785
Reviewed-on: https://go-review.googlesource.com/c/go/+/544396 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:46:48 +0000 (23:46 +0000)]
runtime: move the wakeableSleep lock under sched in the lock rank
Currently the wakeableSleep lock is placed just after timers in the
ranking, but it turns out the timers lock can never be held over a timer
func, so that's wrong. Meanwhile, wakeableSleep can acquire sched.lock.
wakeableSleep, as it turns out, doesn't have any dependencies -- it's
always acquired in a (mostly) regular goroutine context.
Change-Id: Icc8ea76a8b309fbaf0f02215f16e5f706d49cd95
Reviewed-on: https://go-review.googlesource.com/c/go/+/544395
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 23:24:59 +0000 (23:24 +0000)]
runtime: don't hold trace.lock over semrelease in readTrace0
semrelease may unblock a goroutine, but the act of unblocking a
goroutine may emit an event, which in turn may try to acquire trace.lock
again.
It's safe to release trace.lock in readTrace0 for this because all of
the state (one variable) it uses under the lock will be recomputed when
it reacquires the lock. There's also no other synchronization
requirement to hold trace.lock. This is just a mistake.
Change-Id: Iff6c6b02efa298ebed8e60cdf6539ec161d5ec48
Reviewed-on: https://go-review.googlesource.com/c/go/+/544178
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 22:42:35 +0000 (22:42 +0000)]
runtime: don't hold the table lock in (*traceStackTable).dump
There's a conceptual cycle between traceStackTable.lock and
allocation-related locks, but it can't happen in practice because the
caller guarantees that there are no more writers to the table at the
point that dump is called.
But if that's true, then the lock isn't necessary at all. It would be
difficult to model this quiesence in the lockrank mode, so just don't
hold the lock and expand the documentation of the dump method.
Change-Id: Id4db61363f075b7574135529915e8bd4f4f4c082
Reviewed-on: https://go-review.googlesource.com/c/go/+/544177 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Davis Goodin [Wed, 22 Nov 2023 01:12:48 +0000 (17:12 -0800)]
cmd/link/internal/loadpe: fix .xdata unwind info parsing
Unwind info in .xdata was being parsed incorrectly, causing targetOff to
be incorrect and miss finding data in .xdata that it should have found.
This causes a linker issue when using the MinGW MSVCRT compiler.
Contains several fixes based on the exception handling docs: the offset
used to get the number of unwind codes, the calculation of the target
offset based on the dynamic size of the unwind data, and the
UNW_FLAG_CHAININFO flag's value.
Fixes #64200
Change-Id: I6483d921b2bf8a2512a95223bf3c8ce8bc63dc4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/544415
TryBot-Result: Gopher Robot <gobot@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Than McIntosh [Tue, 21 Nov 2023 19:44:41 +0000 (14:44 -0500)]
cmd/link/internal/ld: fix DWARF type DIE "go kind" bug for synthetic ptr types
The code path in linker DWARF type generation that synthesizes pointer
type DIEs needed for other synthesized types wasn't properly setting
the DW_AT_go_kind attribute for the new pointer types.
Fixes #64231.
Change-Id: I70c338d2b33ae3b93a4c6f201e5836d91d368086
Reviewed-on: https://go-review.googlesource.com/c/go/+/544315 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Mauri de Souza Meneguzzo [Tue, 21 Nov 2023 19:34:36 +0000 (19:34 +0000)]
runtime/internal/atomic: deduplicate And/Or code on wasm
When I initially added the wasm code for these ops I did not saw that
wasm actually has the Cas operations implemented, although they are
merely pointer assignments since wasm is single threaded.
Now with a generic implementation for And/Or we can add wasm to the
build tags.
For #61395
Change-Id: I997dc90477c772882d6703df1b795dfc0d90a699
GitHub-Last-Rev: 92736a6e34104a9d234c791673fe0bb79fc97b0b
GitHub-Pull-Request: golang/go#64300
Reviewed-on: https://go-review.googlesource.com/c/go/+/544116
Run-TryBot: Mauri de Souza Meneguzzo <mauri870@gmail.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Srinivas Pokala [Mon, 16 Oct 2023 09:18:50 +0000 (11:18 +0200)]
cmd/asm: fix the KMCTR instruction encoding and argument passing
KMCTR encoding arguments incorrect way, which leading illegal instruction wherver we call KMCTR instruction.IBM z13 machine test's TestAESGCM test using gcmASM implementation, which uses KMCTR instruction to encrypt using AES in counter mode and the KIMD instruction for GHASH. z14+ machines onwards uses gcmKMA implementation for the same.
Fixes #63387
Change-Id: I86aeb99573c3f636a71908c99e06a9530655aa5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/535675 Reviewed-by: Vishwanatha HD <vishwanatha.hd@ibm.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org>
fanzha02 [Tue, 21 Nov 2023 06:52:54 +0000 (06:52 +0000)]
runtime: change the name of variables in asan-related assembly functions
Variables in functions implemented in assembly should have the
same names as when they were defined. The names of some variables
in asan-related assembly functions do not follow the above rule,
which will cause the runtime test to fail. This CL fixes this issue.
Change-Id: I1abf6b220b9802028f8ad5eebc8d3b7cfa3e89ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/541756 Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: M Zhuo <mzh@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Wang Yaduo <wangyaduo@linux.alibaba.com> Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
Meng Zhuo [Sun, 12 Nov 2023 09:05:57 +0000 (17:05 +0800)]
cmd/internal/obj/riscv: add support of PCALIGN directive
Add support for PCALIGN directive on riscv.
This directive can be used within Go asm to align instruction
by padding NOP directives.
This patch also adds a test to verify the correctness of the PCALIGN
directive.
Original credit by Cooper Qu (Alibaba)
https://gitee.com/xuantie_riscv/xuantie-patch
Change-Id: I8b6524a2bf81a1baf7c9d04b7da2db6c1a7b428f
Reviewed-on: https://go-review.googlesource.com/c/go/+/541740
Run-TryBot: M Zhuo <mzh@golangcn.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Wang Yaduo <wangyaduo@linux.alibaba.com> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Mark Ryan <markdryan@rivosinc.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Filippo Valsorda [Tue, 21 Nov 2023 22:16:56 +0000 (23:16 +0100)]
crypto/tls: check and record godebugs more granularly
We should call Value as late as possible to allow programs to set
GODEBUG with os.Setenv, and IncNonDefault only when (and every time) the
GODEBUG has an effect on a connection (that we'd have regularly
rejected).
Change-Id: If7a1446de407db7ca2d904d41dda13558b684dda
Reviewed-on: https://go-review.googlesource.com/c/go/+/544335
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Michael Anthony Knyszek [Tue, 21 Nov 2023 16:51:57 +0000 (16:51 +0000)]
internal/trace/v2: dump text trace on failure only if it fits in the log
Currently we dump text traces to the build log on failure
unconditionally, but this may cause the old infrastructure's builds'
logs to get truncated. Avoid that by setting a threshold on the maximum
size of the text trace we're willing to dump.
We don't need this workaround on the new infrastructure -- logs don't
get truncated there.
Change-Id: I0f50f50bb4b90f87250b673fbe56f48235325610
Reviewed-on: https://go-review.googlesource.com/c/go/+/544216
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Bryan C. Mills [Wed, 28 Jun 2023 13:37:47 +0000 (09:37 -0400)]
testing: simplify concurrency and cleanup logic
While investigating #60083, I found a couple of bugs (notably #61034)
that had slipped through code review in part because the concurrency
patterns used in the testing package were too complex for me to fully
reason about. This change adjusts those patterns to be more in line
with current idioms, and to reduce the number of special cases that
depend on details that should be orthogonal. (For example: the details
of how we invoke the Cleanup functions should not depend on whether
the test happened to run any parallel subtests.)
In the process, this change fixes a handful of bugs:
- Concurrent calls to Run (explicitly allowed by TestParallelSub)
could previously drive the testcontext.running count negative,
causing the number of running parallel tests to exceed the -parallel
flag.
- The -failfast flag now takes effect immediately on failure. It no
longer delays until the test finishes, and no longer misses failures
during cleanup (fixing #61034).
- If a Cleanup function calls runtime.Goexit (typically via t.FailNow)
during a panic, Cleanup functions from its parent tests are no
longer skipped and buffered logs from its parent tests are now
flushed.
- The time reported for a test with subtests now includes the time spent
running those subtests, regardless of whether they are parallel.
(Previously, non-parallel subtests were included but parallel subtests
were not.)
- Calls to (*B).Run in iterations after the first are now diagnosed
with a panic. (This diagnoses badly-behaved benchmarks: if Run is
called during the first iteration, no subsequent iterations are
supposed to occur.)
Fixes #61034.
Change-Id: I3797f6ef5210a3d2d5d6c2710d3f35c0219b02ea
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-linux-amd64-longtest-race,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/506755
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The v2 execution tracer has a rudimentary deadlock detector, but it's
based on an arbitrary threshold that an actually get hit even if there's
no deadlock. This ends up breaking tests sometimes, and it would be bad
if this just appeared in production logs.
Put this 'deadlock detector' behind a flag.
For #55317.
Change-Id: I286f0c05b3ac9600f4f2f9696065cac8bbd25f00
Reviewed-on: https://go-review.googlesource.com/c/go/+/544235 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 03:23:05 +0000 (03:23 +0000)]
runtime: emit a ProcSteal from entersyscall_gcwait
Currently entersyscall_gcwait always emits a ProcStop event. Most of the
time, this is correct, since the thread that just put the P into
_Psyscall is the same one that is putting it into _Pgcstop. However it's
possible for another thread to steal the P, start running a goroutine,
and then enter another syscall, putting the P back into _Psyscall. In
this case ProcStop is incorrect; the P is getting stolen. This leads to
broken traces.
Fix this by always emitting a ProcSteal event from entersyscall_gcwait.
This means that most of the time a thread will be 'stealing' the proc
from itself when it enters this function, but that's theoretically fine.
A ProcSteal is really just a fancy ProcStop.
Well, it would be if the parser correctly handled a self-steal. This is
a minor bug that just never came up before, but it's an update order
error (the mState is looked up and modified, but then it's modified
again at the end of the function to match newCtx). There's really no
reason a self-steal shouldn't be allowed, so fix that up and add a test.
Change-Id: Iec3d7639d331e3f2d127f92ce50c2c4a7818fcd3
Reviewed-on: https://go-review.googlesource.com/c/go/+/544215
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Kir Kolyshkin [Wed, 13 Sep 2023 07:58:20 +0000 (00:58 -0700)]
internal/syscall/unix: add PidFDSendSignal for Linux
CL 520266 added pidfd_send_signal linux syscall numbers to the
syscall package for the sake of a unit test.
As pidfd_send_signal will be used from the os package, let's revert the
changes to syscall package, add the pidfd_send_signal syscall numbers
and the implementation to internal/syscall/unix, and change the above
test to use it.
Updates #51246.
For #62654.
Change-Id: I862174c3c1a64baf1080792bdb3a1c1d1b417bb4
Reviewed-on: https://go-review.googlesource.com/c/go/+/528436
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Olivier Mengué [Thu, 26 Oct 2023 20:18:33 +0000 (22:18 +0200)]
errors: clarify references to Unwrap in doc
CL 535080 incorrectly links the unclear mention of Unwrap to the func
Unwrap in doc for errors.Is and errors.As
Instead we clarify that "Unwrap" is a reference
to the "Unwrap() error" or "Unwrap() []error" methods, not to the
"Unwrap(error) error" function which is also available in the package.
Change-Id: I8314993932e1e7a2dc77400f74d81f3a8aa891de
Reviewed-on: https://go-review.googlesource.com/c/go/+/538155 Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: qiulaidongfeng <2645477756@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 21:19:35 +0000 (21:19 +0000)]
cmd/trace/v2: emit regions in the goroutine-oriented task view
This change emits regions in the goroutine-oriented task view (the
/trace endpoint with the taskid query variable set) in the same way the
old cmd/trace does.
For #60773.
Fixes #63960.
Change-Id: If6c3e7072c694c84a7d2d6c34df668f48d3acc2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/543995 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 19:29:42 +0000 (19:29 +0000)]
cmd/trace/v2: add support for the goroutine-oriented task view
This change adds support for a goroutine-oriented task view via the
/trace?taskid=<taskid> endpoint. This works but it's missing regions.
That will be implemented in a follow-up CL.
For #60773.
For #63960.
Change-Id: I086694143e5c71596ac22fc551416868f0b80923
Reviewed-on: https://go-review.googlesource.com/c/go/+/543937
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 07:28:37 +0000 (07:28 +0000)]
cmd/trace/v2: add thread-oriented mode for v2 traces
This is a nice-to-have that's now straightforward to do with the new
trace format. This change adds a new query variable passed to the
/trace endpoint called "view," which indicates the type of view to
use. It is orthogonal with task-related views.
Unfortunately a goroutine-based view isn't included because it's too
likely to cause the browser tab to crash.
For #60773.
For #63960.
Change-Id: Ifbcb8f2d58ffd425819bdb09c586819cb786478d
Reviewed-on: https://go-review.googlesource.com/c/go/+/543695
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Mon, 20 Nov 2023 05:37:36 +0000 (05:37 +0000)]
cmd/trace/v2: add support for a task-oriented procs-based view
This change implements support for the trace?focustask=<taskid> endpoint
in the trace tool for v2 traces.
Note: the one missing feature in v2 vs. v1 is that the "irrelevant" (but
still rendered) events are not grayed out. This basically includes
events that overlapped with events that overlapped with other events
that were in the task time period, but aren't themselves directly
associated. This is probably fine -- the UI already puts a very obvious
focus on the period of time the selected task was running.
For #60773.
For #63960.
Change-Id: I5c78a220ae816e331b74cb67c01c5cd98be40dd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/543596
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Sat, 18 Nov 2023 05:50:50 +0000 (05:50 +0000)]
cmd/trace/v2: add support for goroutine filtering
This change adds support for the trace?goid=<goid> endpoint to the trace
tool for v2 traces.
In effect, this change actually implements a per-goroutine view. I tried
to add a link to the main page to enable a "view by goroutines" view
without filtering, but the web trace viewer broke the browser tab when
there were a few hundred goroutines. The risk of a browser hang probably
isn't worth the cases where this is nice, especially since filtering by
goroutine already works. Unfortunate, but c'est l'vie. Might be worth
revisiting if we change out the web viewer in the future.
For #60773.
For #63960.
Change-Id: I8e29f4ab8346af6708fd8824505c30f2c43db796
Reviewed-on: https://go-review.googlesource.com/c/go/+/543595
TryBot-Bypass: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 14 Nov 2023 06:19:07 +0000 (06:19 +0000)]
cmd/trace: factor out durationHistogram
This code will be useful for the new tracer, and there's no need to
duplicate it. This change copies it to internal/trace/traceviewer, adds
some comments, and renames it to TimeHistogram.
While we're here, let's get rid of the unused String method which has a
comment talking about how awful the rendering is.
Also, let's get rid of uses of niceDuration. We'd have to bring it
with us in the move and I don't think it's worth it. The difference
between the default time.Duration rendering and the niceDuration
rendering is usually a few extra digits of precision. Yes, it's noisier,
but AFAICT it's not substantially worse. It doesn't seem worth the new
API, even if it's just internal. We can also always bring it back later.
For #60773.
For #63960.
Change-Id: I795f58f579f1d503c540c3a40bed12e52bce38e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/542001
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Mon, 13 Nov 2023 21:06:40 +0000 (21:06 +0000)]
internal/trace: add task analysis for v2 traces
For v1 traces, cmd/trace contains code for analyzing tasks separately
from the goroutine analysis code present in internal/trace. As I started
to look into porting that code to v2 traces, I noticed that it wouldn't
be too hard to just generalize the existing v2 goroutine summary code to
generate exactly the same information.
This change does exactly that.
For #60773.
For #63960.
Change-Id: I0cdd9bf9ba11fb292a9ffc37dbf18c2a6a2483b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/542076
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Michael Anthony Knyszek [Fri, 17 Nov 2023 17:56:27 +0000 (17:56 +0000)]
internal/trace/v2: redefine NoTask and add BackgroundTask
The v2 trace parser currently handles task inheritance and region task
association incorrectly. It assumes that a TaskID of 0 means that there
is no task. However, this is only true for task events. A TaskID of 0
means that a region gets assigned to the "background task." The parser
currently has no concept of a "background task."
Fix this by defining the background task as task ID 0 and redefining
NoTask to ^uint64(0). This aligns the TaskID values more closely with
other IDs in the parser and also enables disambiguating these two cases.
For #60773.
For #63960.
Change-Id: I09c8217b33b87c8f8f8ea3b0203ed83fd3b61e11
Reviewed-on: https://go-review.googlesource.com/c/go/+/543019 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Mon, 13 Nov 2023 18:57:06 +0000 (18:57 +0000)]
cmd/trace/v2: add support for pprof endpoints
This change adds support for the pprof endpoints to cmd/trace/v2.
In the process, I realized we need to pass the goroutine summaries to
more places, and previous CLs had already done the goroutine analysis
during cmd/trace startup. This change thus refactors the goroutine
analysis API once again to operate in a streaming manner, and to run
at the same time as the initial trace parsing. Now we can include it in
the parsedTrace type and pass that around as the de-facto global trace
context.
Note: for simplicity, this change redefines "syscall" profiles to
capture *all* syscalls, not just syscalls that block. IIUC, this choice
was partly the result of a limitation in the previous trace format that
syscalls don't all have complete durations and many short syscalls are
treated as instant. To this end, this change modifies the text on the
main trace webpage to reflect this change.
For #60773.
For #63960.
Change-Id: I601d9250ab0849a0bfaef233fd9b1e81aca9a22a
Reviewed-on: https://go-review.googlesource.com/c/go/+/541999
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 23:27:31 +0000 (23:27 +0000)]
internal/trace/traceviewer: make the mmu handler more self-contained
The last change made the MMU rendering code common and introduced a new
API, but it was kind of messy. Part of the problem was that some of the
Javascript in the template for the main page referred to specific
endpoints on the server.
Fix this by having the Javascript access the same endpoint but with a
different query variable. Now the Javascript code doesn't depend on
specific endpoints, just on query variables for the current endpoint.
For #60773.
For #63960.
Change-Id: I1c559d9859c3a0d62e2094c9d4ab117890b63b31
Reviewed-on: https://go-review.googlesource.com/c/go/+/541259
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 23:10:04 +0000 (23:10 +0000)]
cmd/trace: common up the mmu page and add it to cmd/trace/v2
This change moves the MMU HTTP handlers and functionality into the
traceviewer package, since unlike the goroutine pages the vast majority
of that functionality is identical between v1 and v2. This change
involves some refactoring so that callers can plug in their own mutator
utilization computation functions (which is the only point of difference
between v1 and v2). The new interface isn't especially nice, but part of
the problem is the MMU handlers depend on specific endpoints to exist. A
follow-up CL will clean this up a bit.
Like the previous CL did for goroutine analysis, modify the v2 mutator
utilization API to accept a slice of trace events. Again, we might as
well reuse what was already parsed and will be needed for other
purposes. It also simplifies the API slightly.
For #60773.
For #63960.
Change-Id: I6c21ec8d1bf7e95eff5363d0e0005c9217fa00e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/541258
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Sun, 12 Nov 2023 21:48:31 +0000 (21:48 +0000)]
cmd/trace/v2: add goroutine analysis pages
This is a complete fork and most of a rewrite of the goroutine analysis
pages for v2 traces. It fixes an issue with the old page where GC time
didn't really make any sense, generalizes the page and breaks things
down further, and adds clarifying text.
This change also modifies the SummarizeGoroutines API to not stream the
trace. This is unfortunate, but we're already reading and holding the
entire trace in memory for the trace viewer. We can revisit this
decision in the future. Also, we want to do this now because the
GoroutineSummary holds on to pointers to events, and these events will
be used by the user region and user task analyses. While tracev2 events
are values and they should be equivalent no matter how many times we
parse a trace, this lets us reference the event in the slice directly.
For #60773.
For #63960.
Fixes #62443.
Change-Id: I1c5ab68141869378843f4f2826686038e4533090
Reviewed-on: https://go-review.googlesource.com/c/go/+/541257 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 21 Nov 2023 17:33:20 +0000 (17:33 +0000)]
internal/trace: use the correct stack for goroutine naming in v2 traces
Currently goroutine names are determined (for v2 traces) by
internal/tracev/2.Event.Stack, but this is wrong in general. For
example, if we end up seeing a transition from GoNotExist->GoRunnable
(goroutine creation) then we're taking the stack from the creator, not
the created goroutine (which is what we're naming at that point).
Use the StateTransition.Stack instead. This is always the correct one to
use because we're always naming the goroutine that the state transition
is for.
Change-Id: I3fc7c8e4f85dfee3802d666c0c091b6953c7d6cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/544317
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Rhys Hiltner [Tue, 21 Nov 2023 16:03:54 +0000 (16:03 +0000)]
runtime: profile contended lock calls
Add runtime-internal locks to the mutex contention profile.
Store up to one call stack responsible for lock contention on the M,
until it's safe to contribute its value to the mprof table. Try to use
that limited local storage space for a relatively large source of
contention, and attribute any contention in stacks we're not able to
store to a sentinel _LostContendedLock function.
Avoid ballooning lock contention while manipulating the mprof table by
attributing to that sentinel function any lock contention experienced
while reporting lock contention.
Guard collecting real call stacks with GODEBUG=profileruntimelocks=1,
since the available data has mixed semantics; we can easily capture an
M's own wait time, but we'd prefer for the profile entry of each
critical section to describe how long it made the other Ms wait. It's
too late in the Go 1.22 cycle to make the required changes to
futex-based locks. When not enabled, attribute the time to the sentinel
function instead.
Fixes #57071
This is a roll-forward of https://go.dev/cl/528657, which was reverted
in https://go.dev/cl/543660
Reason for revert: de-flakes tests (reduces dependence on fine-grained
timers, correctly identifies contention on big-endian futex locks,
attempts to measure contention in the semaphore implementation but only
uses that secondary measurement to finish the test early, skips tests on
single-processor systems)
Change-Id: I31389f24283d85e46ad9ba8d4f514cb9add8dfb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/544195 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Rhys Hiltner <rhys@justin.tv>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Michael Anthony Knyszek [Tue, 21 Nov 2023 17:23:43 +0000 (17:23 +0000)]
internal/trace/v2: forward Event.Stack to StateTransition.Stack
Currently StateTransition.Stack is only set for the GoCreate case,
because there are two stacks and we need to distinguish them. But the
docs for StateTransition.Stack say that that stack always references the
resource that is transitioning. There are quite a few cases where
Event.Stack is actually the appropriate stack to for
StateTransition.Stack, but in these cases it's left empty, and the
caller just needs to understand which one to look at. This isn't great.
Forward Event.Stack to StateTransition.Stack whenever Event.Stack also
refers to the resource experiencing the state transition.
Change-Id: Ie43fc6036f2712c7982174d5739d95765312dfcc
Reviewed-on: https://go-review.googlesource.com/c/go/+/544316
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Michael Anthony Knyszek [Tue, 14 Nov 2023 17:03:24 +0000 (17:03 +0000)]
cmd/trace: add almost full support for v2 traces in the trace viewer
This change refactors the cmd/trace package and adds most of the support
for v2 traces.
The following features of note are missing in this CL and will be
implemented in follow-up CLs:
- The focustask filter for the trace viewer
- The taskid filter for the trace viewer
- The goid filter for the trace viewer
- Pprof profiles
- The MMU graph
- The goroutine analysis pages
- The task analysis pages
- The region analysis pages
This CL makes one notable change to the trace CLI: it makes the -d flag
accept an integer to set the debug mode. For old traces -d != 0 works
just like -d. For new traces -d=1 means the high-level events and -d=2
means the low-level events.
Thanks to Felix Geisendörfer (felix.geisendoerfer@datadoghq.com) for
doing a lot of work on this CL; I picked this up from him and got a
massive headstart as a result.
For #60773.
For #63960.
Change-Id: I3626e22473227c5980134a85f1bb6a845f567b1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/542218 Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Bypass: Michael Knyszek <mknyszek@google.com>
Kir Kolyshkin [Mon, 20 Nov 2023 21:16:13 +0000 (13:16 -0800)]
os: remove useless if from Wait on unix
Back in the day, Wait used to accept options argument.
CL 4962042 fixed the issue of setting process.done flag when WNOHANG
option was used.
Later, CL 5688046 removed options argument from Wait, but did not remove
pid1 != 0 check which was meant to be used with WNOHANG only.
Remove the check, which is useless and also confusing.
Change-Id: I73b9ef4a0dbe35466e659ca58b896d515ba86d02
Reviewed-on: https://go-review.googlesource.com/c/go/+/543736
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
order.go ensures expressions that are passed to the runtime by address
are in fact addressable. However, in the case of local variables, if the
variable hasn't already been marked as addrtaken, then taking its
address here will effectively prevent the variable from being converted
to SSA form.
Instead, it's better to just copy the variable into a new temporary,
which we can pass by address instead. This ensures the original variable
can still be converted to SSA form.
Fixes #63332.
Change-Id: I182376d98d419df8bf07c400d84c344c9b82c0fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/541715
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Kir Kolyshkin [Thu, 16 Nov 2023 08:42:04 +0000 (00:42 -0800)]
syscall: fix getting pidfd when using CLONE_NEWUSER
While working on CL 528798, I found out that sys.PidFD field (added
in CL 520266) is not filled in when CLONE_NEWUSER is used.
This happens because the code assumed that the parent and the child
run in the same memory space. This assumption is right only when
CLONE_VM is used for clone syscall, and the code only sets CLONE_VM
when CLONE_NEWUSER is not used.
Fix this, and add a test case (which fails before the fix).
Updates #51246.
Change-Id: I805203c1369cadd63d769568b132a9ffd92cc184
Reviewed-on: https://go-review.googlesource.com/c/go/+/542698
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Than McIntosh [Tue, 7 Nov 2023 15:28:56 +0000 (10:28 -0500)]
cmd/compile/internal/inline: revise -m=2 status messages
This patch revises the compiler's "-m=2" status messages related to
inlining. The "can inline" remarks will continue to use the same
format, but the remarks when a specific call site is inlined will be
changed to refer to the score used; before we had
runtime/traceback.go:1131:28: inlining call to gotraceback
runtime/traceback.go:1183:25: inlining call to readgstatus
and with GOEXPERIMENT=newinliner the new messages will be:
runtime/traceback.go:1131:28: inlining call to gotraceback with score 62
runtime/traceback.go:1183:25: inlining call to readgstatus with score 9
Change-Id: Ia86cf5351d29eda64a5426ca0a2a2ec0c2900d81
Reviewed-on: https://go-review.googlesource.com/c/go/+/540775 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
WANG Xuerui [Tue, 13 Dec 2022 08:30:45 +0000 (16:30 +0800)]
cmd/dist, cmd/link, internal, runtime: implement buildmode=plugin for linux/loong64
According to review, buildmode=shared has unfixed shortcomings and
should be considered legacy at this time. So only buildmode=plugin is
going to be added for loong64 which is a relatively new platform.
Change-Id: Iac0b9f57e4ee01755458e180bb24d1b2a146fdf0
Reviewed-on: https://go-review.googlesource.com/c/go/+/480878
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: WANG Xuerui <git@xen0n.name> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: abner chenc <chenguoqi@loongson.cn> Reviewed-by: Meidan Li <limeidan@loongson.cn>
Carlo Alberto Ferraris [Sun, 2 Apr 2023 14:54:35 +0000 (23:54 +0900)]
sync: do not unnecessarily keep alive functions wrapped by Once(Func|Value|Values)
The function passed to OnceFunc/OnceValue/OnceValues may transitively
keep more allocations alive. As the passed function is guaranteed to be
called at most once, it is safe to drop it after the first call is
complete. This avoids keeping the passed function (and anything it
transitively references) alive until the returned function is GCed.
Than McIntosh [Fri, 3 Nov 2023 18:04:57 +0000 (14:04 -0400)]
cmd/compile/internal/inline: minor compile time improvements in func flags
Some small changes to help reduce compile time for function flags
computation. The current implementation of panic path detection adds
an entry to a map for every node in the function, which is wasteful
(and shows up in cpu profiles). Switch to only adding entries where
they are useful. This is especially important for functions with large
map literals and other constructs with many non-statement nodes.
Change-Id: I9cfb2cd1cbf480f21298e6102aa99e2d77219f3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/539696 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Than McIntosh [Mon, 30 Oct 2023 16:55:42 +0000 (12:55 -0400)]
cmd/compile/internal/inline: fix bug in param flags heuristics
Fix a bug in the code that analyzes how function parameters are used,
which wasn't properly handling certain types of closures. The code
in question has support for deriving flags for a given parameter based
on whether it is passed to a call. Example:
When analyzing "bar", we can derive the "FeedsIndirectCall" flag for
parameter "f1" by virtue of the fact that it is passed directly to
"bar", and bar's corresponding parameter "f2" has the flag set. For
a more complex example such as
func foo(f1 func(int)) func() int {
return func(q int) int { <<-- HERE
bar(99, f1)
return q
}
}
func bar(x int, f2 func()) {
f2(x)
}
The heuristics code would panic when examining the closure marked above
due to the fact that the call to "bar" was passing an ir.Name with class
PPARAM, but no such param was present in the enclosing function.
Change-Id: I30436ce716b51bfb03e42e7abe76a4514e6b9285
Reviewed-on: https://go-review.googlesource.com/c/go/+/539320
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Bryan C. Mills [Wed, 28 Jun 2023 19:04:24 +0000 (15:04 -0400)]
testing: use subprocesses in TestTBHelper and TestTBHelperParallel
These tests are checking the output of test functions that call the
Helper methods. However, they were reaching into package internals
instead of running those test functions as actual tests.
That not only produced significant differences in formatting (such as
indentation for subtests), but also caused test flags such as
"-failfast" passed for the overall test run to interfere with the
output formatting.
Now, we run the test functions as real tests in a subprocess,
so that we get the real output and formatting of those tests.
This makes the tests not only more realistic, but also less
sensitive to otherwise-irrelevant implementation details
(such as the names and signatures of unexported types and
functions in the testing package).
Fixes #61016.
Change-Id: I646fbbd7cfeb00382054677f726c05fc9d35d0dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/506955
Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Than McIntosh [Fri, 27 Oct 2023 12:58:16 +0000 (08:58 -0400)]
cmd/compile/internal/inline: rework use of ir.StaticValue
When running the code to compute function properties that feed
inlining heuristics, the existing heuristics implementation makes
fairly extensive use of ir.StaticValue and ir.Reassigned to sharpen
the analysis. These calls turn out to cause a significant compile time
increase, due to the fact that each call can potentially walk every
node in the IR for the function. To help with this problem, switch the
heuristics code over to using the new "batch mode" reassignment helper
added in the previous CL.
Change-Id: Ib15a62416134386e34b7cfa1130a4b413a37b225
Reviewed-on: https://go-review.googlesource.com/c/go/+/537977
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
qiulaidongfeng [Mon, 20 Nov 2023 23:08:53 +0000 (23:08 +0000)]
os: avoid TestFileChdir fail when GOROOT is a symbolic link
If GOROOT is a symbolic link,
the paths obtained from the
first and second Getwd of TestFileChdir are different,
and this CL fixes the test failure in this situation.