Daniel Martí [Sun, 8 Jan 2023 20:03:33 +0000 (20:03 +0000)]
encoding/gob: shave off some init time cost
Avoid unnecessary allocations when calling reflect.TypeOf;
we can use nil pointers, which fit into an interface without allocating.
This saves about 1% of CPU time.
The builtin types are limited to typeIds between 0 and firstUserId,
and since firstUserId is 64, builtinIdToType does not need to be a map.
We can simply use an array of length firstUserId, which is simpler.
This saves about 1% of CPU time.
idToType is similar to firstUserId in that it is a map keyed by typeIds.
The difference is that it can grow with the user's types.
However, each added type gets the next available typeId,
meaning that we can use a growing slice, similar to the case above.
nextId then becomes the current length of the slice.
This saves about 1% of CPU time.
typeInfoMap is stored globally as an atomic.Value,
where each modification loads the map, makes a whole copy,
adds the new element, and stores the modified copy.
This is perfectly fine when the user registers types,
as that can happen concurrently and at any point in the future.
However, during init time, we sequentially register many types,
and the overhead of copying maps adds up noticeably.
During init time, use a regular global map instead,
which gets replaced by the atomic.Value when our init work is done.
This saves about 2% of CPU time.
Finally, avoid calling checkId in bootstrapType;
we have just called setTypeId, whose logic for getting nextId is simple,
so the extra check doesn't gain us much.
This saves about 1% of CPU time.
Using benchinit, which transforms GODEBUG=inittrace=1 data into Go
benchmark compatible output, results in a nice improvement:
name old time/op new time/op delta
EncodingGob 175µs ± 0% 162µs ± 0% -7.45% (p=0.016 n=5+4)
name old alloc/op new alloc/op delta
EncodingGob 39.0kB ± 0% 36.1kB ± 0% -7.35% (p=0.016 n=5+4)
name old allocs/op new allocs/op delta
EncodingGob 588 ± 0% 558 ± 0% -5.10% (p=0.000 n=5+4)
Updates #26775.
Change-Id: I28618e8b96ef440480e666ef2cd5c4a9a332ef21
Reviewed-on: https://go-review.googlesource.com/c/go/+/460543 Reviewed-by: Carlos Amedee <carlos@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Daniel Martí [Sun, 8 Jan 2023 23:07:43 +0000 (23:07 +0000)]
internal/profile: use internal/lazyregexp for the legacy parser
Per benchinit, this makes a big difference to init times:
name old time/op new time/op delta
InternalProfile 185µs ± 1% 6µs ± 1% -96.51% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
InternalProfile 101kB ± 0% 4kB ± 0% -95.72% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
InternalProfile 758 ± 0% 25 ± 0% -96.70% (p=0.008 n=5+5)
The fixed 0.2ms init cost is saved for any importer of net/http/pprof,
but also for cmd/compile, as it supports PGO now.
A Go program parsing profiles might not even need to compile these
regular expressions at all, if it doesn't encounter any legacy files.
I suspect this will be the case with most invocations of cmd/compile.
Updates #26775.
Change-Id: I8374dc64459f0b6bb09bbdf9d0b6c55d7ae1646e
Reviewed-on: https://go-review.googlesource.com/c/go/+/460545 Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Daniel Martí [Mon, 9 Jan 2023 12:46:20 +0000 (12:46 +0000)]
cmd/link: use strings.LastIndexByte to fix a TODO
Go 1.20 will require Go 1.17 to bootstrap, so we can stop worrying about
older Go bootstrap versions. https://go.dev/issues/44505 fixed most of
those TODOs, but this one was presumably forgotten about.
Change-Id: I0c19ec4eec65cd807e7db9a57c5969845d915c07
Reviewed-on: https://go-review.googlesource.com/c/go/+/461155 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Srinivas Pokala [Wed, 14 Dec 2022 05:31:50 +0000 (06:31 +0100)]
cmd/internal/obj/s390x, runtime: fix breakpoint in s390x
Currently runtime.Breakpoint generates SIGSEGV in s390x.
The solution to this is add new asm instruction BRRK of
type FORMAT_E for the breakpoint exception.
Fixes #52103
Change-Id: I8358a56e428849a5d28d5ade141e1d7310bee084
Reviewed-on: https://go-review.googlesource.com/c/go/+/457456 Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Jorropo [Sun, 6 Nov 2022 05:37:13 +0000 (06:37 +0100)]
cmd/compile: AMD64v3 remove unnecessary TEST comparision in isPowerOfTwo
With GOAMD64=V3 the canonical isPowerOfTwo function:
func isPowerOfTwo(x uintptr) bool {
return x&(x-1) == 0
}
Used to compile to:
temp := BLSR(x) // x&(x-1)
flags = TEST(temp, temp)
return flags.zf
However the blsr instruction already set ZF according to the result.
So we can remove the TEST instruction if we are just checking ZF.
Such as in multiple pieces of code around memory allocations.
This make the code smaller and faster.
Change-Id: Ia12d5a73aa3cb49188c0b647b1eff7b56c5a7b58
Reviewed-on: https://go-review.googlesource.com/c/go/+/448255
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Jorropo [Wed, 30 Nov 2022 08:45:29 +0000 (09:45 +0100)]
cmd/compile: rewrite empty makeslice to zerobase pointer
make\(\[\][a-zA-Z0-9]+, 0\) is seen 52 times in the go source.
And at least 391 times on internet:
https://grep.app/search?q=make%5C%28%5C%5B%5C%5D%5Ba-zA-Z0-9%5D%2B%2C%200%5C%29®exp=true
This used to compile to calling runtime.makeslice.
However we can copy what we do for []T{}, just use a zerobase pointer.
On my machine this is 10x faster (from 3ns to 0.3ns).
Note that an empty loop also runs in 0.3ns,
so this really is free when you count superscallar execution.
Change-Id: I1cfe7e69f5a7a4dabbc71912ce6a4f8a2d4a7f3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/454036 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Jakub Ciolek <jakub@ciolek.dev>
liu-xuewen [Sat, 5 Nov 2022 07:40:49 +0000 (07:40 +0000)]
runtime: remove 104 byte stack guard
The number 104 appears to date back to the
first implementation of split stacks in
https://go.googlesource.com/go/+/b987f7a757f53f460973622a36eebb696f9b5060.
That change introduces a 104 byte stack guard.
it doesn't makes any sense today.
Change-Id: I73069f6d1a827653af63e616f0119fbac809882e
GitHub-Last-Rev: bcf900059047548c1709c6d4cf4649a96ad85e57
GitHub-Pull-Request: golang/go#56594
Reviewed-on: https://go-review.googlesource.com/c/go/+/448036
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Keith Randall [Tue, 17 Nov 2020 23:32:45 +0000 (15:32 -0800)]
cmd/compile: improve scheduling pass
Convert the scheduling pass from scheduling backwards to scheduling forwards.
Forward scheduling makes it easier to prioritize scheduling values as
soon as they are ready, which is important for things like nil checks,
select ops, etc.
Forward scheduling is also quite a bit clearer. It was originally
backwards because computing uses is tricky, but I found a way to do it
simply and with n lg n complexity. The new scheme also makes it easy
to add new scheduling edges if needed.
Fixes #42673
Update #56568
Change-Id: Ibbb38c52d191f50ce7a94f8c1cbd3cd9b614ea8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/270940
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: David Chase <drchase@google.com>
Marcel Meyer [Thu, 19 Jan 2023 22:26:15 +0000 (22:26 +0000)]
all: fix typos in go file comments
This is the second round to look for spelling mistakes. This time the
manual sifting of the result list was made easier by filtering out
capitalized and camelcase words.
This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.
Change-Id: Ie8a2092aaa7e1f051aa90f03dbaf2b9aaf5664a9
GitHub-Last-Rev: fc2bd6e0c51652f13a7588980f1408af8e6080f5
GitHub-Pull-Request: golang/go#57737
Reviewed-on: https://go-review.googlesource.com/c/go/+/461595
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
fangguizhen [Thu, 19 Jan 2023 03:12:12 +0000 (03:12 +0000)]
bytes, strings: rename field in CutSuffix tests
Change-Id: I63181f6540fc1bfcfc988a16bf9fafbd3575cfdf
GitHub-Last-Rev: d90528730a92a087866c1bfc227a0a0bf1cdffbe
GitHub-Pull-Request: golang/go#57909
Reviewed-on: https://go-review.googlesource.com/c/go/+/462284 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Alice [Mon, 16 Jan 2023 11:54:01 +0000 (11:54 +0000)]
misc/cgo/testsanitizers: use fmt.Printf instead fmt.Println
Change-Id: Ie46bc3cbfb2622b5eb70618557ff5398866f5607
GitHub-Last-Rev: a665ef84dd9c11c6c274ad7f1cb51733d8253f6d
GitHub-Pull-Request: golang/go#57813
Reviewed-on: https://go-review.googlesource.com/c/go/+/462044
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
fangguizhen [Mon, 16 Jan 2023 17:46:20 +0000 (17:46 +0000)]
time: no need to wrap error with errors.New
Change-Id: Ibd519ed6419f8f21c89a111a0326d0788aca9d19
GitHub-Last-Rev: 45e3224f9afef57f49eebf3cbdf6b1d01cfd7346
GitHub-Pull-Request: golang/go#57819
Reviewed-on: https://go-review.googlesource.com/c/go/+/462046
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
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>
Damien Neil [Wed, 11 Jan 2023 21:47:38 +0000 (13:47 -0800)]
net/http: close Request.Body when pconn write loop exits early
The pconn write loop closes a request's body after sending the
request, but in the case where the write loop exits with an
unsent request in writech the body is never closed.
Close the request body in this case.
Fixes #49621
Change-Id: Id94a92937bbfc0beb1396446f4dee32fd2059c7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461675
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Ian Lance Taylor [Wed, 11 Jan 2023 20:17:20 +0000 (12:17 -0800)]
archive/zip: use base offset 0 if it has a valid entry
In CL 408734 we introduced a fall back to base offset 0 if reading a
directory entry at the computed base offset failed. We have now found
a file in the wild for which the computed base offset is incorrect,
but happens to refer to a valid directory entry. In this CL, we change
the fallback such that if the first directory header relative to base
offset 0 is valid, we just use base offset 0.
Change-Id: Ia9ace20c1065d1f651035f16f7d91d741ab1dbf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/461598 Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Wayne Zuo [Thu, 12 Jan 2023 11:37:18 +0000 (19:37 +0800)]
cmd/internal/obj/riscv: add check for invalid shift amount input
Current RISCV64 assembler do not check the invalid shift amount. This CL
adds the check to avoid generating invalid instructions.
Fixes #57755
Change-Id: If33877605e161baefd98c50db1f71641ca057507
Reviewed-on: https://go-review.googlesource.com/c/go/+/461755 Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Keith Randall [Sat, 26 Nov 2022 23:03:51 +0000 (15:03 -0800)]
cmd/compile: add memory argument to GetCallerSP
We need to make sure that when we get the stack pointer, we get it
at the right time.
V = GetCallerSP
Call()
W = GetCallerSP
If Call causes a stack growth, then we will be in a situation
where V != W. So it matters when GetCallerSP operations get scheduled.
Add a memory argument to GetCallerSP so it can't be reordered with
things like calls.
Change-Id: I6cc801134c38e358c5a1ec0c09d38379a16a4184
Reviewed-on: https://go-review.googlesource.com/c/go/+/453515 Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Martin Möhrmann <martin@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Dmitri Shuralyov [Thu, 5 Jan 2023 01:54:34 +0000 (20:54 -0500)]
cmd/internal/osinfo: report Node.js version
Seeing the Node.js version that was used during a particular test run
should be helpful during the upcoming migration from Node.js 14 to 18.
Add minimal support for that.
For golang/go#57614.
Change-Id: Id55ba25a7ee4a803788316d4a646cd4b6f4297e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/460655 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com> Reviewed-by: Richard Musiol <neelance@gmail.com> Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Keith Randall [Tue, 22 Nov 2022 06:22:36 +0000 (22:22 -0800)]
cmd/compile: add anchored version of SP
The SPanchored opcode is identical to SP, except that it takes a memory
argument so that it (and more importantly, anything that uses it)
must be scheduled at or after that memory argument.
This opcode ensures that a LEAQ of a variable gets scheduled after the
corresponding VARDEF for that variable.
This may lead to less CSE of LEAQ operations. The effect is very small.
The go binary is only 80 bytes bigger after this CL. Usually LEAQs get
folded into load/store operations, so the effect is only for pointerful
types, large enough to need a duffzero, and have their address passed
somewhere. Even then, usually the CSEd LEAQs will be un-CSEd because
the two uses are on different sides of a function call and the LEAQ
ends up being rematerialized at the second use anyway.
Change-Id: Ib893562cd05369b91dd563b48fb83f5250950293
Reviewed-on: https://go-review.googlesource.com/c/go/+/452916
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com> Reviewed-by: Martin Möhrmann <martin@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Dmitri Shuralyov [Sun, 15 May 2022 02:45:05 +0000 (22:45 -0400)]
cmd/compile/internal/ssa: generate code via a //go:generate directive
The standard way to generate code in a Go package is via //go:generate
directives, which are invoked by the developer explicitly running:
go generate import/path/of/said/package
Switch to using that approach here.
This way, developers don't need to learn and remember a custom way that
each particular Go package may choose to implement its code generation.
It also enables conveniences such as 'go generate -n' to discover how
code is generated without running anything (this works on all packages
that rely on //go:generate directives), being able to generate multiple
packages at once and from any directory, and so on.
Change-Id: I0e5b6a1edeff670a8e588befeef0c445613803c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/460135 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Russ Cox [Thu, 12 Jan 2023 14:30:38 +0000 (09:30 -0500)]
runtime: replace panic(nil) with panic(new(runtime.PanicNilError))
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for #56986.
Russ Cox [Mon, 16 Jan 2023 13:02:34 +0000 (08:02 -0500)]
cmd/go/internal/modindex: remove copy of build.Package
modindex defines its own type Package that must be exactly the
same as build.Package (there are conversions between them).
The normal reason to do this is to provide a different method set,
but there aren't any different methods. And if we needed to do that,
we could write
type Package build.Package
instead of repeating the struct definition. Remove the type entirely
in favor of direct use of build.Package.
This makes the modindex package not break when fields are
added to go/build.Package.
Bryan C. Mills [Tue, 10 Jan 2023 20:59:12 +0000 (15:59 -0500)]
cmd/go: shorten TestScript/test_shuffle and skip it in short mode
test_shuffle was added in CL 310033. It takes about 4½ seconds on my
workstation prior to this CL, most of which is spent linking and
running test binaries in 'go test'.
We can reduce that time somewhat (to 3¾ seconds) by simply running the
test fewer times (cases of 'off', 'on', positive, zero, and negative
values seem sufficient), but we should also avoid that linking
overhead at all in short mode.
Bryan C. Mills [Tue, 10 Jan 2023 17:59:16 +0000 (12:59 -0500)]
cmd/go/internal/lockedfile: avoid failing tests due to arbitrary timeouts
The mustBlock helper returns a function that verifies that the blocked
operation does eventually complete. However, it used an arbitrary
10-second timeout in place of “eventually”.
Since the test is checking a synchronization library, bugs are likely
to manifest as deadlocks. It may be useful to log what operation is in
flight if such a deadlock occurs; however, since we can't bound how
long a “reasonable” operation should take, the log message should only
be informational — it should not cause the test to fail.
While we're here, let's also set a better example by not leaking
time.After timers in the tests..
This uses the new Cancel and WaitDelay fields of os/exec.Cmd
(added in #50436) to interrupt or kill the 'hg serve' command
when its incoming http.Request is canceled.
This should keep the vcweb hg handler from getting stuck if 'hg serve'
hangs after the request either completes or is canceled.
Bryan C. Mills [Thu, 5 Jan 2023 19:00:25 +0000 (14:00 -0500)]
net: delete TestTCPSelfConnect
This test is flaky, apparently due to a typo'd operator in CL 21447
that causes it to compare “same port OR IP” instead of
“same port AND IP”.
If we merely fixed the comparison, the test would hopefully stop being
flaky itself, but we would still be left with another problem:
repeatedly dialing a port that we believe to be unused can interfere
with other tests, which may open the previously-unused port and then
attempt a single Dial and expect it to succeed. Arbitrary other Dial
calls for that port may cause the wrong connection to be accepted,
leading to spurious test failures.
Moreover, the test can be extremely expensive for the amount of data
we hope to get from it, depending on the system's port-reuse
algorithms and dial implementations. It is already scaled back by up
to 1000x on a huge number of platforms due to latency, and may even be
ineffective on those platforms because of the arbitrary 1ms Dial
timeout. And the incremental value from it is quite low, too: it tests
the workaround for what is arguably a bug in the Linux kernel, which
ought to be fixed (and tested) upstream instead of worked around in
every open-source project that dials local ports.
Instead of trying to deflake this test, let's just get rid of it.
Bryan C. Mills [Thu, 8 Dec 2022 13:44:51 +0000 (08:44 -0500)]
runtime: remove arbitrary GOARCH constraints in finalizer tests
These tests were only run on GOARCH=amd64, but the rationale given in
CL 11858043 was GC precision on 32-bit platforms. Today, we have far
more 64-bit platforms than just amd64, and I believe that GC precision
on 32-bit platforms has been substantially improved as well.
The GOARCH restriction seems unnecessary.
Bryan C. Mills [Tue, 13 Dec 2022 19:19:59 +0000 (14:19 -0500)]
os: deflake TestPipeEOF and TestFifoEOF
- Consolidate the two test bodies as one helper function.
- Eliminate arbitrary timeout.
- Shorten arbitrary sleeps in short mode.
- Simplify goroutines.
- Mark the tests as parallel.
Fixes #36107.
Updates #24164.
Change-Id: I14fe4395963a7256cb6d2d743d348a1ade077d5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/457336 Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Joe Tsai [Wed, 18 Jan 2023 18:22:54 +0000 (10:22 -0800)]
time: revert strict parsing of RFC 3339
CL 444277 fixed Time.UnmarshalText and Time.UnmarshalJSON to properly
unmarshal timestamps according to RFC 3339 instead of according
to Go's bespoke time syntax that is a superset of RFC 3339.
However, this change seems to have broken an AWS S3 unit test
that relies on parsing timestamps with single digit hours.
It is unclear whether S3 emits these timestamps in production or
whether this is simply a testing artifact that has been cargo culted
across many code bases. Either way, disable strict parsing for now
and re-enable later with better GODEBUG support.
Updates #54580
Change-Id: Icced2c7f9a6b2fc06bbd9c7e90f90edce24c2306
Reviewed-on: https://go-review.googlesource.com/c/go/+/462286 Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Wed, 18 Jan 2023 15:08:34 +0000 (10:08 -0500)]
cmd/go: do not attempt to install cmd/addr2line in TestScript/mod_outside
Tests must not write to GOROOT: it might not writable (for example, if
it is owned by root and the user is non-root), and in general we can't
assume that the configuration in which the test is run matches the
configuration with which the installed tools were built.
In this specific case, CL 454836 (for #57007) installs 'cmd' with
CGO_ENABLED=0, but most builders still run the tests with CGO_ENABLED
unset.
Ian Lance Taylor [Wed, 18 Jan 2023 00:14:05 +0000 (16:14 -0800)]
misc/reboot: overlay $GOROOT/lib in temporary goroot
This fixes the test after CL 455357, which builds the time/tzdata file
from $GOROOT/lib/time/zoneinfo.zip.
Change-Id: I0c5afa6521b58dd3b57c3b4c3c704a622b846382
Reviewed-on: https://go-review.googlesource.com/c/go/+/462279
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Ian Lance Taylor <iant@golang.org> Reviewed-by: Than McIntosh <thanm@google.com>
Russ Cox [Sat, 14 Jan 2023 21:40:22 +0000 (16:40 -0500)]
cmd/go: introduce GOROOT/go.env and move proxy/sumdb config there
Various Linux distributions edit cmd/go/internal/cfg/cfg.go to change
the default settings of GOPROXY and GOSUMDB. Make it possible for
them to do this without editing the go command source code by
introducing GOROOT/go.env and moving those defaults there.
With the upcoming changes for reproducible builds (#24904),
this should mean that Linux distributions distribute binaries
that are bit-for-bit identical to the Go distribution binaries,
even when rebuilding the distribution themselves.
Russ Cox [Fri, 2 Dec 2022 15:39:30 +0000 (10:39 -0500)]
cmd/dist: make toolchain build reproducible
- Build cmd with CGO_ENABLED=0. Doing so removes the C compiler
toolchain from the reproducibility perimeter and also results in
cmd/go and cmd/pprof binaries that are statically linked,
so that they will run on a wider variety of systems.
In particular the Linux versions will run on Alpine and NixOS
without needing a simulation of libc.so.6.
The potential downside of disabling cgo is that cmd/go and cmd/pprof
use the pure Go network resolver instead of the host resolver on
Unix systems. This means they will not be able to use non-DNS
resolver mechanisms that may be specified in /etc/resolv.conf,
such as mDNS. Neither program seems likely to need non-DNS names
like those, however.
macOS and Windows systems still use the host resolver, which they
access without cgo.
- Build cmd with -trimpath when building a release.
Doing so removes $GOPATH from the file name prefixes stored in the
binary, so that the build directory does not leak into the final artifacts.
- When CC and CXX are empty, do not pick values to hard-code into
the source tree and binaries. Instead, emit code that makes the
right decision at runtime. In addition to reproducibility, this
makes cross-compiled toolchains work better. A macOS toolchain
cross-compiled on Linux will now correctly look for clang,
instead of looking for gcc because it was built on Linux.
- Convert \ to / in file names stored in .a files.
These are converted to / in the final binaries, but the hashes of
the .a files affect the final build ID of the binaries. Without this
change, builds of a Windows toolchain on Windows and non-Windows
machines produce identical binaries except for the input hash part
of the build ID.
- Due to the conversion of \ to / in .a files, convert back when
reading inline bodies on Windows to preserve output file names
in error messages.
Combined, these four changes (along with Go 1.20's removal of
installed pkg/**.a files and conversion of macOS net away from cgo)
make the output of make.bash fully reproducible, even when
cross-compiling: a released macOS toolchain built on Linux or Windows
will contain exactly the same bits as a released macOS toolchain
built on macOS.
The word "released" in the previous sentence is important.
For the build IDs in the binaries to work out the same on
both systems, a VERSION file must exist to provide a consistent
compiler build ID (instead of using a content hash of the binary).
Russ Cox [Fri, 9 Dec 2022 20:09:46 +0000 (15:09 -0500)]
make.bash, make.rc: fix GOROOT detection when GOEXPERIMENT is set
We need to clear GOEXPERIMENT any time we are invoking a bootstrap
toolchain. One line missed the clearing of GOEXPERIMENT.
There were three different lines using different syntaxes and subtly
different sets of variables being cleared, so hoist them into a function
so it's all in one place.
Russ Cox [Tue, 6 Dec 2022 04:39:49 +0000 (23:39 -0500)]
time/tzdata: generate zip constant during cmd/dist
We have a make.bash-time generation capability,
so use it to generate the embedded zip file for time/tzdata.
This is one less file to try to review in CLs like CL 455356.
For #22487.
Fixes #43350.
Change-Id: I2fcd0665fa0b1c830baec5fb4cd714483fea25a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/455357
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Russ Cox [Fri, 13 Jan 2023 15:21:56 +0000 (10:21 -0500)]
cmd/go: do not confuse files for standard library packages
I often create dummy files holding various data named things like 'z'.
If a file (not directory) GOROOT/src/z exists, it confuses cmd/go into
thinking z is a standard library package, which breaks the test
Script/mod_vendor.
This CL fixes internal/goroot to only report that something is a standard
library package when a directory with that name exists, not just a file.
Matthew Dempsky [Tue, 17 Jan 2023 18:59:25 +0000 (10:59 -0800)]
cmd/compile: fix static init inlining for hidden node fields
Unified IR added several new IR fields for holding *runtime._type
expressions. To avoid throwing off any frontend semantics
(particularly inlining cost heuristics), they were marked as
`mknode:"-"` so that code wouldn't visit them.
Unfortunately, this has a bad interaction with the static init
inlining optimization, because the latter relies on ir.EditChildren to
substitute all parameters. This potentially includes dictionary
parameters, which can appear within the new RType fields.
This CL adds a new ir.EditChildrenWithHidden function that also edits
these fields, and switches staticinit to use it. Longer term, we
should unhide the RType fields so that ir.EditChildren visits them
normally, but that's scarier so late in the release cycle.
Fixes #57778.
Change-Id: I98c1e8cf366156dc0c81a0cb79029cc5e59c476f
Reviewed-on: https://go-review.googlesource.com/c/go/+/461686
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Robert Griesemer [Wed, 11 Jan 2023 17:17:55 +0000 (09:17 -0800)]
go/types, types2: factor out position comparison, share more code
This CL introduces the new files util.go and util_test.go for both
type checkers; these files factor out functionality that is different
between the type checkers so that more code (that is otherwise mostly
the same) can be generated.
With cmpPos/CmpPos factored out, go/types/scope.go can now be generated.
Change-Id: I35f67e53d83b3c5086a559b1e826db83d38ee217
Reviewed-on: https://go-review.googlesource.com/c/go/+/461596
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Mon, 9 Jan 2023 21:02:08 +0000 (13:02 -0800)]
go/types: make tracing configurable (matching types2)
This CL replaces the internal trace flag with Config.trace.
While unexported, it can still be set for testing via reflection.
The typical use is for manual tests, where -v (verbose) turns on
tracing output. Typical use:
go test -run Manual -v
This change makes go/types match types2 behavior.
Change-Id: I22842f4bba8fd632efe5929c950f4b1cab0a8569
Reviewed-on: https://go-review.googlesource.com/c/go/+/461081
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
Robert Griesemer [Thu, 1 Dec 2022 22:29:11 +0000 (14:29 -0800)]
go/types, types2: do not abort constraint type inference eagerly
During constraint type inference, unification may fail because it
operates with limited information (core types) even if the actual
type argument satisfies the type constraint in question.
On the other hand, it is safe to ignore failing unification during
constraint type inference because if the failure is true, an error
will be reported when checking instantiation.
Fixes #53650.
Change-Id: Ia76b21ff779bfb1282c1c55f4174847b29cc6f3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/454655
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Fri, 9 Dec 2022 00:20:37 +0000 (16:20 -0800)]
go/types, types2: distinguish between substring and regexp error patterns
Use ERROR for substrings, and ERRORx for regexp error patterns.
Correctly unquote patterns for ERROR and ERRORx.
Adjust all tests in internal/types/testdata and locally as needed.
The changes to internal/types/testdata were made through
repeated applications of regexpr find/replace commands
and manual cleanups.
Fixes #51006.
Change-Id: Ib9ec5001243b688bf5aee56b7d4105fb55999ab4
Reviewed-on: https://go-review.googlesource.com/c/go/+/455755 Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 6 Dec 2022 22:43:39 +0000 (14:43 -0800)]
internal/types: consistently use double quotes around ERROR patterns
Before matching the pattern, the double quotes are simply stripped
(no Go string unquoting) for now. This is a first step towards use
of proper Go strings as ERROR patterns.
The changes were obtained through a couple of global regexp
find/replace commands:
followed up by manual fixes where multiple "/* ERROR"-style
errors appeared on the same line (in that case, the first
regexp matches the first and last ERROR).
For #51006.
Change-Id: Ib92c2d5e339075aeec1ea74c339b5fecf953d1a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/455718
Auto-Submit: Robert Griesemer <gri@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Robert Griesemer [Thu, 8 Dec 2022 04:40:37 +0000 (20:40 -0800)]
go/types: use commentMap to collect error comments
Adjust the testFiles function to use the new commentMap
function. This makes it possible for testFiles to match
the types2.TestFiles logic more closely.
For #51006.
Change-Id: I6c5ecbeb86d095404ec04ba4452fb90d404b8280
Reviewed-on: https://go-review.googlesource.com/c/go/+/456137 Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Robert Griesemer [Thu, 8 Dec 2022 16:29:14 +0000 (08:29 -0800)]
go/types: add commentMap and test
This adds the (adjusted) syntax.CommentMap function and corresponding
test to the types_test package so that we can use it for collecting
ERROR comments in the next CL.
For #51006.
Change-Id: I63ce96e7394c28c02d5a292250586cc49c1f6e19
Reviewed-on: https://go-review.googlesource.com/c/go/+/456125 Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Cherry Mui [Tue, 17 Jan 2023 19:22:33 +0000 (14:22 -0500)]
internal/goversion: update Version to 1.21
This is the start of the Go 1.21 development cycle, so update the
Version value accordingly. It represents the Go 1.x version that
will soon open up for development (and eventually become released).
For #40705.
For #57736.
Change-Id: I31b739f632bdc8d14f46560e0e5bf333fb8e7740
Reviewed-on: https://go-review.googlesource.com/c/go/+/462456 Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
We need to avoid nospill registers at this point in regalloc.
Make sure that we don't restrict our register set to avoid registers
desired by other instructions, if the resulting set includes only
nospill registers.
Fixes #57846
Change-Id: I05478e4513c484755dc2e8621d73dac868e45a27
Reviewed-on: https://go-review.googlesource.com/c/go/+/461685 Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
fangguizhen [Tue, 17 Jan 2023 16:37:42 +0000 (16:37 +0000)]
strings: remove redundant symbols
Change-Id: Ie3fe0274288d6cb6303acdcec1340c480e5c0b20
GitHub-Last-Rev: ce9d44619e970b1319fbccf3aace1ddf719bcec1
GitHub-Pull-Request: golang/go#57848
Reviewed-on: https://go-review.googlesource.com/c/go/+/462277
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Than McIntosh [Fri, 13 Jan 2023 21:46:49 +0000 (16:46 -0500)]
cmd/go: include coverage build flags for "go list"
This patch ensures that the go command's "list" subcommand accepts
coverage-related build options, which were incorrectly left out when
"go build -cover" was rolled out. This is needed in order to do things
like check the staleness of an installed cover-instrumented target.
Filippo Valsorda [Wed, 4 Jan 2023 20:51:50 +0000 (21:51 +0100)]
crypto/x509: clarify that CheckSignatureFrom and CheckSignature are low-level APIs
In particular, CheckSignatureFrom just can't check the path length
limit, because it might be enforced above the parent.
We don't need to document the supported signature algorithms for
CheckSignatureFrom, since we document at the constants in what contexts
they are allowed and not. That does leave CheckSignature ambiguous,
though, because that function doesn't have an explicit context.
Russ Cox [Sat, 14 Jan 2023 20:08:49 +0000 (15:08 -0500)]
cmd/go: document GODEBUG=installgoroot=all
At the moment the only documentation is the release notes,
but everything mentioned in the release notes should have
proper documentation separate from them.
Russ Cox [Tue, 17 Jan 2023 14:21:43 +0000 (09:21 -0500)]
doc/go1.20: remove mention of arena goexperiment
The arena goexperiment contains code used inside Google in very
limited use cases that we will maintain, but the discussion on #51317
identified serious problems with the very idea of adding arenas to the
standard library. In particular the concept tends to infect many other
APIs in the name of efficiency, a bit like sync.Pool except more
publicly visible.
It is unclear when, if ever, we will pick up the idea and try to push
it forward into a public API, but it's not going to happen any time
soon, and we don't want users to start depending on it: it's a true
experiment and may be changed or deleted without warning.
The arena text in the release notes makes them seem more official
and supported than they really are, and we've already seen a couple
blog posts based on that erroneous belief. Delete the text to try to
set expectations better.
Archana R [Wed, 11 Jan 2023 17:45:28 +0000 (11:45 -0600)]
runtime: fix performance regression in morestack_noctxt on ppc64
In the fix for 54332 the MOVD R1, R1 instruction was added to
morestack_noctxt function to set the SPWRITE bit. However, the
instruction MOVD R1, R1 results in or r1,r1,r1 which is a special
instruction on ppc64 architecture as it changes the thread priority
and can negatively impact performance in some cases.
More details on such similar nops can be found in Power ISA v3.1
Book II on Power ISA Virtual Environment architecture in the chapter
on Program Priority Registers and Or instructions.
Replacing this by OR R0, R1 has the same affect on setting SPWRITE as
needed by the first fix but does not affect thread priority and
hence does not cause the degradation in performance
Robert Findley [Wed, 11 Jan 2023 19:41:03 +0000 (14:41 -0500)]
go/types, types2: don't look up fields or methods when expecting a type
As we have seen many times, the type checker must be careful to avoid
accessing named type information before the type is fully set up. We
need a more systematic solution to this problem, but for now avoid one
case that causes a crash: checking a selector expression on an
incomplete type when a type expression is expected.
For golang/go#57522
Change-Id: I7ed31b859cca263276e3a0647d1f1b49670023a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/461577
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Robert Griesemer [Wed, 4 Jan 2023 01:05:53 +0000 (17:05 -0800)]
cmd/compile: better error message for when a type is in a constraint but not the type set
While at it, also remove the word "constraint" in the detail explanation
of an unsatisfied constraint.
Fixes #57500.
Change-Id: I55dae1694de2cfdb434aeba9d4a3530af7aca8f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460455 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Russ Cox [Wed, 4 Jan 2023 14:21:14 +0000 (09:21 -0500)]
cmd/link, runtime: Apple libc atfork workaround take 3
CL 451735 worked around bugs in Apple's atfork handlers by calling
notify_is_valid_token and xpc_atfork_child at startup, so that init
code that wouldn't be safe in the child process would be warmed up in
the parent process instead, but xpc_atfork_child broke use of the xpc
library in Go programs, and xpc is internally used by various macOS
frameworks (#57263).
CL 459175 reverted that change, and then CL 459176 tried a new
approach: use __fork, which doesn't call any of the atfork handlers at all.
That worked, but an Apple engineer reviewing the change in private
email suggests that since __fork is not public API, it should be avoided.
The same engineer (with access to the source code for the xpc library)
suggests that the breakage in #57263 is caused by xpc_atfork_child
marking the library as unusable, expecting an imminent call to exec,
and that calling xpc_date_create_from_current instead would do the
necessary initialization without marking xpc as unusable.
CL 460475 reverted that change, to prepare for this one.
This CL goes back to the original “call functions to warm things up”
approach, replacing xpc_atfork_child with xpc_date_create_from_current.
The CL also updates cmd/link to use OS and SDK version 10.13.0 for
x86 macOS binaries, up from 10.9.0, also suggested by the Apple engineer.
Combined with the two warmup calls, this makes the fork hangs go away.
The minimum macOS version has been 10.13 High Sierra since Go 1.17,
so there should be no problem with writing that in the binaries too.
Russ Cox [Wed, 4 Jan 2023 14:18:02 +0000 (09:18 -0500)]
runtime: revert use of __fork to work around Apple atfork bugs
An Apple engineer suggests that since __fork is not public API,
it would be better to use a different fix. With the benefit of source code,
they suggest using xpc_date_create_from_current instead of
xpc_atfork_child. The latter sets some flags that disable certain
functionality for the remainder of the process lifetime (expecting exec),
while the former should do the necessary setup.
Reverting the __fork fix in order to prepare a clean fix based
on CL 451735 using xpc_date_create_from_current.
Bryan C. Mills [Tue, 10 Jan 2023 16:29:25 +0000 (11:29 -0500)]
os/exec: avoid leaking an exec.Cmd in TestWaitInterrupt
In CL 436655 I added a GODEBUG setting to this test process to verify
that Wait is eventually called for every exec.Cmd before it becomes
unreachable. However, the cmdHang test helpers in
TestWaitInterrupt/Exit-hang and TestWaitInterrupt/SIGKILL-hang
intentially leak a subprocess in order to simulate a leaky third-party
program, as Go users might encounter in practical use.
To avoid tripping over the leak check, we call Wait on the leaked
subprocess in a background goroutine. Since we expect the process
running cmdHang to exit before its subprocess does, the call to Wait
should have no effect beyond suppressing the leak check.
Keith Randall [Mon, 9 Jan 2023 17:49:32 +0000 (09:49 -0800)]
cmd/compile: prevent IsNewObject from taking quadratic time
As part of IsNewObject, we need to go from the SelectN[0] use of
a call to the SelectN[1] use of a call. The current code does this
by just looking through the block. If the block is very large,
this ends up taking quadratic time.
Instead, prepopulate a map from call -> SelectN[1] user of that call.
That lets us find the SelectN[1] user in constant time.
Fixes #57657
Change-Id: Ie2e0b660af5c080314f4f17ba2838510a1147f9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/461080
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Michael Pratt [Mon, 9 Jan 2023 18:59:54 +0000 (13:59 -0500)]
cmd/compile/internal/pgo: add hint to missing start_line error
Profiles only began adding Function.start_line in 1.20. If it is
missing, add a hint to the error message that they may need to profile a
build of the application built with a newer version of the toolchain.
Technically profiles are not required to come from Go itself (e.g., they
could be converted from perf), but in practice they most likely are.
Fixes #57674.
Change-Id: I87eca126d3fed0cff94bbb8dd748bd4652f88b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/461195
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Ian Lance Taylor [Thu, 5 Jan 2023 02:03:46 +0000 (18:03 -0800)]
runtime: skip TestCgoPprofCallback in short mode, don't run in parallel
Fixes #54778
Change-Id: If9aef0c06b993ef2aedbeea9452297ee9f11fa06
Reviewed-on: https://go-review.googlesource.com/c/go/+/460461 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Austin Clements [Thu, 5 Jan 2023 14:08:25 +0000 (09:08 -0500)]
runtime/pprof: document possibility of empty stacks
I spent quite a while determining the cause of empty stacks in
profiles and reasoning out why this is okay. There isn't a great place
to record this knowledge, but a documentation comment on
appendLocsForStack is better than nothing.
Updates #51550.
Change-Id: I2eefc6ea31f1af885885c3d96199319f45edb4ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/460695 Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Michael Pratt <mpratt@google.com>
Austin Clements [Wed, 4 Jan 2023 17:31:23 +0000 (12:31 -0500)]
runtime/pprof: improve output of TestLabelSystemstack
The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.
Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):
1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.
2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.
3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.
Fixes #51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.
Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com> Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com> Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>