This pulls in the changes to remove 1.18 support in counter and
countertest, to add counter.CountCommandLineFlags, and to add
countertest.SupportedPlatform
Commands run:
go get golang.org/x/telemetry@abedc37
go mod tidy
go mod vendor
Change-Id: I5c17c5b3ca38df14883ba43316d59437a737b28b
Reviewed-on: https://go-review.googlesource.com/c/go/+/571801
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Paul E. Murphy [Thu, 7 Mar 2024 21:37:14 +0000 (15:37 -0600)]
cmd/compile/internal: generate ADDZE on PPC64
This usage shows up in quite a few places, and helps reduce
register pressure in several complex cryto functions by
removing a MOVD $0,... instruction.
Change-Id: I9444ea8f9d19bfd68fb71ea8dc34e109681b3802
Reviewed-on: https://go-review.googlesource.com/c/go/+/571055
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Ian Lance Taylor [Fri, 15 Mar 2024 04:22:31 +0000 (21:22 -0700)]
net: #define _GNU_SOURCE to 1
Makes the build work with CGO_CPPFLAGS=-D_GNU_SOURCE,
as reportedly used by TinyGo.
Fixes #66325
Change-Id: I794f1cd89814638fdb6c3066d13bbd7da88c9d93
Reviewed-on: https://go-review.googlesource.com/c/go/+/571875
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Commit-Queue: Ian Lance Taylor <iant@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Russ Cox [Fri, 15 Mar 2024 16:56:05 +0000 (12:56 -0400)]
net/http: revert header changes in Error
This reverts CL 544019 and CL 569815, because they break a variety
of tests inside Google that do not expect the Cache-Control header
to be set to no-cache.
A followup CL will add this functionality back after a proposal.
For #50905.
Change-Id: Ie377bfb72ce2c77d11bf31f9617ab6db342a408a
Reviewed-on: https://go-review.googlesource.com/c/go/+/571975
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
Russ Cox [Thu, 14 Mar 2024 20:40:34 +0000 (16:40 -0400)]
runtime: fixes to traceback_system_test.go
Minor cleanups to CL 561635's test for better debuggability
when it crashes. In a separate CL so that it's clear this CL is
not changing the code under test.
Change-Id: I12b72ae538f8454b5c382127eafd766c22c69b67
Reviewed-on: https://go-review.googlesource.com/c/go/+/571799 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Than McIntosh [Wed, 13 Mar 2024 17:49:32 +0000 (17:49 +0000)]
cmd/link: support -bindnow option and permit use of "-Wl,-z,now"
This is a partial roll-forward of CL 473495, which was subsequently
reverted. The second half of CL 473495 will appear in a future CL.
In this patch we introduce a new Go linker "-bindnow" command line
flag, and update the Go command to permit the use of the -Wl,-z,now
option, to allow users to produce binaries that have immediate
binding.
Updates #45681.
Change-Id: Idd61b0d6597bcd37b16c343714c55a4ef6dfb534
Reviewed-on: https://go-review.googlesource.com/c/go/+/571416 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
apocelipes [Thu, 14 Mar 2024 12:55:10 +0000 (12:55 +0000)]
internal/bisect: replace atomicPointerDedup to simplify the code
"atomicPointerDedup" is a redundancy of "atomic.Pointer".
Since Go 1.22 now requires the final point release of Go 1.20 or
later for bootstrap, Go 1.19's atomic.Pointer can be used
without problems.
atomicPointerDedup is unnecessary and we can remove it now.
Change-Id: I0a65ad0b6649cecb73d58dc39c5fd736390d5fa5
GitHub-Last-Rev: 6c6e9421fbdf34c2d4b3ea21359f847ccf9a34cd
GitHub-Pull-Request: golang/go#65987
Reviewed-on: https://go-review.googlesource.com/c/go/+/567656 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Robert Griesemer [Thu, 14 Mar 2024 22:45:30 +0000 (15:45 -0700)]
go/types, types2: do not overwrite nest entries in Checker.validType
In Checker.validType, when we encounter a type parameter, we evaluate
the validity of the respective type argument in the "type nest" of the
enclosing type (at the nesting depth at which the type argument was
passed) (*). Specifically, we call validType recursively, with the slice
representing the type nest shortened by 1. This recursive call continues
to use the nest slice and in the process may overwrite the (previously)
last entry. Upon return of that recursive call, validType proceeds with
the old length, possibly using an incorrect last nest entry.
In the concrete example for this issue we have the type S
type S[T any] struct {
a T
b time.Time
}
instantiated with time.Time. When validType encounters the type parameter
T inside the struct (S is in the type nest) it evaluates the type argument
(time.Time) in the empty type nest (outside of S). In the process of
evaluating the time.Time struct, the time.Time type is appended to the
(shortened) nest slice and overwrites the previous last nest entry (S).
Once processing of T is done, validType continues with struct field b,
using the original-length nest slice, which now has time.Time rather
than S as a last element. The type of b has type time.Time, which now
appears to be nested in time.Time (rather than S), which (incorrectly)
means that there's a type cycle. validType proceeds with reporting the
error. But time.Time is an imported type, imported types are correct
(otherwise they could not be imported in the first place), and the
assertion checking that package of time.Time is local fails.
The fix is trivial: restore the last entry of the nest slice when it
may have been overwriten.
(*) In hindsight we may be able to sigificantly simplify validType by
evaluating type arguments when they are passed instead of when
the respective type parameters are encountered. For another CL.
Fixes #66323.
Change-Id: I3bf23acb8ed14d349db342ca5c886323a6c7af58
Reviewed-on: https://go-review.googlesource.com/c/go/+/571836 Reviewed-by: Russ Cox <rsc@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Quentin McGaw [Wed, 29 Nov 2023 21:44:34 +0000 (21:44 +0000)]
net: fixes to dnsReadConfig in dnsconfig_windows.go
- Only search DNS servers for network interfaces with at least one gateway
- Clarify comment on deprecated site local anycast fec0/10 DNS IPv6 addresses
- Minor maintenance: skip not "up" interfaces earlier in outer loop
Change-Id: I98ca7b81d3d51e6aa6bfa4a10dcd651305a843df
GitHub-Last-Rev: 3b358c7e3f89971d069286f997dc19e092ec8f08
GitHub-Pull-Request: golang/go#64441
Reviewed-on: https://go-review.googlesource.com/c/go/+/545775
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Peter Collingbourne [Wed, 13 Mar 2024 03:01:40 +0000 (20:01 -0700)]
debug/elf: avoid using binary.Read() in NewFile()
With this change my test program that reads a tree of ELF files runs
1.71 ± 0.12 times faster without parallelism or 1.39 ± 0.06 times
faster using 8 goroutines.
Change-Id: I443d1a02736f16f5532ef28e1447c97aa87c7126
Reviewed-on: https://go-review.googlesource.com/c/go/+/571436
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Robert Griesemer [Wed, 13 Mar 2024 15:56:06 +0000 (08:56 -0700)]
go/types, types2: consistently report "duplicate method" error in go1.13
Go 1.13 is not supported anymore, but this CL removes an unnecessary
check and in turn fixes an old bug.
Fixes #66285.
Change-Id: I15ee1712b31f8ac8c915f18410d99cbf44334d35
Reviewed-on: https://go-review.googlesource.com/c/go/+/571058 Reviewed-by: Robert Griesemer <gri@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Robert Griesemer [Tue, 12 Mar 2024 16:31:06 +0000 (09:31 -0700)]
go/types, types2: don't do version checks for embedded types of imported interfaces
This is a cherry-pick of CL 571075 combined with adjustments for 1.23:
Imported interfaces don't have position information for embedded types.
When computing the type set of such interfaces, doing a version check
may fail because it will rely on the Go version of the current package.
We must not do a version check for features of types from imported
packages - those types have already been typechecked and are "correct".
The version check code does look at packages to avoid such incorrect
version checks, but we don't have the package information available
in an interface type (divorced from its object).
Instead, rely on the fact that imported interfaces don't have position
information for embedded types: if the position is unknown, don't do a
version check.
In Checker.allowVersion, still allow for unknown positions and resort
to the module version in that case (source code may be generated by
tools and not contain position information). Also, remove the *Package
argument as it was always check.pkg except in one case, and that case
may in fact be incorrect; treat that case separately for now.
Fixes #66064.
Change-Id: I773d57e5410c3d4a911ab3e018b3233c2972b3c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/571075 Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/571137
Michael Matloob [Wed, 13 Mar 2024 17:27:03 +0000 (13:27 -0400)]
cmd/internal/telemetry: add a shim package around telemetry
The purpose of this package is to have a build tagged variant so that
when we're building the bootstrap go command it does not depend on the
net package. (net is a dependency of golang.org/x/telemetry/counter on
Windows).
The TESTGO_TELEMETRY_DIR environment variable used by the go tests to
change the telemetry directory is renamed to TEST_TELEMETRY_DIR to
make it more general to other commands that might want to set it for
the purpose of tests. The test telemetry directory is now set using
telemetry.Start instead of countertest.Open. This also means that the
logic that decides whether to upload counter files is now going to run
from the cmd/go tests (but that's okay because it's aleady been
running when cmd/go has been invoked outside of its tests.
Change-Id: Ic4272e5083facde010482d8b8fc3c95c03564bc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/571096
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
apocelipes [Thu, 14 Mar 2024 10:41:08 +0000 (10:41 +0000)]
net/netip: use built-in clear to simplify code
Change-Id: Ic7b390935df107c5b7f53f9347a52031eac8a897
GitHub-Last-Rev: a7194571e1c2f90537f1caa8a3b5bcd60cea60be
GitHub-Pull-Request: golang/go#66310
Reviewed-on: https://go-review.googlesource.com/c/go/+/571635
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: qiulaidongfeng <2645477756@qq.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Reason for revert: This works for telemetry but broke various other
properties of the tracebacks as well as some programs that read
tracebacks. We should figure out a solution that works for all uses,
and in the interim we should not be making telemetry work at the
cost of breaking other, existing valid uses.
See #65761 for details.
Change-Id: I467993ae778887e5bd3cca4c0fb54e9d44802ee1
Reviewed-on: https://go-review.googlesource.com/c/go/+/571797
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Russ Cox [Fri, 1 Mar 2024 03:39:49 +0000 (22:39 -0500)]
time: avoid stale receives after Timer/Ticker Stop/Reset return
A proposal discussion in mid-2020 on #37196 decided to change
time.Timer and time.Ticker so that their Stop and Reset methods
guarantee that no old value (corresponding to the previous configuration
of the Timer or Ticker) will be received after the method returns.
The trivial way to do this is to make the Timer/Ticker channels
unbuffered, create a goroutine per Timer/Ticker feeding the channel,
and then coordinate with that goroutine during Stop/Reset.
Since Stop/Reset coordinate with the goroutine and the channel
is unbuffered, there is no possibility of a stale value being sent
after Stop/Reset returns.
Of course, we do not want an extra goroutine per Timer/Ticker,
but that's still a good semantic model: behave like the channels
are unbuffered and fed by a coordinating goroutine.
The actual implementation is more effort but behaves like the model.
Specifically, the timer channel has a 1-element buffer like it always has,
but len(t.C) and cap(t.C) are special-cased to return 0 anyway, so user
code cannot see what's in the buffer except with a receive.
Stop/Reset lock out any stale sends and then clear any pending send
from the buffer.
Some programs will change behavior. For example:
package main
import "time"
func main() {
t := time.NewTimer(2 * time.Second)
time.Sleep(3 * time.Second)
if t.Reset(2*time.Second) != false {
panic("expected timer to have fired")
}
<-t.C
<-t.C
}
This program (from #11513) sleeps 3s after setting a 2s timer,
resets the timer, and expects Reset to return false: the Reset is too
late and the send has already occurred. It then expects to receive
two values: the one from before the Reset, and the one from after
the Reset.
With an unbuffered timer channel, it should be clear that no value
can be sent during the time.Sleep, so the time.Reset returns true,
indicating that the Reset stopped the timer from going off.
Then there is only one value to receive from t.C: the one from after the Reset.
In 2015, I used the above example as an argument against this change.
Note that a correct version of the program would be:
func main() {
t := time.NewTimer(2 * time.Second)
time.Sleep(3 * time.Second)
if !t.Reset(2*time.Second) {
<-t.C
}
<-t.C
}
This works with either semantics, by heeding t.Reset's result.
The change should not affect correct programs.
However, one way that the change would be visible is when programs
use len(t.C) (instead of a non-blocking receive) to poll whether the timer
has triggered already. We might legitimately worry about breaking such
programs.
In 2020, discussing #37196, Bryan Mills and I surveyed programs using
len on timer channels. These are exceedingly rare to start with; nearly all
the uses are buggy; and all the buggy programs would be fixed by the new
semantics. The details are at [1].
To further reduce the impact of this change, this CL adds a temporary
GODEBUG setting, which we didn't know about yet in 2015 and 2020.
Specifically, asynctimerchan=1 disables the change and is the default
for main programs in modules that use a Go version before 1.23.
We hope to be able to retire this setting after the minimum 2-year window.
Setting asynctimerchan=1 also disables the garbage collection change
from CL 568341, although users shouldn't need to know that since
it is not a semantically visible change (unless we have bugs!).
As an undocumented bonus that we do not officially support,
asynctimerchan=2 disables the channel buffer change but keeps
the garbage collection change. This may help while we are
shaking out bugs in either of them.
guoguangwu [Wed, 13 Mar 2024 01:52:03 +0000 (01:52 +0000)]
time: replace time.Now().Sub call with time.Since in test
Change-Id: I56ca2d11637d60c6b0656fdc1d900a2384aba141
GitHub-Last-Rev: 686e02db77797fd81aafcde8ae40c85cee8dd817
GitHub-Pull-Request: golang/go#66264
Reviewed-on: https://go-review.googlesource.com/c/go/+/570916 Reviewed-by: qiulaidongfeng <2645477756@qq.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
guoguangwu [Thu, 14 Mar 2024 03:16:12 +0000 (03:16 +0000)]
cmd/compile/internal/syntax: replace bytes.Compare call with bytes.Equal
Change-Id: I783e02e215efaebf4936146c6aaa032634fdfa64
GitHub-Last-Rev: 24680a73ee22fe03d7e33c122c95ed1372a2b406
GitHub-Pull-Request: golang/go#66304
Reviewed-on: https://go-review.googlesource.com/c/go/+/571595
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Russ Cox [Tue, 12 Mar 2024 16:51:44 +0000 (12:51 -0400)]
encoding/gob: make x509.Certificate marshalable again
The OID type is not exported data like most of the other x509 structs.
Using it in x509.Certificate made Certificate not gob-compatible anymore,
which breaks real-world code. As a temporary fix, make gob ignore
that field, making it work as well as it did in Go 1.21.
For Go 1.23, we anticipate adding a proper fix and removing the gob
workaround. See #65633 and #66249 for more details.
For #66249.
Fixes #65633.
Change-Id: Idd1431d15063b3009e15d0565cd3120b9fa13f61
Reviewed-on: https://go-review.googlesource.com/c/go/+/571095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Roland Shoemaker <roland@golang.org>
guoguangwu [Thu, 14 Mar 2024 03:16:23 +0000 (03:16 +0000)]
cmd/go/internal/modcmd: fix typo in comment
Change-Id: I331c46083e9608227615183ba7e25f6299669341
GitHub-Last-Rev: 0cb78ae1c1e7554b0ef54c5e82fab0901a178494
GitHub-Pull-Request: golang/go#66305
Reviewed-on: https://go-review.googlesource.com/c/go/+/571536
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Russ Cox [Wed, 13 Mar 2024 23:48:58 +0000 (19:48 -0400)]
runtime: fix lost sleep causing TestZeroTimer flakes
Classic operating system kernel mistake: if you start using
per-CPU data without disabling interrupts on the CPU,
and then an interrupt reschedules the process onto a different
CPU, now you're using the wrong CPU's per-CPU data.
The same thing happens in Go if you use per-M or per-P
data structures while not holding a lock nor using acquirem.
In the original timer.modify before CL 564977, I had been
very careful about this during the "unlock t; lock ts" dance,
only calling releasem after ts was locked. That made sure
we used the right ts. The refactoring of that code into its
own helper function in CL 564977 missed that nuance.
The code
ts := &getg().m.p.p.ptr().timers
ts.lock()
was now executing without holding any locks nor acquirem.
If the goroutine changed its M or P between deciding which
ts to use and actually locking that ts, the code would proceed
to add the timer t to some other P's timers. If the P was idle
by then, the scheduler could have already checked it for timers
and not notice the newly added timer when deciding when the
next timer should trigger.
The solution is to do what the old code correctly did, namely
acquirem before deciding which ts to use, rather than assume
getg().m.p won't change before ts.lock can complete.
This CL does that.
ran without failure for over an hour on my laptop.
Starting in CL 564977, it consistently failed within a few minutes.
After this CL, it now runs without failure for over an hour again.
Fixes #66006.
Change-Id: Ib9e7ccaa0f22a326ce3fdef2b9a92f7f0bdafcbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/571196
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Alan Donovan [Wed, 20 Sep 2023 18:13:35 +0000 (14:13 -0400)]
cmd/go/internal/test: add 'tests' vet check to 'go test' suite
The tests analyser reports structural problems in test
declarations. Presumably most of these would be caught by
go test itself, which compiles and runs (some subset of) the
tests, but Benchmark and Fuzz functions are executed less
frequently and may benefit more from static checks.
Also, reflect the change in go test help message.
+ release note
Fixes golang/go#44251
Change-Id: If5b9dee6d18fa0bc4de7f5f5f549eddeae953fc2
Reviewed-on: https://go-review.googlesource.com/c/go/+/529816 Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Dominik Honnef [Sat, 20 Jan 2024 16:37:50 +0000 (17:37 +0100)]
internal/trace/v2: support old trace format
Add support for traces from Go 1.11–1.19 by converting old traces to the
Go 1.22 format on the fly.
We import Gotraceui's trace parser, which is an optimized parser based
on Go 1.19's internal/trace package, and further modify it for the needs
of the conversion process.
With the optimized parser, loading old traces using the new API is twice
as fast and uses less total memory than 'go tool trace' did in older
versions.
The new parser does not, however, support traces from versions older
than 1.11.
This commit does not update cmd/trace to use the new API for old traces.
Change-Id: If9380aa515e29445ff624274d1760ee945ca4816
Reviewed-on: https://go-review.googlesource.com/c/go/+/557356 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Russ Cox [Thu, 15 Feb 2024 01:36:47 +0000 (20:36 -0500)]
time: garbage collect unstopped Tickers and Timers
From the beginning of Go, the time package has had a gotcha:
if you use a select on <-time.After(1*time.Minute), even if the select
finishes immediately because some other case is ready, the underlying
timer from time.After keeps running until the minute is over. This
pins the timer in the timer heap, which keeps it from being garbage
collected and in extreme cases also slows down timer operations.
The lack of garbage collection is the more important problem.
The docs for After warn against this scenario and suggest using
NewTimer with a call to Stop after the select instead, purely to work
around this garbage collection problem.
Oddly, the docs for NewTimer and NewTicker do not mention this
problem, but they have the same issue: they cannot be collected until
either they are Stopped or, in the case of Timer, the timer expires.
(Tickers repeat, so they never expire.) People have built up a shared
knowledge that timers and tickers need to defer t.Stop even though the
docs do not mention this (it is somewhat implied by the After docs).
This CL fixes the garbage collection problem, so that a timer that is
unreferenced can be GC'ed immediately, even if it is still running.
The approach is to only insert the timer into the heap when some
channel operation is blocked on it; the last channel operation to stop
using the timer takes it back out of the heap. When a timer's channel
is no longer referenced, there are no channel operations blocked on
it, so it's not in the heap, so it can be GC'ed immediately.
This CL adds an undocumented GODEBUG asynctimerchan=1
that will disable the change. The documentation happens in
the CL 568341.
Russ Cox [Mon, 11 Mar 2024 03:41:33 +0000 (23:41 -0400)]
time: clean up benchmarks
Comparing BenchmarkStop against very old commits like
CL 13094043, I was very confused about how timers had
gotten almost 10X slower since 2013.
It turns out that CL 68060043 introduced a factor of 1000
in the benchmark cost, by counting batches of 1000 as 1 op
instead of 1000 ops, and timers have actually gotten
dramatically faster since 2013, with the addition of per-P
timer heaps and other optimizations.
This CL rewrites the benchmarks to use testing.PB directly,
so that the factor of 1000 disappears, and "/op" really means "/op".
In the few tests that need to run in batches for one reason or
another, add "1000" to the name to make clear that batches
are being run.
Change-Id: I27ed74d1e420934982e4205aad4f218cdfc42509
Reviewed-on: https://go-review.googlesource.com/c/go/+/570495
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: Id7de15feabe08f66c048dc114c09494813c9febc
Reviewed-on: https://go-review.googlesource.com/c/go/+/570695 Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
kwakubiney [Mon, 11 Mar 2024 22:53:08 +0000 (22:53 +0000)]
encoding/binary: cache struct sizes to speed up Read and Write for slice of structs.
A lot of allocations happen in dataSize due to reflection.
Cache the result of the function when encoding a
slice of structs similar to what is done for struct types
so that subsequent calls to dataSize can avoid allocations.
Change-Id: I8227e61306db1fe103489ea4fee2429247c3debc
Reviewed-on: https://go-review.googlesource.com/c/go/+/570855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Michael Matloob [Mon, 11 Mar 2024 20:35:45 +0000 (16:35 -0400)]
cmd/go: change some counter names
Primarily, this change removes the cmd/ prefix on the go command
counter names. The 'error' counter is changed to 'errors' reflecting
that it's a bucket that contains multiple errors. the switch-exec and
select-exec counters are moved into a 'toolchain' grouping.
For #58894
Change-Id: Id6e0e7a0b4a5e42a0aef04b1210d2bb5256eb6c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/570736 Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Russ Cox [Sat, 9 Mar 2024 18:36:58 +0000 (13:36 -0500)]
runtime: clean up timer state
The timers had evolved to the point where the state was stored as follows:
if timer in heap:
state has timerHeaped set
if heap timer is stale:
heap deadline in t.when
real deadline in t.nextWhen
state has timerNextWhen set
else:
real deadline in t.when
t.nextWhen unset
else:
real deadline in t.when
t.nextWhen unset
That made it hard to find the real deadline and just hard to think about everything.
The new state is:
real deadline in t.when (always)
if timer in heap:
state has timerHeaped set
heap deadline in t.whenHeap
if heap timer is stale:
state has timerModified set
Separately, the 'state' word itself was being used as a lock
and state bits because the code started with CAS loops,
which we abstracted into the lock/unlock methods step by step.
At this point, we can switch to a real lock, making sure to
publish the one boolean needed by timers fast paths
at each unlock.
All this simplifies various logic considerably.
Change-Id: I35766204f7a26d999206bd56cc0db60ad1b17cbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/570335
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Russ Cox [Wed, 13 Mar 2024 02:00:22 +0000 (22:00 -0400)]
runtime: fix another lock ordering problem
https://logs.chromium.org/logs/golang/buildbucket/cr-buildbucket/8753622336585847105/+/u/step/11/log/2
shows a staticlockranking crash with pollcache (defaulted to LEAF)
being held during a write barrier, which got unlucky and acquired
wbufSpans, triggering a lock ordering throw.
My change in https://go-review.googlesource.com/c/go/+/570335/13/src/runtime/netpoll.go
around line 700 caused batching of many write barriers on the first
call rather than having just a few write barriers on each call,
making the crash much more likely, but the ordering problem
appears to have always existed. We just never allocated enough
pollDescs to trigger it.
Paul E. Murphy [Fri, 16 Feb 2024 19:29:16 +0000 (13:29 -0600)]
cmd/asm,cmd/compile: generate less instructions for most 32 bit constant adds on ppc64x
For GOPPC64 < 10 targets, most large 32 bit constants (those
exceeding int16 capacity) can be added using two instructions
instead of 3.
This cannot be done for values greater than 0x7FFF7FFF, so this
must be done during asm preprocessing as the optab matching
rules cannot differentiate this special case.
Likewise, constants 0x8000 <= x < 0x10000 are not converted. The
assembler currently generates 2 instructions sequences for these
constants.
Change-Id: I1ccc839c6c28fc32f15d286b2e52e2d22a2a06d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/568116 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>
Run-TryBot: Paul Murphy <murp@ibm.com> Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Keith Randall [Sun, 3 Mar 2024 03:22:07 +0000 (19:22 -0800)]
cmd/compile: fix sign/zero-extension removal
When an opcode generates a known high bit state (typically, a sub-word
operation that zeros the high bits), we can remove any subsequent
extension operation that would be a no-op.
x = (OP ...)
y = (ZeroExt32to64 x)
If OP zeros the high 32 bits, then we can replace y with x, as the
zero extension doesn't do anything.
However, x in this situation normally has a sub-word-sized type. The
semantics of values in registers is typically that the high bits
beyond the value's type size are junk. So although the opcode
generating x *currently* zeros the high bits, after x is rewritten to
another opcode it may not - rewrites of sub-word-sized values can
trash the high bits.
To fix, move the extension-removing rules to late lower. That ensures
that their arguments won't be rewritten to change their high bits.
I am also worried about spilling and restoring. Spilling and restoring
doesn't preserve the high bits, but instead sets them to a known value
(often 0, but in some cases it could be sign-extended). I am unable
to come up with a case that would cause a problem here, so leaving for
another time.
Fixes #66066
Change-Id: I3b5c091b3b3278ccbb7f11beda8b56f4b6d3fde7
Reviewed-on: https://go-review.googlesource.com/c/go/+/568616 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>
qmuntal [Fri, 8 Mar 2024 10:19:14 +0000 (11:19 +0100)]
internal/syscall/windows: implement SupportUnixSocket by enumerating protocols
windows.SupportUnixSocket is currently implemented using a Windows
version check. This approach is not reliable, see #27943 and #28061.
Also, it uses the undocumented RtlGetNtVersionNumbers API, which
we should try to avoid.
This PR implements SupportUnixSocket by enumerating the available
protocols and checking for AF_UNIX support.
Cq-Include-Trybots: luci.golang.try:gotip-windows-arm64
Change-Id: I76cd635067309f09571ad0eac4a5699450a2709a
Reviewed-on: https://go-review.googlesource.com/c/go/+/570075 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com> Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
apocelipes [Tue, 12 Mar 2024 10:18:00 +0000 (10:18 +0000)]
strconv: use generics to reduce redundant helper functions
Benchstat shows there are no noticeable performance changes here.
Change-Id: If2250334fe6664986f044cbaabfa1bfc84f871f7
GitHub-Last-Rev: d41a498d54483759b9c85c3d8efa848c0cc1bbd9
GitHub-Pull-Request: golang/go#66266
Reviewed-on: https://go-review.googlesource.com/c/go/+/570935
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
Bryan C. Mills [Wed, 6 Mar 2024 22:50:45 +0000 (17:50 -0500)]
cmd/go/internal/modfetch: pass "-c" arguments to git before subcommands
I accidentally transposed the arguments in CL 556358, causing the
shallow 'git fetch' attempt to always fail. That didn't break any
tests because we fall back to a full fetch, which works for nearly all
real Git servers, and we didn't have a test that checked for shallow
fetches.
Tested manually using:
GOPROXY=direct go mod download -x -json gerrit.wikimedia.org/r/mediawiki@v0.0.0-20240202145822-67da0cbcfdf7
(I'm still thinking about how to add a proper regression test.)
sivchari [Mon, 11 Mar 2024 14:28:30 +0000 (23:28 +0900)]
all: gofmt
These files are not formatted by gofmt. Thus, run gofmt to format them.
Change-Id: Iea9650e64b1f47cf82739f3a8a34f47740a96455
Reviewed-on: https://go-review.googlesource.com/c/go/+/570398 Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Jes Cok [Sun, 10 Mar 2024 12:36:44 +0000 (20:36 +0800)]
all.bash: allow spaces in $GOTOOLDIR to print build info
For consistency with all.bat: "%GOTOOLDIR%/dist" banner
Fixes #66061
Change-Id: I3387003a77a5fe82fe132e7aba472b06dd9068f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/570395
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
guoguangwu [Mon, 11 Mar 2024 11:09:50 +0000 (11:09 +0000)]
io: close PipeReader in test
Change-Id: I33858efc00dff02432f28f1e5a94aeea261a5bad
GitHub-Last-Rev: 98861f8d6e187a03330a0947ff651826024fcad2
GitHub-Pull-Request: golang/go#66230
Reviewed-on: https://go-review.googlesource.com/c/go/+/570357
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
apocelipes [Mon, 11 Mar 2024 17:41:43 +0000 (17:41 +0000)]
strconv: use slices.BinarySearch to simplify makeisprint.go
Change-Id: I9886a99f730b7616f6f8a5e6154e1beb7d3c79e6
GitHub-Last-Rev: 3f9dc7707377f79968e2dfcd206b83db21e60e60
GitHub-Pull-Request: golang/go#66242
Reviewed-on: https://go-review.googlesource.com/c/go/+/570535
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Michael Matloob [Mon, 11 Mar 2024 17:53:40 +0000 (13:53 -0400)]
cmd/go: provide a better error message when there's no go directive
On Go 1.21+ it's an error for a workspace to contain a module with a
version newer than the workspace's stated go version. If the workspace
doesn't explicitly have a go version it's explicitly 1.18. So if a
workspace without a go directive contains a module whose go directive
is newer on it's always an error for 1.21+. In the error, before this
CL the error would read "module <path> listed in go.work requires go
>= <version>, but go.work lists go 1.18". After this change the second
clause would read "but go.work implicitly requires go 1.18.
Fixes #66207
Change-Id: I44680880162a82e5cee9cfc8655d6774add6f762
Reviewed-on: https://go-review.googlesource.com/c/go/+/570735 Reviewed-by: Alan Donovan <adonovan@google.com> Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
胡玮文 [Fri, 8 Mar 2024 05:42:49 +0000 (13:42 +0800)]
net/http: support socks5h proxy schema
Extend the net/http Transport to recognize the 'socks5h' schema as an
alias for 'socks5'. Traditionally, the 'socks5h' schema indicates that
the hostname should be resolved by the proxy server, which is behavior
already implemented in Go for 'socks5'.
Fixes #24135
Change-Id: I0a6a92bbd282a3200dc4dc7b47a9b0628f931783
Reviewed-on: https://go-review.googlesource.com/c/go/+/569977
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Damien Neil <dneil@google.com>
qiulaidongfeng [Sat, 9 Mar 2024 04:09:45 +0000 (04:09 +0000)]
time: fix typo in BenchmarkReset
Change-Id: I1dbd1c5aa26f458cdac7a3f0ca974254a069311f
GitHub-Last-Rev: da481ba7a9082a5fae5cc7c72821167d9879f54f
GitHub-Pull-Request: golang/go#66219
Reviewed-on: https://go-review.googlesource.com/c/go/+/569897 Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
guoguangwu [Sat, 9 Mar 2024 03:10:36 +0000 (03:10 +0000)]
compress/gzip: close writer in test
Change-Id: I12bc9287106f1492cbc9e74b4163cce97c957d31
GitHub-Last-Rev: cda1b48fe3ee9083a2262f1d6eeb039c66c12b40
GitHub-Pull-Request: golang/go#66185
Reviewed-on: https://go-review.googlesource.com/c/go/+/569896 Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
guoguangwu [Mon, 11 Mar 2024 05:03:53 +0000 (05:03 +0000)]
os: close pipe in test
Change-Id: Ic8b06c6fd9fc6a30b26f4e4614aa40b5cad3a5e7
GitHub-Last-Rev: 8397a8b30cf11c00e53b35e528f82a8534a00e01
GitHub-Pull-Request: golang/go#66240
Reviewed-on: https://go-review.googlesource.com/c/go/+/570515
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
guoguangwu [Sun, 10 Mar 2024 13:05:30 +0000 (13:05 +0000)]
cmd/preprofile: fix typo in comment
Change-Id: Ib44e9e6345fa8df7f46bc9cbdc19ad8ba73c8b83
GitHub-Last-Rev: 5a37ad798807c1bbc1600086ff162dc7019d1bca
GitHub-Pull-Request: golang/go#66233
Reviewed-on: https://go-review.googlesource.com/c/go/+/570415 Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
guoguangwu [Sun, 10 Mar 2024 07:36:36 +0000 (07:36 +0000)]
cmd/trace/v2: fix typo in comment
Change-Id: Icbf295e668335945084616a88c3ea2cef1bb2527
GitHub-Last-Rev: 0341d0fea71a194d7a85741f6951c8c7c21aee33
GitHub-Pull-Request: golang/go#66229
Reviewed-on: https://go-review.googlesource.com/c/go/+/570356 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
guoguangwu [Mon, 11 Mar 2024 01:22:21 +0000 (01:22 +0000)]
cmd/compile: use raw strings to avoid double escapes
Change-Id: I39917b90b67f630f8212853c0a201635960275cb
GitHub-Last-Rev: fe886534b493fc6241b4451256c889b2fdee997f
GitHub-Pull-Request: golang/go#66180
Reviewed-on: https://go-review.googlesource.com/c/go/+/569975
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Keith Randall <khr@golang.org>
Joel Sing [Wed, 21 Feb 2024 12:29:12 +0000 (23:29 +1100)]
cmd/link,debug/elf: mark Go binaries with no branch target CFI on openbsd
OpenBSD enables Indirect Branch Tracking (IBT) on amd64 and Branch Target
Identification (BTI) on arm64, where hardware permits. Since Go generated
binaries do not currently support IBT or BTI, temporarily mark them with
PT_OPENBSD_NOBTCFI which prevents branch target CFI from being enforced
on execution. This should be removed as soon asn IBT and BTI support are
available.
Fixes #66040
Updates #66054
Change-Id: I91ac05736e6942c54502bef4b8815eb8740d2d5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/568435
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Josh Rickmar <jrick@zettaport.com> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Keith Randall <khr@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Russ Cox [Thu, 29 Feb 2024 23:03:23 +0000 (18:03 -0500)]
time: move runtimeTimer out of Timer struct
If user code has two timers t1 and t2 and does *t1 = *t2
(or *t1 = Timer{}), it creeps me out that we would be
corrupting the runtime data structures inlined in the
Timer struct. Replace that field with a pointer to the
runtime data structure instead, so that the corruption
cannot happen, even in a badly behaved program.
In fact, remove the struct definition entirely and linkname
a constructor instead. Now the runtime can evolve the struct
however it likes without needing to keep package time in sync.
Also move the workaround logic for #21874 out of
runtime and into package time.
Change-Id: Ia30f7802ee7b3a11f5d8a78dd30fd9c8633dc787
Reviewed-on: https://go-review.googlesource.com/c/go/+/568339 Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
guoguangwu [Fri, 8 Mar 2024 02:59:26 +0000 (02:59 +0000)]
cmd/link: fix typo in comment
Change-Id: Ib24841f4823c357ddeefa28435c2b80867d752d2
GitHub-Last-Rev: b0c6c58b24af43b0a0e759b152eb245b3bf1ce4e
GitHub-Pull-Request: golang/go#66182
Reviewed-on: https://go-review.googlesource.com/c/go/+/570015 Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Russ Cox [Fri, 1 Mar 2024 01:14:32 +0000 (20:14 -0500)]
runtime: fix spurious race using Ticker.Reset
Ticker.Reset was added in CL 217362 in 2020.
It added the runtime helper modTimer, which is
analogous to startTimer and resetTimer but for tickers.
Unlike those, it does not contain a racerelease, which
means that code synchronizing by starting a ticker
will be diagnosed with a spurious race.
Add racerelease to modTimer and add tests of all
three racereleases (in startTimer, resetTimer, and modTimer).
Also do not call time.resetTimer from elsewhere in runtime,
since that function is only for package time. Use t.reset instead.
For #33184.
Change-Id: Ie40c1ad24911f21e81b1d3cc608cf086ff2bc83d
Reviewed-on: https://go-review.googlesource.com/c/go/+/568340
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
allp < timers has not been necessary since CL 258303.
sched < timers was implied by allp < timers, and that
was still necessary, but only when the world is stopped.
Rewrite the code to avoid that lock since the world is stopped.
Now timers and timer are independent of the scheduler,
so they could call into the scheduler (for example to ready
a goroutine) if we wanted them to.
Change-Id: I12a93013c98e51c9e2f2148175b02afce8384a59
Reviewed-on: https://go-review.googlesource.com/c/go/+/568337
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Russ Cox [Fri, 1 Mar 2024 00:08:59 +0000 (19:08 -0500)]
runtime: re-add call to ts.cleanHead (née cleantimers) during timer add
Before CL 564118, there were two ways to add a new timer:
addtimer or modtimer. Much code was duplicated between them
and it was always valid to call modtimer instead of addtimer
(but not vice versa), so that CL changed all addtimer call sites
to use modtimer and deleted addtimer.
One thing that was unique to addtimer, however, was that it
called cleantimers (now named ts.cleanHead) after locking the
timers, while modtimer did not. This was the only difference
in the duplicated code, and I missed it. Restore the call to
ts.cleanHead when adding a new timer.
Also fix double-unlock in cleanHead.
Change-Id: I26cc50d650f31f977c0c31195cd013244883dba9
Reviewed-on: https://go-review.googlesource.com/c/go/+/568338 Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
Russ Cox [Thu, 29 Feb 2024 21:52:58 +0000 (16:52 -0500)]
runtime: avoid pp.timers.lock in updateTimerPMask
The comment in updateTimerPMask is wrong. It says:
// Looks like there are no timers, however another P
// may be adding one at this very moment.
// Take the lock to synchronize.
This was my incorrect simplification of the original comment
from CL 264477 when I was renaming all the things it mentioned:
// Looks like there are no timers, however another P may transiently
// decrement numTimers when handling a timerModified timer in
// checkTimers. We must take timersLock to serialize with these changes.
updateTimerPMask is being called by pidleput, so the P in question
is not in use. And other P's cannot add to this P.
As the original comment more precisely noted, the problem was
that other P's might be calling timers.check, which updates ts.len
occasionally while ts is locked, and one of those updates might
"leak" an ephemeral len==0 even when the heap is not going to
be empty when the P is finally unlocked. The lock/unlock in
updateTimerPMask synchronizes to avoid that. But this defeats
most of the purpose of using ts.len in the first place.
Instead of requiring that synchronization, we can arrange that
ts.len only ever shows a "publishable" length, meaning the len(ts.heap)
we leave behind during ts.unlock.
Having done that, updateTimerPMask can be inlined into pidleput.
The big comment on updateTimerPMask explaining how timerpMask
works is better placed as the doc comment for timerpMask itself,
so move it there.
Change-Id: I5442c9bb7f1473b5fd37c43165429d087012e73f
Reviewed-on: https://go-review.googlesource.com/c/go/+/568336 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Russ Cox [Thu, 29 Feb 2024 16:56:07 +0000 (11:56 -0500)]
time: make sure tests avoid the special-case channel code
Many of the tests in package time are about proper manipulation
of the timer heap. But now NewTimer bypasses the timer heap
except when something is blocked on the associated channel.
Make the tests test the heap again by using AfterFunc instead of
NewTimer.
In particular, adds a non-chan version of TestZeroTimer, which
was flaky-broken and then fixed by CLs in the cleanup stack.
This new tests makes sure we notice if it breaks again.
Fixes #66006.
Change-Id: Ib59fc1b8b85ef5a21e72fe418c627c9b8b8a083a
Reviewed-on: https://go-review.googlesource.com/c/go/+/568255
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Russ Cox <rsc@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Russ Cox [Fri, 8 Mar 2024 02:17:04 +0000 (21:17 -0500)]
runtime: fix mishandling of timer zombie count
The timer zombie count was fundamentally racy and worked around
in CL 569995. We worked around that by ignoring underflow.
The fundamnental race was because t.ts was set before t was
inserted into ts. CL 564997 corrected that fundamental problem,
so now we can account for zombies completely accurately,
never seeing values less than zero. Do that.
Russ Cox [Fri, 16 Feb 2024 22:56:43 +0000 (17:56 -0500)]
runtime: fix timers.wakeTime inaccuracy race
timers.wakeTime, which is called concurrently by P's trying to decide
how long they should sleep, can return inaccurate values while
timers.adjust is running. (Before the refactoring, this was still true
but the code did not have good names and was spread across more
files, making the race harder to see.)
The runtime thread sleeping code is complex enough that I am not
confident that the inaccuracy can cause delayed timer wakeups,
but I am also not confident that it can't, nor that it won't in the future.
There are two parts to the fix:
1. A simple logic change in timers.adjust.
2. The introduction of t.maybeAdd to avoid having a t that is
marked as belonging to a specific timers ts but not present
in ts.heap. That was okay before when everything was racy
but needs to be eliminated to make timers.adjust fully consistent.
The cost of the change is an extra CAS-lock operation on a timer add
(close to free since the CAS-lock was just unlocked) and a change
in the static lock ranking to allow malloc while holding a timer lock.
Russ Cox [Fri, 16 Feb 2024 23:42:08 +0000 (18:42 -0500)]
runtime: update timers.len with Store instead of Add
Writes to timers.len are protected by the timers.lock.
There is no need to use an Add instead of a Store,
and the code is clearer (and perhaps slightly faster)
using the Store.
Russ Cox [Fri, 16 Feb 2024 23:38:51 +0000 (18:38 -0500)]
runtime: rename timers fields for clarity
These names were copied over from the p field names,
but now that they are part of the timers type they can use
shorter names that make the relationship clearer.
Michael Anthony Knyszek [Wed, 28 Feb 2024 22:24:48 +0000 (22:24 +0000)]
runtime: fix EvFrequency event value on Windows in the new tracer
The value produced for the EvFrequency event on Windows is missing the
fact that the cputicks clock gets divided. This results in durations
that are consistently wrong by the same factor (about 256).
Fixes #65997.
Change-Id: I930cbfce3499d435c20699f41c11e3227d84f911
Reviewed-on: https://go-review.googlesource.com/c/go/+/567937
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, 26 Feb 2024 20:36:29 +0000 (20:36 +0000)]
runtime: clean up dead P trace state when disabling tracing too
Right now, we're careful to clean up dead P state when we advance to
future trace generations. If we don't, then if that P comes back to
life, we might end up using its old stale trace state.
Unfortunately, we never handled this in the case when tracing stops,
only when advancing to new generations. As a result, stopping a trace,
starting it again, and then bringing a P back to life in the following
generation meant that the dead P could be using stale state.
Fixes #65318.
Change-Id: I9297d9e58a254f2be933b8007a6ef7c5ec3ef4f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/567077 Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Michael Anthony Knyszek [Tue, 20 Feb 2024 20:58:18 +0000 (20:58 +0000)]
internal/trace/v2: clean up the ordering interface
This change cleans up the ordering interface in several ways.
First, it resolves a TODO about using a proper queue of events for extra
events produced while performing ordering. This will be necessary for a
follow-up change to handle coroutine switch events.
Next, it simplifies the ordering.advance method's signature by not
returning a schedCtx. Instead, ordering.advance will take responsibility
for constructing the final Event instead of the caller, and places it on
its own internal queue (in addition to any other Events generated). The
caller is then responsible for taking events off of the queue with a new
method Next.
Finally, hand-in-hand with the signature change, the implementation of
ordering.advance no longer forces each switch case to return but instead
has them converge past the switch. This has two effects. One is that we
eliminate the deferred call to update the M state. Using a defer here is
technically incorrect, because we might end up changing the M state even
if we don't advance the event! We got lucky here that curCtx == newCtx
in all such cases, but there may have been a subtle bug lurking here.
Unfortunately because of the queue's semantics however, we can't
actually avoid pushing into the queue at every possible successful exit
out of the switch. Hopefully this can become less error-prone in the
future by splitting up the switch into a dispatch of different
functions, instead of everything living in one giant function. This
cleanup will happen in a follow-up change.
Change-Id: Ifebbbf14e8ed5c08be5c1b0fadc2e5df3915c656
Reviewed-on: https://go-review.googlesource.com/c/go/+/565936
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 [Thu, 8 Feb 2024 17:16:49 +0000 (17:16 +0000)]
runtime: make checking if tracing is enabled non-atomic
Tracing is currently broken when using iter.Pull from the rangefunc
experiment partly because the "tracing is off" fast path in traceAcquire
was deemed too expensive to check (an atomic load) during the coroutine
switch.
This change adds trace.enabled, a non-atomic indicator of whether
tracing is enabled. It doubles trace.gen, which is the source of truth
on whether tracing is enabled. The semantics around trace.enabled are
subtle.
When tracing is enabled, we need to be careful to make sure that if gen
!= 0, goroutines enter the tracer on traceAcquire. This is enforced by
making sure trace.enabled is published atomically with trace.gen. The
STW takes care of synchronization with most Ms, but there's still sysmon
and goroutines exiting syscalls. We need to synchronize with those
explicitly anyway, which luckily takes care of trace.enabled as well.
When tracing is disabled, it's always OK for trace.enabled to be stale,
since traceAcquire will always double-check gen before proceeding.
For #61897.
Change-Id: I47c2a530fb5339c15e419312fbb1e22d782cd453
Reviewed-on: https://go-review.googlesource.com/c/go/+/565935
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>
Russ Cox [Thu, 7 Mar 2024 18:51:04 +0000 (13:51 -0500)]
time: gracefully handle ts.zombies underflow
The current implementation sets t.ts before adding t to ts;
that can cause inconsistencies with temporarily negative
ts.zombies values. Handle them gracefully, since we only
care about detecting very positive values.
Pending CL 564977 removes the race that sets t.ts early,
and then CL 569996 builds on top of that to make the count precise.
This CL just gets examples like the new test working sooner.
Reason for revert: Breaking real-world tests inside Google,
which means it probably breaks real-world tests outside Google.
One instance I have seen is a <!-- --> comment (often a copyright notice) before the procinst.
Another test checks that a canonicalizer can handle a test input that simply has procinsts mid-XML.
XML is full of contradictions, XML implementations more so. If we are going to start being picky, that probably needs to be controlled by a GODEBUG (and a proposal).
For #65691 (will reopen manually).
Change-Id: Ib52d0944b1478e71744a2a35b271fdf7e1c972ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/570175 Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Damien Neil [Tue, 21 Feb 2023 19:24:56 +0000 (11:24 -0800)]
net/url: consider an empty base Path as equivalent to / in JoinPath
A Path that starts with / is absolute.
A Path that starts with any other character is relative.
The meaning of a Path of "" is not defined,
but RequestURI converts a "" Path to "/"
and an empty Path may represent a URL with just
a hostname and no trailing / such as "http://localhost".
Handle empty paths in the base URL of JoinPath consistently with
RequestURI, so that joining to an empty base produces an absolute
path rather than a relative one.
u, _ := url.Parse("http://localhost")
u = u.JoinPath("x")
fmt.Println(u.Path) // "/x", not "x"
Fixes #58605
Change-Id: Iacced9c173b0aa693800dd01caf774f3f9a66d56
Reviewed-on: https://go-review.googlesource.com/c/go/+/469935 Reviewed-by: Jonathan Amsterdam <jba@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Jes Cok [Thu, 7 Mar 2024 23:34:10 +0000 (07:34 +0800)]
unicode/utf8: update doc for RuneLen
As CL 569755 did, for consistency, this CL slightly improves
the documentation for RuneLen.
Change-Id: Ic9776648baf2809af36cd16a94d1313938bb0e52
Reviewed-on: https://go-review.googlesource.com/c/go/+/569816
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
CL 541715 added an optimization to copy SSA-able variables.
When handling m[k] = append(m[k], ...) case, it uses ir.SameSafeExpr to
check that m[k] expressions are the same, then doing type assertion to
convert the map index to ir.IndexExpr node. However, this assertion is
not safe for m[k] expression in append(m[k], ...), since it may be
wrapped by ir.OCONVNOP node.
Fixing this by un-wrapping any ir.OCONVNOP before doing type assertion.
Fixes #66096
Change-Id: I9ff7165ab97bc7f88d0e9b7b31604da19a8ca206
Reviewed-on: https://go-review.googlesource.com/c/go/+/569716
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>