Joe Tsai [Thu, 24 Aug 2023 19:49:10 +0000 (12:49 -0700)]
encoding/json: modernize tests
There are no changes to what is being tested.
No test cases were removed or added.
Changes made:
* Use a local implementation of test case position marking. See #52751.
* Use consistent names for all test tables and variables.
* Generally speaking, follow modern Go style guide for tests.
* Move global tables local to the test function if possible.
* Make every table entry run in a distinct testing.T.Run.
The purpose of this change is to make it easier to perform
v1-to-v2 development where we want v2 to support close to
bug-for-bug compatibility when running in v1 mode.
Annotating each test case with the location of the test data
makes it easier to jump directly to the test data itself
and understand why this particular case is failing.
Having every test case run in its own t.Run makes it easier
to isolate a particular failing test and work on fixing the code
until that test case starts to pass again.
Unfortunately, many tests are annotated with an empty name.
An empty name is better than nothing, since the testing framework
auto assigns a numeric ID for duplicate names.
It is not worth the trouble to give descriptive names to each
of the thousands of test cases.
Change-Id: I43905f35249b3d77dfca234b9c7808d40e225de8
Reviewed-on: https://go-review.googlesource.com/c/go/+/522880
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: Damien Neil <dneil@google.com>
korzhao [Tue, 22 Aug 2023 06:22:53 +0000 (14:22 +0800)]
time: make time.Until a few nanoseconds faster
This is similar to CL 518336.
For #61765.
Change-Id: I7c1d92a3b3e2b6c1c0058a2094997d93082ad139
Reviewed-on: https://go-review.googlesource.com/c/go/+/521237
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
To avoid duplicating them in net/netip and os and to allow these
packages automatically benefiting from future performance improvements
when optimized native LastIndexByte{,String} implementations are added.
korzhao [Fri, 11 Aug 2023 09:20:36 +0000 (17:20 +0800)]
encoding/json: avoid allocation when decoding number types
In CL 345488, we optimized strconv.ParseXXX for []byte arguments.
That allows immediate casting of a []byte to a string that does not escape to be copied on the stack.
Joel Sing [Sat, 7 Jan 2023 05:26:15 +0000 (16:26 +1100)]
cmd/internal/obj/arm64: load large constants into vector registers from rodata
Load large constants into vector registers from rodata, instead of placing them
in the literal pool. This treats VMOVQ/VMOVD/VMOVS the same as FMOVD/FMOVS and
makes use of the existing mechanism for storing values in rodata. Two additional
instructions are required for a load, however these instructions are used
infrequently and already have a high latency.
Updates #59615
Change-Id: I54226730267689963d73321e548733ae2d66740e
Reviewed-on: https://go-review.googlesource.com/c/go/+/515617 Reviewed-by: Eric Fang <eric.fang@arm.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Ian Lance Taylor [Wed, 23 Aug 2023 22:57:48 +0000 (15:57 -0700)]
net/http: use reflect.TypeFor for known types
For #60088
Change-Id: I9e4044d9c2694fe86aab1f5220622c8d952b1a90
Reviewed-on: https://go-review.googlesource.com/c/go/+/522338 Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Matthew Dempsky [Thu, 24 Aug 2023 10:04:01 +0000 (03:04 -0700)]
cmd/compile: create "init" function during noding
This CL arranges for package-scope initialization statements to be
constructed directly into their eventual "init" function, so we can
eliminate the roundabout solution of using InitTodoFunc.
While here, somewhat simplify and generalize the logic for outlining
map initialization statements.
Change-Id: I8aff042e6b266f7024de436424ec6711b8b69129
Reviewed-on: https://go-review.googlesource.com/c/go/+/522318
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Fri, 18 Aug 2023 07:32:11 +0000 (00:32 -0700)]
cmd/compile/internal/gc: steps towards work-queue
[This is a reattempt of go.dev/cl/520611.]
This CL reorganizes the top-level functions for handling package-level
declarations, runtime type descriptors, and SSA compilation to work in
a loop. This generalizes the loop that previously existed in dumpdata.
Change-Id: I7502798a8662b3cec92d3001169f3af4f804df2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/522339 Reviewed-by: Than McIntosh <thanm@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Tobias Klauser [Thu, 24 Aug 2023 09:58:58 +0000 (11:58 +0200)]
cmd/compile/internal/syntax: use strings.LastIndexByte in trailingDigits
Previously, strings.LastIndexByte couldn't be used because it was only
added in Go 1.5 but Go 1.4 was required for bootstrapping. In Go 1.18,
the bootstrap toolchain was bumped to Go 1.17 (see #44505), thus
strings.LastIndexByte can be used now.
Change-Id: I01a70a59dbfc853cf03d49747a2dd62d21ba74e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/522197 Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Austin Clements [Thu, 24 Aug 2023 19:14:44 +0000 (15:14 -0400)]
cmd/go: clarify promotion of C warnings to errors
CL 23005 (back in 2016!) added logic to promote C compiler warnings to
errors when running on the Go builders.
CL 437298 kept the logic to promote warnings to errors on the
builders, but dropped the explanatory message, I believe
unintentionally. Indeed, now there isn't even a comment in the code
explaining what's going on.
This CL adds back an explanatory message to the printed output, which
also serves as a explanation in the code as to why we're checking
$GO_BUILDER_NAME.
mstmdev [Mon, 21 Aug 2023 09:04:07 +0000 (09:04 +0000)]
cmd/internal/goobj: fix the size of Header
Missed the length of the Fingerprint field.
Change-Id: I690955466895e73821dc1e30f8400efc30338ae6
GitHub-Last-Rev: 87bd06d8053c003a412a6ea11b3eafcc280b94d1
GitHub-Pull-Request: golang/go#62174
Reviewed-on: https://go-review.googlesource.com/c/go/+/521495
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: Carlos Amedee <carlos@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>
Luka Krmpotić [Mon, 21 Aug 2023 23:17:31 +0000 (23:17 +0000)]
strconv: add missing function name, avoid unnecessary raw string
Change-Id: Iecd68f69f5265f1a4ea41b90aa819ce68fe55908
GitHub-Last-Rev: 4d1f4a1bc2740fc19bbb4768009ccfea1c8eb5bc
GitHub-Pull-Request: golang/go#62177
Reviewed-on: https://go-review.googlesource.com/c/go/+/521277
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Carlos Amedee <carlos@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Bryan C. Mills [Thu, 24 Aug 2023 14:45:11 +0000 (10:45 -0400)]
syscall: skip TestUnshareMountNameSpaceChroot on platforms that require external linking
TestUnshareMountNameSpaceChroot attempts to build a statically-linked
binary to run in a chroot, and sets CGO_ENABLED=0 in order to do so.
Rather than trying to figure out some other way to coax the linker
into building a static binary, let's just skip the test on Linux
platforms that require external linking (namely android/arm).
Matthew Dempsky [Wed, 23 Aug 2023 20:52:25 +0000 (13:52 -0700)]
cmd/compile/internal/syntax: add Unparen and UnpackListExpr helpers
We've added Unparen to go/ast, so add syntax.Unparen to be
consistent (and because it's similarly useful).
Also, types2 and noder both have similar functions for unpacking
ListExprs, so might as well add a common implementation in package
syntax too.
Finally, addressing the TODO: UnpackListExpr is small enough to be
inlined (when default optimizations are enabled), and for typical uses
of UnpackListExpr (e.g., "range UnpackListExpr(x)") the single-element
slice result is stack allocated in the caller. This CL adds a test
using testing.AllocsPerRun to ensure this remains so in the future.
Change-Id: I96a5591d202193ed5bf1ce6f290919107e3dc01b
Reviewed-on: https://go-review.googlesource.com/c/go/+/522336
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Joe Tsai [Mon, 21 Aug 2023 20:06:23 +0000 (13:06 -0700)]
encoding/json: encode \b and \f as '\b' and '\f' in JSON strings
According to RFC 8259, there are exactly 5 control characters
that have a shorter escape sequence than the generic \uXXXX format.
Over the years, we added ad-hoc support for the short sequences:
* https://go.dev/cl/4678046 supports \r and \n
* https://go.dev/cl/162340043 supports \t
This CL completes the set by supporting \b and \f.
This may change the encoding of strings in relatively rare cases,
but is a permissible change since the Go 1 compatibility document does
not guarantee that "json" produces byte-for-byte identical outputs.
In fact, we have made even more observable output changes in the past
such as with https://go.dev/cl/30371 which changes the representation
of many JSON numbers.
This change is to prepare the path forward for a potential
v2 "json" package, which has more consistent encoding of JSON strings.
Change-Id: I11102a0602dfb1a0c14eaad82ed23e8df7553c6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/521675
Auto-Submit: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net> Reviewed-by: Bryan Mills <bcmills@google.com>
Bryan C. Mills [Tue, 22 Aug 2023 15:32:20 +0000 (11:32 -0400)]
os: use testenv.Command and os.Executable in tests
On Unix platforms, testenv.Command sends SIGQUIT to stuck commands
before the test times out. For subprocesses that are written in Go,
that causes the runtime to dump running goroutines, and in other
languages it triggers similar behavior (such as a core dump).
If the subprocess is stuck due to a bug (such as #57999), that may
help to diagnose it.
Matthew Dempsky [Wed, 23 Aug 2023 00:44:19 +0000 (17:44 -0700)]
cmd/compile/internal/noder: elide statically known "if" statements
In go.dev/cl/517775, I moved the frontend's deadcode elimination pass
into unified IR. But I also made a small enhancement: a branch like
"if x || true" is now detected as always taken, so the else branch can
be eliminated.
However, the inliner also has an optimization for delaying the
introduction of the result temporary variables when there's a single
return statement (added in go.dev/cl/266199). Consequently, the
inliner turns "if x || true { return true }; return true" into:
if x || true {
~R0 := true
goto .i0
}
.i0:
// code that uses ~R0
In turn, this confuses phi insertion, because it doesn't recognize
that the "if" statement is always taken, and so ~R0 will always be
initialized.
With this CL, after inlining we instead produce:
_ = x || true
~R0 := true
goto .i0
.i0:
Fixes #62211.
Change-Id: Ic8a12c9eb85833ee4e5d114f60e6c47817fce538
Reviewed-on: https://go-review.googlesource.com/c/go/+/522096 Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Michael Anthony Knyszek [Mon, 21 Aug 2023 22:29:11 +0000 (22:29 +0000)]
runtime: create wrappers for gcDrain to identify time in profiles
Currently it's impossible to identify in profiles where gcDrain-related
time is coming from. More specifically, what kind of worker. Create
trivial wrappers for each worker so that the difference shows up in
stack traces.
Joel Sing [Wed, 31 Aug 2022 08:18:19 +0000 (18:18 +1000)]
cmd/internal/obj/riscv,cmd/link: add support for internal cgo linking on riscv64
Make it possible to internally link cgo on riscv64, which also adds
support for SDYNIMPORT calls without external linking being required.
This reduces the time of an ./all.bash run on a Sifive Hifive Unleashed by
approximately 20% (~140 minutes down to ~110 minutes).
Change-Id: I43f1348de31672718ae8676cc82f6fdc1dfee054
Reviewed-on: https://go-review.googlesource.com/c/go/+/431104
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Than McIntosh <thanm@google.com>
Joel Sing [Thu, 10 Aug 2023 14:57:27 +0000 (00:57 +1000)]
cmd/go/testdata/mod: add golang toolchain test data for openbsd/ppc64
Updates #56001
Change-Id: Ic7b4ecb2e471292894c54610e8acda8822c890fb
Reviewed-on: https://go-review.googlesource.com/c/go/+/518275 Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Paul Murphy <murp@ibm.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Keith Randall [Fri, 18 Aug 2023 19:44:37 +0000 (12:44 -0700)]
cmd/compile: allow non-pointer writes in the middle of a write barrier
This lets us combine more write barriers, getting rid of some of the
test+branch and gcWriteBarrier* calls.
With the new write barriers, it's easy to add a few non-pointer writes
to the set of values written.
We allow up to 2 non-pointer writes between pointer writes. This is enough
for, for example, adjacent slice fields.
Fixes #62126
Change-Id: I872d0fa9cc4eb855e270ffc0223b39fde1723c4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/521498
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
Bryan C. Mills [Tue, 22 Aug 2023 21:33:50 +0000 (17:33 -0400)]
cmd/go: retry ETXTBSY errors when running test binaries
An ETXTBSY error when starting a test binary is almost certainly
caused by the race reported in #22315. That race will resolve quickly
on its own, so we should just retry the command instead of reporting a
spurious failure.
Ian Lance Taylor [Wed, 16 Aug 2023 00:29:40 +0000 (17:29 -0700)]
runtime/cgo: get getstackbound for set_stacklo
Change-Id: Ia63a4604449b5e460e6f54c962fb7d6db2bc6a43
Reviewed-on: https://go-review.googlesource.com/c/go/+/519457
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
Matthew Dempsky [Sun, 20 Aug 2023 23:14:50 +0000 (16:14 -0700)]
cmd/compile/internal/typecheck: merge SubstArgTypes into LookupRuntime
LookupRuntime is the only reason for using SubstArgTypes, and most
callers to LookupRuntime need to immediately call it anyway. So might
as well fuse them together.
Change-Id: Ie0724ed164b949040e898a2a77bea632801b64fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/521415 Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Matthew Dempsky [Sun, 20 Aug 2023 22:07:00 +0000 (15:07 -0700)]
cmd/compile/internal/types: simplify iterating all parameters
The types.RecvsParamsResults, etc. helpers existed to make it "easier"
to iterate over all parameters, or recvs+params, or params+results;
but they end up still being quite clumsy to use due to the design goal
of not allocating temporary slices.
Now that recvs+params+results are stored in a single consecutive slice
anyway, we can just return different subslices and simplify the loops.
Change-Id: I84791b80dc099dfbfbbe6eddbc006135528c23b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/521375
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Matthew Dempsky [Sun, 20 Aug 2023 21:33:32 +0000 (14:33 -0700)]
cmd/compile/internal/types: simpler signature type representation
Now that all of the uses of signature types have been cleaned up, we
can simplify the internal representation significantly.
In particular, instead of 3 separate struct objects each with 3
separate slices of fields, we can store all of the parameters in a
single slice and track the boundaries between them.
We still need a results tuple struct for representing the type of
multi-value call expressions, but just a single one and it can safely
reuse the results subsection of the full parameters slice.
Note: while Sizeof(Func) has increased (e.g., 32->56 on amd64), we're
saving on the allocation of 2 Types, 2 Structs, and 2 []*Field (288
bytes total on amd64), not counting any extra GC size class padding
from using a single shared []*Field instead of 3 separate ones.
Change-Id: I119b5e960e715b3bc4f1f726e58b910a098659da
Reviewed-on: https://go-review.googlesource.com/c/go/+/521335
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
cmd/compile: add all runtime package dependencies to NoInstrumentPkgs
Currently, this list includes *almost* all runtime packages, but not
quite all.
We leave out internal/bytealg for reasons explained in the code.
Compiling with or without race instrumentation has no effect on the
other packages added to the list here, so this is a no-op change
today, but makes this more robust.
This adds a test that all packages imported by runtime are marked as
runtime tests by LookupPkgSpecial. We add two packages that were
missing from the list.
Austin Clements [Fri, 30 Jun 2023 20:33:49 +0000 (16:33 -0400)]
cmd/compile,cmd/dist,cmd/go: compute -+ flag from package path
As we did for the asm -compiling-runtime flag, this CL modifies the
compiler to compute the -+ (compiling runtime) flag from the package
path. Unlike for asm, some tests use -+ explicitly to opt in to
runtime restrictions, so we leave the flag, but it's no longer passed
by any build tools.
This lets us eliminate cmd/go's list of "runtime packages" in favor of
the unified objabi.LookupPkgSpecial. It also fixes an inconsistency
with dist, which only passed -+ when compiling "runtime" itself.
One consequence of this is that the compiler now ignores the -N flag
when compiling runtime packages. Previously, cmd/go would strip -N
when passing -+ and the compiler would fatal if it got both -N and -+,
so the overall effect was that the compiler never saw -N when
compiling a runtime package. Now we simply move that logic to disable
-N down into the compiler.
There are several implementations of "is this package path a runtime
package". They all have slightly different lists because they all care
about slightly different properties of building the runtime.
To start converging these, we replace objabi.IsRuntimePackagePath with
objabi.LookupPkgSpecial, which returns a struct we can extend with
various special build properties. We'll extend this with several other
flags in the following CLs.
Austin Clements [Fri, 30 Jun 2023 20:33:49 +0000 (16:33 -0400)]
cmd/asm,cmd/dist,cmd/go: remove asm -compiling-runtime flag
Currently, dist and go pass a -compiling-runtime flag to asm if
they're compiling a runtime package. However, now that we always pass
the package path to asm, it can make that determination just as well
as its callers can. This CL moves that check into asm and drops the
flag.
This in turn makes dist's copy of IsRuntimePackagePath unnecessary, so
we delete it.
Currently, the types package has IsRuntimePkg and IsReflectPkg
predicates for testing if a Pkg is the runtime or reflect packages.
IsRuntimePkg returns "true" for any "CompilingRuntime" package, which
includes all of the packages imported by the runtime. This isn't
inherently wrong, except that all but one use of it is of the form "is
this Sym a specific runtime.X symbol?" for which we clearly only want
the package "runtime" itself. IsRuntimePkg was introduced (as
isRuntime) in CL 37538 as part of separating the real runtime package
from the compiler built-in fake runtime package. As of that CL, the
"runtime" package couldn't import any other packages, so this was
adequate at the time.
We could fix this by just changing the implementation of IsRuntimePkg,
but the meaning of this API is clearly somewhat ambiguous. Instead, we
replace it with a new RuntimeSymName function that returns the name of
a symbol if it's in package "runtime", or "" if not. This is what
every call site (except one) actually wants, which lets us simplify
the callers, and also more clearly addresses the ambiguity between
package "runtime" and the general concept of a runtime package.
IsReflectPkg doesn't have the same issue of ambiguity, but it
parallels IsRuntimePkg and is used in the same way, so we replace it
with a new ReflectSymName for consistency.
Michael Anthony Knyszek [Mon, 7 Aug 2023 19:09:59 +0000 (19:09 +0000)]
runtime: avoid MADV_HUGEPAGE for heap memory
Currently the runtime marks all new memory as MADV_HUGEPAGE on Linux and
manages its hugepage eligibility status. Unfortunately, the default
THP behavior on most Linux distros is that MADV_HUGEPAGE blocks while
the kernel eagerly reclaims and compacts memory to allocate a hugepage.
This direct reclaim and compaction is unbounded, and may result in
significant application thread stalls. In really bad cases, this can
exceed 100s of ms or even seconds.
Really all we want is to undo MADV_NOHUGEPAGE marks and let the default
Linux paging behavior take over, but the only way to unmark a region as
MADV_NOHUGEPAGE is to also mark it MADV_HUGEPAGE.
The overall strategy of trying to keep hugepages for the heap unbroken
however is sound. So instead let's use the new shiny MADV_COLLAPSE if it
exists.
MADV_COLLAPSE makes a best-effort synchronous attempt at collapsing the
physical memory backing a memory region into a hugepage. We'll use
MADV_COLLAPSE where we would've used MADV_HUGEPAGE, and stop using
MADV_NOHUGEPAGE altogether.
Because MADV_COLLAPSE is synchronous, it's also important to not
re-collapse huge pages if the huge pages are likely part of some large
allocation. Although in many cases it's advantageous to back these
allocations with hugepages because they're contiguous, eagerly
collapsing every hugepage means having to page in at least part of the
large allocation.
However, because we won't use MADV_NOHUGEPAGE anymore, we'll no longer
handle the fact that khugepaged might come in and back some memory we
returned to the OS with a hugepage. I've come to the conclusion that
this is basically unavoidable without a new madvise flag and that it's
just not a good default. If this change lands, advice about Linux huge
page settings will be added to the GC guide.
Verified that this change doesn't regress Sweet, at least not on my
machine with:
/sys/kernel/mm/transparent_hugepage/enabled [always or madvise]
/sys/kernel/mm/transparent_hugepage/defrag [madvise]
/sys/kernel/mm/transparent_hugepage/khugepaged/max_ptes_none [0 or 511]
Unfortunately, this workaround means that we only get forced hugepages
on Linux 6.1+.
Fixes #61718.
Change-Id: I7f4a7ba397847de29f800a99f9cb66cb2720a533
Reviewed-on: https://go-review.googlesource.com/c/go/+/516795 Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Bryan C. Mills [Tue, 22 Aug 2023 15:01:29 +0000 (11:01 -0400)]
net/http: use testenv.Command instead of exec.Command in tests
On Unix platforms, testenv.Command sends SIGQUIT to stuck commands
before the test times out. For subprocesses that are written in Go,
that causes the runtime to dump running goroutines, and in other
languages it triggers similar behavior (such as a core dump).
If the subprocess is stuck due to a bug (such as #57999), that may
help to diagnose it.
Keith Randall [Mon, 21 Aug 2023 21:10:24 +0000 (14:10 -0700)]
cmd/compile: use better line numbers for write barriers
When the write barrier does several pointer writes under one
write barrier flag check, the line numbers aren't really correct.
The writes inside the write barrier have a confusing set of positions.
The loads of the old values are given the line number of the
corresponding store instruction, but the stores into the write buffer
are given the line number of the first store. Instead, give them all
line numbers corresponding to the store instruction.
The writes at the merge point, which are the original writes and the
only ones that happen when the barrier is off, are currently all given
the line number of the first write. Instead give them their original
line number.
Change-Id: Id64820b707f45f07b0978f8d03c97900fdc4bc0b
Reviewed-on: https://go-review.googlesource.com/c/go/+/521499 Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Meng Zhuo [Wed, 28 Jun 2023 08:45:07 +0000 (16:45 +0800)]
cmd/compile: add single-precision FMA code generation for riscv64
This CL adds FMADDS,FMSUBS,FNMADDS,FNMSUBS SSA support for riscv
Change-Id: I1e7dd322b46b9e0f4923dbba256303d69ed12066
Reviewed-on: https://go-review.googlesource.com/c/go/+/506616 Reviewed-by: Joel Sing <joel@sing.id.au> Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: M Zhuo <mzh@golangcn.org>
Ref: The RISC-V Instruction Set Manual Volume I: Unprivileged ISA
11.6 Single-Precision Floating-Point Computational Instructions
Change-Id: I89aa3a4df7576fdd47f4a6ee608ac16feafd093c
Reviewed-on: https://go-review.googlesource.com/c/go/+/506036 Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: M Zhuo <mzh@golangcn.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joel Sing [Sat, 11 Mar 2023 15:15:43 +0000 (02:15 +1100)]
syscall: add support for openbsd/ppc64
Add syscall support for the openbsd/ppc64 port.
Updates #56001
Change-Id: I695c5c296e90645515de0c8f89f1bc57e976679d
Reviewed-on: https://go-review.googlesource.com/c/go/+/475636 Reviewed-by: Eric Grosse <grosse@gmail.com> Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Joel Sing [Sat, 18 Mar 2023 08:51:52 +0000 (19:51 +1100)]
runtime: rework asmcgocall on ppc64x
On some platforms asmcgocall can be called with a nil g. Additionally, it
can be called when already on a the system (g0) stack or on a signal stack.
In these cases we do not need to switch (and/or cannot switch) to the
system stack and as a result, do not need to save the g.
Rework asmcgocall on ppc64x to follow the pattern used on other architectures,
such as amd64 and arm64, where a separate nosave path is called in the above
cases. The nil g case will be needed to support openbsd/ppc64.
Updates #56001
Change-Id: I431d4200bcbc4aaddeb617aefe18590165ff2927
Reviewed-on: https://go-review.googlesource.com/c/go/+/478775 Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com> Reviewed-by: Paul Murphy <murp@ibm.com>
Run-TryBot: Joel Sing <joel@sing.id.au> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Aug 2023 17:05:29 +0000 (10:05 -0700)]
cmd/compile/internal/types: overhaul and simplify API
This CL removes a lot of the redundant methods for accessing struct
fields and signature parameters. In particular, users never have to
write ".Slice()" or ".FieldSlice()" anymore; the exported APIs just do
what you want.
Further internal refactorings to follow.
Change-Id: I45212f6772fe16aad39d0e68b82d71b0796e5639
Reviewed-on: https://go-review.googlesource.com/c/go/+/521295
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Rather than constructing a new runtime._defer struct type at each
defer statement, we can use a single shared one. Also, by naming it
runtime._defer, we avoid emitting new runtime and DWARF type
descriptors in every package that contains a "defer" statement.
Shaves ~1kB off cmd/go.
Change-Id: I0bd819aec9f856546e684abf620e339a7555e73f
Reviewed-on: https://go-review.googlesource.com/c/go/+/521676 Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Fri, 18 Aug 2023 22:52:34 +0000 (15:52 -0700)]
spec: clarify that []byte("") must be non-nil
The example text below suggests that []byte("") always evaluates to
the non-nil value []byte{}, but the text proper doesn't explicitly
require that. This CL makes it clear that it must not evaluate to
[]byte(nil), which otherwise was allowed by the wording.
Change-Id: I6564bfd5e2fd0c820d9b55d17406221ff93ce80c
Reviewed-on: https://go-review.googlesource.com/c/go/+/521035 Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Jes Cok [Sat, 19 Aug 2023 00:21:00 +0000 (00:21 +0000)]
os/exec: don't convert byte slice to string when using verb %s
Change-Id: I4d755e401acf670fb5a154ff59e4e4335ed2138e
GitHub-Last-Rev: a91d74ae55f84a0e572d2ace335ec42038d7a76f
GitHub-Pull-Request: golang/go#62150
Reviewed-on: https://go-review.googlesource.com/c/go/+/520918
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>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The go resolver shouldn't attempt to query .onion domains, but
the restriction was not restricted for search domains.
Also before this change query for "sth.onion" would
not be suffixed with any search domain (for "go.dev" search
domain, it should query fine the "std.onion.go.dev" domain).
Change-Id: I0f3e1387e0d59721381695f94586e3743603c30e
GitHub-Last-Rev: 7e8ec44078529353c18c8fe34e5207014ce1e685
GitHub-Pull-Request: golang/go#60678
Reviewed-on: https://go-review.googlesource.com/c/go/+/501701
Run-TryBot: Mateusz Poliwczak <mpoliwczak34@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Matthew Dempsky [Sun, 20 Aug 2023 17:23:03 +0000 (10:23 -0700)]
cmd/compile/internal/compare: simplify unit test framework
This CL refactors the compare unit tests to be simpler and to stop
using the types API in non-idiomatic ways, to facilitate further
refactoring of the API.
Change-Id: I864a66b2842a0d8dd45f4e3d773144d71666caf2
Reviewed-on: https://go-review.googlesource.com/c/go/+/521275
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Sun, 20 Aug 2023 06:04:46 +0000 (23:04 -0700)]
cmd/compile/internal/abi: stop using types.Func
This is supposed to be an internal type within package types. At least
for now, users of the types package should stick to the types.Type
APIs as much as possible.
This CL also unexports FuncType and a few others to prevent
backsliding.
Change-Id: I053fc115a5e6a57c148c8149851a45114756072f
Reviewed-on: https://go-review.googlesource.com/c/go/+/521255
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Austin Clements [Tue, 1 Aug 2023 18:41:42 +0000 (14:41 -0400)]
runtime: drop stack-allocated pcvalueCaches
Now that pcvalue keeps its cache on the M, we can drop all of the
stack-allocated pcvalueCaches and stop carefully passing them around
between lots of operations. This significantly simplifies a fair
amount of code and makes several structures smaller.
This series of changes has no statistically significant effect on any
runtime Stack benchmarks.
I also experimented with making the cache larger, now that the impact
is limited to the M struct, but wasn't able to measure any
improvements.
Austin Clements [Tue, 1 Aug 2023 17:54:32 +0000 (13:54 -0400)]
runtime: move pcvalue cache to M
Currently, the pcvalue cache is stack allocated for each operation
that needs to look up a lot of pcvalues. It's not always clear where
to put it, a lot of the time we just pass a nil cache, it doesn't get
reused across operations, and we put a surprising amount of effort
into threading these caches around.
This CL moves it to the M, where it can be long-lived and used by all
pcvalue lookups, and we don't have to carefully thread it across
operations.
This is a re-roll of CL 515276 with a fix for reentrant use of the
pcvalue cache from the signal handler.
Keith Randall [Fri, 18 Aug 2023 19:38:32 +0000 (12:38 -0700)]
cmd/compile: ensure we keep top 32 bits zeroed for 32-bit arm64 ops
When rewriting, for example, MSUBW, we need to ensure that the result
has its 32 top bits zeroed. That's what the instruction is spec'd to do.
Normally, we'd only use MSUBW for computations on 32-bit values, and
as such the top 32 bits aren't normally used. But some situations, like
if we cast the result to a uint64, the top 32 bits do matter.
This comes up in 62131 because we have a rule saying, MOVWUreg applied
to a MSUBW is unnecessary, as the arg to MOVWUreg already has zeroed
top 32 bits. But if MSUBW is later rewritten to another op that doesn't
zero the top 32 bits (SUB, probably), getting rid of the MOVWUreg earlier
causes a problem.
So change rewrite rules to always maintain the top 32 bits as zero if the
instruction is spec'd to provide that. We need to introduce a few *W operations
to make that happen.
Fixes #62131
Change-Id: If3d160821e285fd7454746b735a243671bff8894
Reviewed-on: https://go-review.googlesource.com/c/go/+/520916
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Reviewed-by: Keith Randall <khr@google.com>
Bryan C. Mills [Mon, 21 Aug 2023 16:55:43 +0000 (12:55 -0400)]
context: fix synchronization in ExampleAfterFunc_cond
Condition variables are subtle and error-prone, and this example
demonstrates exactly the sorts of problems that they introduce.
Unfortunately, we're stuck with them for the foreseeable future.
As previously implemented, this example was racy: since the callback
passed to context.AfterFunc did not lock the mutex before calling
Broadcast, it was possible for the Broadcast to occur before the
goroutine was parked in the call to Wait, causing in a missed wakeup
resulting in deadlock.
The example also had a more insidious problem: it was not safe for
multiple goroutines to call waitOnCond concurrently, but the whole
point of using a sync.Cond is generally to synchronize concurrent
goroutines. waitOnCond must use Broadcast to ensure that it wakes up
the target goroutine, but the use of Broadcast in this way would
produce spurious wakeups for all of the other goroutines waiting on
the same condition variable. Since waitOnCond did not recheck the
condition in a loop, those spurious wakeups would cause waitOnCond
to spuriously return even if its own ctx was not yet done.
Fixing the aforementioned bugs exposes a final problem, inherent to
the use of condition variables in this way. This one is a performance
problem: for N concurrent calls to waitOnCond, the resulting CPU cost
is at least O(N²). This problem cannot be addressed without either
reintroducing one of the above bugs or abandoning sync.Cond in the
example entirely. Given that this example was already published in Go
1.21, I worry that Go users may think that it is appropriate to use a
sync.Cond in conjunction with context.AfterFunc, so I have chosen to
retain the Cond-based example and document its pitfalls instead of
removing or replacing it entirely.
I described this class of bugs and performance issues — and suggested
some channel-based alternatives — in my GopherCon 2018 talk,
“Rethinking Classical Concurrency Patterns”. The section on condition
variables starts on slide 37. (https://youtu.be/5zXAHh5tJqQ?t=679)
Bryan C. Mills [Fri, 18 Aug 2023 19:38:44 +0000 (15:38 -0400)]
cmd/go: find GOROOT using os.Executable when installed to GOROOT/bin/GOOS_GOARCH
When running make.bash in a cross-compiled configuration
(for example, GOARCH different from GOHOSTARCH), cmd/go
is installed to GOROOT/bin/GOOS_GOARCH instead of GOROOT/bin.
That means that we need to look for GOROOT in both ../.. and ../../..,
not just the former.
cmd/compile: move racewalk comment to walk/race.go
This comment got left behind in some refactoring and now refers to
code "below" that is no longer below. Move it to be with the code it's
referring to.
Andy Pan [Thu, 10 Aug 2023 06:02:03 +0000 (14:02 +0800)]
runtime: document maxStack and m.createstack in more details
Change-Id: If93b6cfa5a598a5f4101c879a0cd88a194e4a6aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/518116 Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Andy Pan <panjf2000@gmail.com>
This CL refactors common patterns for constructing field and method
selector expressions. Notably, XDotField and XDotMethod are now the
only two functions where a SelecterExpr with OXDOT is constructed.
Change-Id: I4c087225d8b295c4a6a92281ffcbcabafe2dc94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/520979
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Matthew Dempsky [Sat, 19 Aug 2023 00:43:15 +0000 (17:43 -0700)]
cmd/compile/internal/typecheck: refactor and simplify DeclFunc
This CL refactors typecheck.DeclFunc to require the caller to have
already constructed the ir.Func and signature type using ir.NewFunc
and types.NewSignature, and simplifies typecheck.DeclFunc to simply
return the slices of param and results ONAMEs.
typecheck.DeclFunc was the last reason that ir.Field still exists, so
this CL also gets rid of that.
Change-Id: Ib398420bac2fd135a235810b8af1635fa754965c
Reviewed-on: https://go-review.googlesource.com/c/go/+/520977
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>