s==s is always true for strings. This comes up in NaN testing in
generic code, where we want x==x to compile completely away except for
float types.
Fixes#60777
Change-Id: I3ce054b5121354de2f9751b010fb409f148cb637
Reviewed-on: https://go-review.googlesource.com/c/go/+/503795
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
The 5ms sleep in (*Process).Wait was added to mitigate errors while
removing executable files using os.RemoveAll.
Windows 10 1903 implements POSIX semantics for DeleteFile, making the
implementation of os.RemoveAll on Windows much more robust. Older
Windows 10 versions also made internal improvements to avoid errors
when removing files, making it less likely that the 5ms sleep is
necessary.
Windows 10 is the oldest version that Go supports (see #57004), so it
makes sense to unconditionally remove the 5ms sleep now. We have all
the Go 1.22 development cycle to see if this causes any regression.
Fixes#25965
Change-Id: Ie0bbe6dc3e8389fd51a32484d5d20cf59b019451
Reviewed-on: https://go-review.googlesource.com/c/go/+/509335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The default WASI runtime was originally set to Wazero, because it was
the first runtime used to test the Go implementation and because we
could easily find and fix issues in our implementation and theirs.
In CL 498675 we switched the default wasip1 runner to Wasmtime as it
runs faster and is a more established and mature runtime. We should
switch the default runtime to Wasmtime to consistently promote
Wasmtime as the primary tested and approved runtime.
Change-Id: Ic6c064142321af90f015e02b7fe0e71444d8842c
Reviewed-on: https://go-review.googlesource.com/c/go/+/513235
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Eli Bendersky <eliben@google.com>
Run-TryBot: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Rather than duplicating this code, factor it out into a function and
add test coverage.
Change-Id: I37ce568ded4659d98a4ff1361520c5fb2207e947
Reviewed-on: https://go-review.googlesource.com/c/go/+/512537
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This code stems from the original 7l C code, where one way to determine
the end of a table is to put a sentinel entry, then scan for it. This is
now Go code and the length of an array is readily available.
Remove the sentinel and sentinel scan, then adjust the remaining code to
work accordingly.
Change-Id: I8964c787f5149f3548fa78bf8923aa7a93f9482e
Reviewed-on: https://go-review.googlesource.com/c/go/+/512536
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au>
Same as https://go-review.googlesource.com/c/go/+/510635, reduces risk of overflow
Change-Id: I18f5560d73af76c3e853464a89ad7e42dbbd5894
GitHub-Last-Rev: 652c8c6712
GitHub-Pull-Request: golang/go#61547
Reviewed-on: https://go-review.googlesource.com/c/go/+/512200
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
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: Matthew Dempsky <mdempsky@google.com>
This leaves the specific unification details out in favor
of a (forthcoming) section in an appendix.
Change-Id: If984c48bdf71c278e1a2759f9a18c51ef58df999
Reviewed-on: https://go-review.googlesource.com/c/go/+/507417
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com>
On AIX and Solaris the errno value is fetched using m.mOS.perrno.
When checkfds is called, that value has not yet been set up by minit.
Since the error value doesn't really matter in checkfds,
don't bother to check it on AIX and Solaris.
Fixes#61584
Change-Id: I4e679ee3fdad4f0b833ae102597b2d6b8cb46cb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/513215
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
While here, also update the go15bootstrap link to use the
shorter go.dev domain and https:// prefix for consistency.
For #54265.
Change-Id: I881eeda235589511a93bf47186f91f6c47c12932
Reviewed-on: https://go-review.googlesource.com/c/go/+/512720
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
On Unix-like platforms, enforce that the standard file descriptions (0,
1, 2) are always open during initialization. If any of the FDs are
closed, we open them pointing at /dev/null, or fail.
Fixes#60641
Change-Id: Iaab6b3f3e5ca44006ae3ba3544d47da9a613f58f
Reviewed-on: https://go-review.googlesource.com/c/go/+/509020
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Also add a go:generate command to the standard library slices package.
For #61374
Change-Id: I7aae8e451b7c6c4390e0344257478d1a96a14189
Reviewed-on: https://go-review.googlesource.com/c/go/+/511660
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Eli Bendersky <eliben@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Also use a unique share name for each run of the test.
This may help with #61467, but since I couldn't reproduce the failure
in the first place I don't know. It passes locally for me.
For #61467.
Change-Id: Ie51e3cf381063e02e4849af5c1a1ed7441ce21c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/512075
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
At the beginning of the for-loop iteration cap(data) > len(data) always.
Therefore, in the first iteration, this check becomes unnecessary.
we can move this check to after the read operation.
Change-Id: I205e4a842ced74f31124b45a39b70523b56ad840
GitHub-Last-Rev: 2fdf25dff2
GitHub-Pull-Request: golang/go#60473
Reviewed-on: https://go-review.googlesource.com/c/go/+/498915
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
In most cases this change removes assumptions that there is a single
main module in vendor mode and iterates over the workspace modules
when doing checks. The go mod vendor command will now, if in workspace
mode, create a vendor directory in the same directory as the go.work
file, containing the packages (and modules in modules.txt) loaded from
the workspace. When reassembling the module graph from the vendor
directory, an edges are added from each of the main modules to their
requirements, plus additionally to a fake 'vendor/modules.txt' module
with edges to all the modules listed in vendor/modules.txt.
For #60056
Change-Id: I4a485bb39836e7ab35cdc7726229191c6599903e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495801
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
This was missed in the update of the bootstrap toolchain
and should help people who don't set GOROOT_BOOTSTRAP
and instead assume these scripts will find the right one.
For #54265.
Change-Id: I37a0d0976006d13b73df00013780be5abf202e91
Reviewed-on: https://go-review.googlesource.com/c/go/+/512275
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The Test{ARCH}Errors tests will call ctxt.Arch.Assemble, but this
function requires the assembler has been initialized. So this CL adds
a call to architecture.Init(ctxt) in testErrors, otherwise running
Test{ARCH}Errors alone would fail.
Change-Id: I4f3ba5a5fc1375d28779701989cf700cb4d1b635
Reviewed-on: https://go-review.googlesource.com/c/go/+/505976
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.com>
The following instruction is wrongly encoded on arm64:
MOVD (R2)(R3<<0), R1
It's incorrectly encoded as
MOVD (R2)(R3<<3), R1
The reason for the error is that we hard-coded the shift encoding to 6,
which is correct for the MOVB and MOVBU instructions because it only
allows a shift amount of 0, but it is wrong for the MOVD instruction
because it also allows other shift values.
For instructions MOVB, MOVBU and FMOVB, the extension amount must be 0,
encoded in "S" as 0 if omitted, or as 1 if present. But in Go, we don't
distinguish between Rn.<EXT> and Rn.<EXT><<0, so we encode it as that
does not present. This makes no difference to the function of the
instruction.
Change-Id: I2afe3498392cc9b2ecd524c7744f28b9d6d107b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/510995
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Eric Fang <eric.fang@arm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The go test flag -skip had no effect in example tests.
Fixes#61482
Change-Id: I28dfddb88fef3fead2a3c74f9cb63a674a768231
GitHub-Last-Rev: e8c3c3460a
GitHub-Pull-Request: golang/go#61491
Reviewed-on: https://go-review.googlesource.com/c/go/+/511837
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
If we load 2 values and then store those 2 loaded values, we can likely
perform that operation with a single wider load and store.
Fixes#60709
Change-Id: Ifc5f92c2f1b174c6ed82a69070f16cec6853c770
Reviewed-on: https://go-review.googlesource.com/c/go/+/502295
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Add the PID to the core file name if the current system uses it
when generating core files.
Fixes#61487
Change-Id: I3b53a6c850c754795c8022921160f03c588d4c91
Reviewed-on: https://go-review.googlesource.com/c/go/+/511659
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Change-Id: I9cdb301163b67add39928c8fc7df2b7f3893f45e
Reviewed-on: https://go-review.googlesource.com/c/go/+/511836
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Bypass: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
The -s flag is to documented to disable symbol table, not DWARF
(which is the -w flag). However, due to a bug (#15166), -s was
made to also disable DWARF. That bug can be fixed without
disabling DWARF. So do that, and make it possible to enable DWARF
with -s.
Since -s has been disabling DWARF for quite some time, and users
who use -s may want to suppress all symbol information, as DWARF
also contains symbol information, we keep the current behavior,
having -s continue to disable DWARF by default. But we allow
enabling DWARF by specifying -w=0 (or false).
In summary, this is the behavior now:
-s no symbol table, no DWARF
-w has symbol table, no DWARF
-s -w no symbol table, no DWARF (same as -s)
-s -w=0 no symbol table, has DWARF
Change-Id: I1883f0aa3618abccfd735d104d983f7f531813d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/492984
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Add a test checking the -s flag actually suppresses the symbol
table.
Change-Id: I7216d4811a72c62b823d2daa12f6462568243b12
Reviewed-on: https://go-review.googlesource.com/c/go/+/506759
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
If -v is specified, print dsymutil and strip commands.
Change-Id: Icaff2b41ab582d8c58a4ec65438c2986d88def9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/506758
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Currently, on Mach-O in external linking mode, the handling of -s
and -w flags are a bit mixed: neither flag disables the symbol
table, and both flags disable DWARF.
This CL makes it do what is documented: -s disables symbol table,
and -w disables DWARF. For the Darwin system linker, the -s flag
(strip symbol table) is obsolete. So we strip it afterwards. We
already use the strip command to strip the debug STAB symbols if
we need to combine DWARF. With this CL we'll use an additional
flag to strip more symbols. And we now also use strip if -s is
specified and we don't need to combine DWARF.
Change-Id: I9bed24fd388f2bd5b0ffa4ec2db46a4a2f6b1016
Reviewed-on: https://go-review.googlesource.com/c/go/+/493136
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, on Mach-O, we don't strip the symbol table even the -s
flag is set. This CL makes it suppress the symbol table, as
documented.
On Mach-O, even with -s, we still need to keep symbols that are
dynamically exported or referenced symbol. Otherwise the dynamic
linker cannot resolve them and the binary doesn't run.
(Interestingly, for a PIE binary it is okay to strip the symbol
table entirely. We keep the dynamic symbols for consistency. And
this is also in consistent with what the system "strip" command
does.)
Change-Id: I39c572553fe0215ae3bdf5349bf2bab7205fbdc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/492744
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We already skip testcarchive, testcshared, and testplugin in short
mode and not on builders. The shared build mode is not more
supported than the c-archive, c-shared, and plugin build modes. No
need to run it everywhere by default.
Updates #61025.
Change-Id: I6a06e04c1a1dc78f0f85456320d128bd67277915
Reviewed-on: https://go-review.googlesource.com/c/go/+/511696
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Change-Id: I0a55cdc38ae496e2070f0b9ef317a41f82352afd
GitHub-Last-Rev: c19527a26b
GitHub-Pull-Request: golang/go#61407
Reviewed-on: https://go-review.googlesource.com/c/go/+/510635
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This extra newline causes pkg.go.dev and gopls to only show the bottom
half of this comment; I'm pretty sure this entire thing is meant to be
in the docs.
Change-Id: I5bbf081fb2072d9d773d5a995bc3693dc44f65ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/511855
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
The timeout field is documented as being available so that it's possible
to override timeout by setting a non-zero value. If it's left at zero,
we don't need to override the default go test timeout, but we still need
to apply the timeout scale whenever it's something other than 1.
Fixes (via backport) #61468.
Change-Id: I63634e9b3ef8c4ec7f334b5a6b4bf3cad121355c
Reviewed-on: https://go-review.googlesource.com/c/go/+/511567
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Update the documentation for Config.GoVersion to reflect the changes
made for #61175.
Change-Id: I9f3fbcf8ee88e52d6a5e7cf80dad3d2fb5313893
Reviewed-on: https://go-review.googlesource.com/c/go/+/511096
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
It is possible to call reflect.ValueOf(ch).Close() on a recv-only channel,
while close(ch) is a compile-time error. Following the same reflect
semantics as send and recv this should result in a panic.
Fixes#61445
Change-Id: I2a9ee8f45963593a37bd6df4643dd64fb322f9f9
GitHub-Last-Rev: fe2d5e09f5
GitHub-Pull-Request: golang/go#61453
Reviewed-on: https://go-review.googlesource.com/c/go/+/511295
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: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Every call from C to Go does acquire a mutex to check whether Go runtime
has been fully initialized. This often does not matter, because the lock
is held only briefly. However, with code that does a lot of parallel
calls from C to Go could cause heavy contention on the mutex.
Since this is an initialization guard, we can double check with atomic
operation to provide a fast path in case the initialization is done.
With this CL, program in #60961 reduces from ~2.7s to ~1.8s.
Fixes#60961
Change-Id: Iba4cabbee3c9bc646e70ef7eb074212ba63fdc04
Reviewed-on: https://go-review.googlesource.com/c/go/+/505455
Reviewed-by: Heschi Kreinick <heschi@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Removing a lot of functions/variables/constants which are un-unsed
anymore in Unified IR frontend.
Change-Id: Iccf73754196bf4fa40fe701a6468f4c8a1a0c655
Reviewed-on: https://go-review.googlesource.com/c/go/+/506477
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently the GC creates a sweepLocker before restarting the world at
the end of the mark phase, so that it can safely flush mcaches without
the runtime incorrectly concluding that sweeping is done before that
happens.
However, with GODEBUG=gcstoptheworld=2, where sweeping happens during
that STW phase, creating that sweepLocker will fail, since the runtime
will conclude that sweeping is in fact complete (all the queues will be
drained). The problem however is that gcSweep, which does the
non-concurrent sweeping, doesn't actually flush mcaches.
In essence, this failure to create a sweepLocker is indicating a real
issue: sweeping is marked as complete, but we haven't flush the mcaches
yet!
The fix to this is to flush mcaches in gcSweep when in a non-concurrent
sweep. Now that gcSweep actually completes a full sweep, it's safe to
ignore a failure to create a sweepLocker (and in fact, it *must* fail).
While we're here, let's also remove _ConcurrentSweep, the debug flag.
There's already an alias for it called concurrentSweep, and there's only
one use of it in gcSweep.
Lastly, add a dist test for the GODEBUG=gcstoptheworld=2 mode.
Fixes#53885.
Change-Id: I8a1e5b8f362ed8abd03f76e4950d3211f145ab1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/479517
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Fixes#60811
Change-Id: Ica947a4789e71826284f9f6e41c298baa3d033e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/503922
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
The new section describes type inference as the problem
of solving a set of type equations for bound type parameters.
The next CL will update the section on unification to match
the new inference approach.
Change-Id: I2cb49bfb588ccc82d645343034096a82b7d602e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/503920
TryBot-Bypass: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
goschedImpl transitions the current goroutine from _Grunning to
_Grunnable and places it on the global run queue before calling into
schedule.
It does _not_ call wakep after adding the global run queue. I believe
the intuition behind skipping wakep is that since we are immediately
calling the scheduler so we don't need to wake anything to run this
work. Unfortunately, this intuition is not correct, as it breaks
coordination with spinning Ms [1].
Consider this example scenario:
Initial conditions:
M0: Running P0, G0
M1: Spinning, holding P1 and looking for work
Timeline:
M1: Fails to find work; drops P
M0: newproc adds G1 to P0 runq
M0: does not wakep because there is a spinning M
M1: clear mp.spinning, decrement sched.nmspinning (now in "delicate dance")
M1: check sched.runqsize -> no global runq work
M0: gosched preempts G0; adds G0 to global runq
M0: does not wakep because gosched doesn't wakep
M0: schedules G1 from P0 runq
M1: check P0 runq -> no work
M1: no work -> park
G0 is stranded on the global runq with no M/P looking to run it. This is
a loss of work conservation.
As a result, G0 will have unbounded* scheduling delay, only getting
scheduled when G1 yields. Even once G1 yields, we still won't start
another P, so both G0 and G1 will switch back and forth sharing one P
when they should start another.
*The caveat to this is that today sysmon will preempt G1 after 10ms,
effectively capping the scheduling delay to 10ms, but not solving the P
underutilization problem. Sysmon's behavior here is theoretically
unnecessary, as our work conservation guarantee should allow sysmon to
avoid preemption if there are any idle Ps. Issue #60693 tracks changing
this behavior and the challenges involved.
[1] It would be OK if we unconditionally entered the scheduler as a
spinning M ourselves, as that would require schedule to call wakep when
it finds work in case there is more work.
Fixes#55160.
Change-Id: I2f44001239564b56ea30212553ab557051d22588
Reviewed-on: https://go-review.googlesource.com/c/go/+/501976
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
When a thread transitions to spinning to non-spinning it must recheck
all sources of work because other threads may submit new work but skip
wakep because they see a spinning thread.
However, since the beginning of time (CL 7314062) we do not check the
global run queue, only the local per-P run queues.
The global run queue is checked just above the spinning checks while
dropping the P. I am unsure what the purpose of this check is. It
appears to simply be opportunistic since sched.lock is already held
there in order to drop the P. It is not sufficient to synchronize with
threads adding work because it occurs before decrementing
sched.nmspinning, which is what threads us to decide to wake a thread.
Resolve this by adding an explicit global run queue check alongside the
local per-P run queue checks.
Almost nothing happens between dropped sched.lock after dropping the P
and relocking sched.lock: just clearing mp.spinning and decrementing
sched.nmspinning. Thus it may be better to just hold sched.lock for this
entire period, but this is a larger change that I would prefer to avoid
in the freeze and backports.
For #55160.
Change-Id: Ifd88b5a4c561c063cedcfcfe1dd8ae04202d9666
Reviewed-on: https://go-review.googlesource.com/c/go/+/501975
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Previously, FindPkg returned the empty string as a sentinel value,
causing Import to collapse all errors to "can't find import".
(See also https://go.dev/wiki/CodeReviewComments#in-band-errors.)
For #61064.
Change-Id: I21f335d206308b44fe585619e00782abb0b65a94
Reviewed-on: https://go-review.googlesource.com/c/go/+/507360
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
