This will be required for #69536 but is also good hygiene and required
by go.dev/wiki/AssemblyPolicy.
> The code must be tested in our CI. This means there need to be
> builders that support the instructions, and if there are multiple (or
> fallback) paths they must be tested separately.
The new crypto/internal/impl registry lets us select alternative
implementations from both the same package and importers (such as
crypto/sha256 tests once we have crypto/internal/fips/sha256, or
crypto/hmac).
Updates #69592
Updates #69593
Change-Id: Ifea22a9fc9ccffcaf4924ff6bd08da7c9bd39e99
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
Reviewed-on: https://go-review.googlesource.com/c/go/+/614656
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Ensure separate implementations are implemented in different functions
called from Go, and that they can be turned off from a GODEBUG.
This will be necessary to test implementations separately for #69536.
Change-Id: I3e081deb7abb01b0665265e39c72fd4037dd48b3
Cq-Include-Trybots: luci.golang.try:gotip-linux-arm64-longtest,gotip-linux-amd64-longtest,gotip-linux-ppc64le_power8,gotip-linux-ppc64_power8
Reviewed-on: https://go-review.googlesource.com/c/go/+/614495
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This CL is inspired by:
https://github.com/golang/go/issues/29310#issuecomment-758768325
When I read omitempty option in encoding/xml package, I find it's
a bit different than encoding/json package.
I think it's more precise to say:
"any array, slice, map, or string of length zero."
Update #29310
Change-Id: Ia77167c3155411640224b349d4b34d0bb91ee11e
GitHub-Last-Rev: a4cf00dcc7
GitHub-Pull-Request: golang/go#69984
Reviewed-on: https://go-review.googlesource.com/c/go/+/621835
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: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Michael Pratt <mpratt@google.com>
Simplify the code and reduce global variables.
Change-Id: Id322836e8b6b6c4434136b95700ed4070ba52300
GitHub-Last-Rev: 7c9d409855
GitHub-Pull-Request: golang/go#69962
Reviewed-on: https://go-review.googlesource.com/c/go/+/621456
Reviewed-by: Michael Pratt <mpratt@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: Ian Lance Taylor <iant@google.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Fixes#69635
Change-Id: I4f8d7dafb34ccfb943c29f96c982278ab7edcd05
Reviewed-on: https://go-review.googlesource.com/c/go/+/621357
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Change some vars to consts, remove some unneeded string conversions.
Change-Id: Ib12eed11ef080c4b593c8369bb915117e7100045
Reviewed-on: https://go-review.googlesource.com/c/go/+/621838
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
For #51026Fixes#69971
Change-Id: I47f2938d20cbe9462bf738a506baedad4a7006c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/621837
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
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>
Change the description of an operand x that has a named type of sorts
by providing a description of the type structure (array, struct, slice,
pointer, etc).
For instance, given a (variable) operand x of a struct type T, the
operand is mentioned as (new):
x (variable of struct type T)
instead of (old):
x (variable of type T)
This approach is also used when a basic type is renamed, for instance
as in:
x (value of uint type big.Word)
which makes it clear that big.Word is a uint.
This change is expected to produce more informative error messages.
Fixes#69955.
Change-Id: I544b0698f753a522c3b6e1800a492a94974fbab7
Reviewed-on: https://go-review.googlesource.com/c/go/+/621458
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Simplify functionality of compositeKind (formerly: kindString) by
giving it a smaller scope. Move it into operand.go for future use
in that file. Adjust existing uses.
Change-Id: I73d04a8c0be44d9604e56bd4c0289afdcdd32238
Reviewed-on: https://go-review.googlesource.com/c/go/+/621457
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
Change-Id: I27bf98e84545746d90948dd06c4a7bd70782c49d
Reviewed-on: https://go-review.googlesource.com/c/go/+/621895
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: Keith Randall <khr@google.com>
This change improves error message for recursive types.
Currently, compilation of the [following program](https://go.dev/play/p/3ef84ObpzfG):
package main
type T1[T T2] struct{}
type T2[T T1] struct{}
returns an error:
./prog.go:3:6: invalid recursive type T1
./prog.go:3:6: T1 refers to
./prog.go:4:6: T2 refers to
./prog.go:3:6: T1
With the patch applied the error message looks like:
./prog.go:3:6: invalid recursive type T1
./prog.go:3:6: T1 refers to T2
./prog.go:4:6: T2 refers to T1
Change-Id: Ic07cdffcffb1483c672b241fede4e694269b5b79
GitHub-Last-Rev: cd042fdc38
GitHub-Pull-Request: golang/go#69574
Reviewed-on: https://go-review.googlesource.com/c/go/+/614084
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This package is no longer needed now that typeparams
are unconditionally enabled. Its declarations have been
moved into the go/{types,parser} packages.
Change-Id: Ife79a17eb9d29b076cabbf8a4b2ff2aea5edfc8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/621640
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This causes more inlining for rangefunc code.
The PAUTOHEAP case catches closure-passed-as-param
to function that returns a closure, that calls the
outer PPARAM.
Change-Id: I927cf2e1924ed2191e0ad8be9a1f2e793b6f2b38
Reviewed-on: https://go-review.googlesource.com/c/go/+/620220
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
If a function f being considered for inlining calls
one of its parameters, reduce the normal cost of that
call (57) to 17 to increase the chance that f will
be inlined and (with luck) that parameter will be
revealed as a constant function (which unblocks
escape analysis) or perhaps even be inlined.
The least-change value for that was still effective for
iter_test benchmarks was 32; however tests showed no
particular harm even when reduced as low as 7, and there
have been reports of other performance problems with
rangefunc overheads and so I picked a middling number
in hopes of warding off such reports.
Updates #69015
Change-Id: I2a525c1beffb9f88daa14caa8a622864b023675c
Reviewed-on: https://go-review.googlesource.com/c/go/+/609095
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Tim King <taking@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The spill/restore code around morestack is almost never exectued, so
we should make it as small as possible. Using 2-register loads/stores
makes sense here. Also, the offsets from SP are pretty small so the
offset almost always fits in the (smaller than a normal load/store)
offset field of the instruction.
Makes cmd/go 0.6% smaller.
Change-Id: I8845283c1b269a259498153924428f6173bda293
Reviewed-on: https://go-review.googlesource.com/c/go/+/621556
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Check presence of LSE support on ARM64 chip if we targeted it at compile time.
Related to #69124
Update #60905
Change-Id: I6fe244decbb4982548982e1f88376847721a33c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/610195
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Shu-Chun Weng <scw@google.com>
As described in issue #69912, type checking dot-imported identifiers can
result in a call to objDecl on an imported object, which leads to a data
race to the color_ field.
There are multiple potential fixes for this race. Opt for avoiding the
call to objDecl altogether, rather than setting color_ during import.
The color_ field is an internal property of objects that should only be
valid during the type checking of their package. We should not be
calling objDecl on imported objects.
Fixes#69912
Change-Id: I55eb652479715f2a7ac84104db2f448091c4e7ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/621637
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
For #54766.
Cq-Include-Trybots: luci.golang.try:gotip-linux-ppc64_power10,gotip-linux-amd64-longtest-swissmap
Change-Id: I6695c0b143560d974b710e1d78e7a7d09278f7cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/620215
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently, for Wasm, the linker sets the initial memory size to
the size of global data plus 16 MB. The intention is that it
covers the global data and runtime initialization without growing
the linear memory. However, the code accounts only the data
"section", not the bss "section", therefore the extra 16 MB is
actually used to cover bss variables. Also, as seen on the
previous CL, the runtime actually didn't use the extra space,
which means the program can start without that space.
This CL corrects the global data size calculation, and reduces the
extra to 1 MB. Currently the runtime's allocation pattern at
startup is that it allocates a few pages for the page allocator's
metadata, the an 8 MB reservation for the first 4 MB size, 4 MB
aligned heap arena (it may be possible to reduce that, but we'll
leave that for later). Here we use 1 MB extra space to cover the
small allocations, but let the runtime allocate the heap arena, so
the linker code and the runtime's allocator are not tightly
coupled.
For #69018.
Change-Id: I39fe1172382ecc03f4b537e43ec710af8075eab3
Reviewed-on: https://go-review.googlesource.com/c/go/+/621636
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CL 476717 adopted the memory management mechanism on Plan 9 to
manage Wasm's linear memory. But the Plan 9 code uses global
variable bloc and blocMax to keep track of the runtime's and the
OS's sense of break, whereas the Wasm sbrk function doesn't use
those global variables, and directly goes to grow the linear
memory instead. This causes that if there is any unused portion at
the end of the linear memory, the runtime doesn't use it. This CL
fixes it, adopts the same mechanism as the Plan 9 code.
In particular, the runtime is not aware of any unused initial
memory at startup. Therefore, (most of) the extra initial memory
set by the linker are not actually used. This CL fixes this as
well.
For #69018.
Change-Id: I2ea6a138310627eda5f19a1c76b1e1327362e5f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/621635
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change switches isSending to be an atomic.Int32 instead of an
atomic.Uint8. The Int32 version is managed as a counter, which is
something that we couldn't do with Uint8 without adding a new intrinsic
which may not be available on all architectures.
That is, instead of only being able to support 8 concurrent timer
firings on the same timer because we only have 8 independent bits to set
for each concurrent timer firing, we can now have 2^31-1 concurrent
timer firings before running into any issues. Like the fact that each
bit-set was matched with a clear, here we match increments with
decrements to indicate that we're in the "sending on a channel" critical
section in the timer code, so we can report the correct result back on
Stop or Reset.
We choose an Int32 instead of a Uint32 because it's easier to check for
obviously bad values (negative values are always bad) and 2^31-1
concurrent timer firings should be enough for anyone.
Previously, we avoided anything bigger than a Uint8 because we could
pack it into some padding in the runtime.timer struct. But it turns out
that the type that actually matters, runtime.timeTimer, is exactly 96
bytes in size. This means its in the next size class up in the 112 byte
size class because of an allocation header. We thus have some free space
to work with. This change increases the size of this struct from 96
bytes to 104 bytes.
(I'm not sure if runtime.timer is often allocated directly, but if it
is, we get lucky in the same way too. It's exactly 80 bytes in size,
which means its in the 96-byte size class, leaving us with some space to
work with.)
Fixes#69969.
Related to #69880 and #69312.
Change-Id: I9fd59cb6a69365c62971d1f225490a65c58f3e77
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/621616
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CL 594596 already did this for regabi, but missed non-regabi.
Stack allocated swiss maps don't call rand32.
For #54766.
Change-Id: I312ea77532ecc6fa860adfea58ea00b01683ca69
Reviewed-on: https://go-review.googlesource.com/c/go/+/621615
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Simplify populateMaps with sync.OnceValues.
Change-Id: Id52e6e1623c621b8d51e11fecbe3f1fab1e74eb4
GitHub-Last-Rev: 3cf736ae29
GitHub-Pull-Request: golang/go#69946
Reviewed-on: https://go-review.googlesource.com/c/go/+/621255
Reviewed-by: Michael Pratt <mpratt@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: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Remove linkname directives that are no longer necessary given
parquet-go/parquet-go#142 removes the dependency on the `memhash{32,64}`
functions.
This change also removes references to segmentio/parquet-go since that
repository was archived in favor of parquet-go/parquet-go.
Updates #67401
Change-Id: Ibafb0c41b39cdb86dac5531f62787fb5cb8d3f01
GitHub-Last-Rev: e14c4e4dfe
GitHub-Pull-Request: golang/go#67784
Reviewed-on: https://go-review.googlesource.com/c/go/+/589795
Auto-Submit: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
CL 603736 added a check looking for a specific trampoline symbol
name. PPC64 uses a slightly different name for the trampoline,
update the test to accept both.
Change-Id: I177dadb25d82bc3ffeb7530d7ab865482d907d34
Reviewed-on: https://go-review.googlesource.com/c/go/+/621455
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Paul Murphy <murp@ibm.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Update doc for Null[T] to clarify that T should be one of the types
accepted by driver.Value.
Modify the Value() method of Null[T]:
1) recognize T implementing driver.Valuer interface and invoke it.
2) use the DefaultParameterConverter to convert native types that
are not directly supported as driver.Value types.
Fixes#69728Fixes#69837
Change-Id: Iba782c878b2bde168125f5390abf319b88424149
GitHub-Last-Rev: 3df182d23d
GitHub-Pull-Request: golang/go#69938
Reviewed-on: https://go-review.googlesource.com/c/go/+/620858
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Change-Id: I7ff869e21e67cf6a193f7a92bf7b05f047ee005c
GitHub-Last-Rev: bf01f58249
GitHub-Pull-Request: golang/go#69957
Reviewed-on: https://go-review.googlesource.com/c/go/+/620778
Reviewed-by: Michael Pratt <mpratt@google.com>
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>
Whenn O_TRUNC is set, Opentat ends up calling syscall.Ftruncate, which
needs write access. Make sure write access is not removed when O_TRUNC
and O_APPEND are both set.
Updates #67002.
Change-Id: Iccc470b7be3c62144318d6a707057504f3b74c97
Reviewed-on: https://go-review.googlesource.com/c/go/+/620576
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Change-Id: I351b88276307c8d21e43b5b992b30b64996d129c
Reviewed-on: https://go-review.googlesource.com/c/go/+/620777
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
On ELF, default to "-B gobuildid", so it generates GNU build ID
based on Go buildid by default.
Updates #41004.
Fixes#63934.
Fixes#68652.
Change-Id: I5619dfaa4eeb6575c52922ae1de3430b46e31db6
Reviewed-on: https://go-review.googlesource.com/c/go/+/618601
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
This is similar to CL 618597, but for GNU build ID on ELF. This
makes it possible to enable "-B gobuildid" by default on ELF.
Updates #41004.
For #63934.
Change-Id: I4e663a27a2f7824bce994c783fe6d9ce8d1a395a
Reviewed-on: https://go-review.googlesource.com/c/go/+/618600
Reviewed-by: Than McIntosh <thanm@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently, on Mach-O, the -B UUID setting is only applied in
internal linking mode, whereas in external linking mode the UUID
is always rewritten to a hash of Go build ID. This CL makes it
apply to external linking as well. This makes the behavior
consistent on both linkmodes, and also consistent with the -B
flag's behavior for GNU build ID on ELF.
Add tests.
Updates #68678.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13
Change-Id: I276a5930e231141440cdba16e8812df28ac4237b
Reviewed-on: https://go-review.googlesource.com/c/go/+/618599
Reviewed-by: Than McIntosh <thanm@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
On Mach-O, default to "-B gobuildid", so it generates the UUID
based on Go buildid by default.
Fixes#68678.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13
Change-Id: I6c1a6bcafd8370a13174657e05d7d9620a8d4f12
Reviewed-on: https://go-review.googlesource.com/c/go/+/618598
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
With the "-B gobuildid" linker option (which will be the default
on some platforms), the host build ID (GNU build ID, Mach-O UUID)
depends on the Go buildid. If the host build ID is included in the
Go buildid computation, it will lead to convergence problem for
the toolchain binaries. So ignore the host build ID in the buildid
computation.
This CL only handles Mach-O UUID. ELF GNU build ID will be handled
later.
For #68678.
For #63934.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13
Change-Id: Ie8ff20402a1c6083246d25dea391140c75be40d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/618597
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
Currently the linker has some code handling and manipulating
Mach-O files. Specifically, it augments the debug/macho package
with file offset and length, so the content can be handled or
updated easily with the file.
Move this code to an internal package, so it can be used by other
part of the toolchain, e.g. buildid computation.
For #68678.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13
Change-Id: I2311af0a06441b7fd887ca5c6ed9e6fc44670a16
Reviewed-on: https://go-review.googlesource.com/c/go/+/618596
Reviewed-by: Than McIntosh <thanm@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently, on Mach-O, the Go linker doesn't generate LC_UUID in
internal linking mode. This causes some macOS system tools unable
to track the binary, as well as in some cases the binary unable
to access local network on macOS 15.
This CL makes the linker start generate LC_UUID. Currently, the
UUID is generated if the -B flag is specified. And we'll make it
generate UUID by default in a later CL. The -B flag is currently
for generating GNU build ID on ELF, which is a similar concept to
Mach-O's UUID. Instead of introducing another flag, we just use
the same flag and the same setting. Specifically, "-B gobuildid"
will generate a UUID based on the Go build ID.
For #68678.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64_14,gotip-darwin-arm64_13
Change-Id: I90089a78ba144110bf06c1c6836daf2d737ff10a
Reviewed-on: https://go-review.googlesource.com/c/go/+/618595
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ingo Oeser <nightlyone@googlemail.com>
Reviewed-by: Than McIntosh <thanm@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This is a peace-of-mind change to make sure that delayed-zeroed memory
(in the large alloc case) is globally visible from the moment the
allocation is published back to the caller.
The way it's written right now is good enough for the garbage collector
(we already have a publication barrier for a nil span.largeType, so the
GC will ignore the noscan span) but this might matter for user code on
weak memory architectures.
Change-Id: I06ac9b95863074e5f09382629083b19bfa87fdb8
Reviewed-on: https://go-review.googlesource.com/c/go/+/619036
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Last CL we separated mallocgc into several specialized paths. Let's
split up heapSetType too. This will make the specialized heapSetType
functions inlineable and cut out some branches as well as a function
call.
Microbenchmark results at this point in the stack:
│ before.out │ after-5.out │
│ sec/op │ sec/op vs base │
Malloc8-4 13.52n ± 3% 12.15n ± 2% -10.13% (p=0.002 n=6)
Malloc16-4 21.49n ± 2% 18.32n ± 4% -14.75% (p=0.002 n=6)
MallocTypeInfo8-4 27.12n ± 1% 18.64n ± 2% -31.30% (p=0.002 n=6)
MallocTypeInfo16-4 28.71n ± 3% 21.63n ± 5% -24.65% (p=0.002 n=6)
geomean 21.81n 17.31n -20.64%
Change-Id: I5de9ac5089b9eb49bf563af2a74e6dc564420e05
Reviewed-on: https://go-review.googlesource.com/c/go/+/614795
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Right now mallocgc is a monster of a function. In real programs, we see
that a substantial amount of time in mallocgc is spent in mallocgc
itself. It's very branch-y, holds a lot of state, and handles quite a few
disparate cases, trying to merge them together.
This change breaks apart mallocgc into separate, leaner functions.
There's some duplication now, but there are a lot of branches that can
be pruned as a result.
There's definitely still more we can do here. heapSetType can be inlined
and broken down for each case, since its internals roughly map to each
case anyway (done in a follow-up CL). We can probably also do more with
the size class lookups, since we know more about the size of the object
in each case than before.
Below are the savings for the full stack up until now.
│ after-3.out │ after-4.out │
│ sec/op │ sec/op vs base │
Malloc8-4 13.32n ± 2% 12.17n ± 1% -8.63% (p=0.002 n=6)
Malloc16-4 21.64n ± 3% 19.38n ± 10% -10.47% (p=0.002 n=6)
MallocTypeInfo8-4 23.15n ± 2% 19.91n ± 2% -14.00% (p=0.002 n=6)
MallocTypeInfo16-4 25.86n ± 4% 22.48n ± 5% -13.11% (p=0.002 n=6)
MallocLargeStruct-4 270.0n ± ∞ ¹
geomean 20.38n 30.97n -11.58%
Change-Id: I681029c0b442f9221c4429950626f06299a5cfe4
Reviewed-on: https://go-review.googlesource.com/c/go/+/614257
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This change breaks out the debug.malloc codepaths into dedicated
functions, both for making mallocgc easier to read, and to reduce the
function's size (currently all that code is inlined and really doesn't
need to be).
This is a microoptimization that on its own changes very little, but
together with other optimizations and a breaking up of the various
malloc paths will matter all together ("death by a thousand cuts").
Change-Id: I30b3ab4a1f349ba85b4a1b5b2c399abcdfe4844f
Reviewed-on: https://go-review.googlesource.com/c/go/+/617879
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
These debug checks are very occasionally helpful, but they do cost real
time. The biggest issue seems to be the bloat of mallocgc due to the
"throw" paths. Overall, after some follow-ups, this change cuts about
1ns off of the mallocgc fast path.
This is a microoptimization that on its own changes very little, but
together with other optimizations and a breaking up of the various
malloc paths will matter all together ("death by a thousand cuts").
Change-Id: I07c4547ad724b9f94281320846677fb558957721
Reviewed-on: https://go-review.googlesource.com/c/go/+/617878
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
shouldhelpgc is a very unhelpful name, because it has nothing to do with
assists and solely to do with GC triggering. Name it checkGCTrigger
instead, which is much clearer.
Change-Id: Id38debd424ddb397376c0cea6e74b3fe94002f71
Reviewed-on: https://go-review.googlesource.com/c/go/+/617877
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
This change stops tracking assistG across malloc to reduce number of
slots the compiler must keep track of in mallocgc, which adds to
register pressure. It also makes the call to deductAssistCredit only
happen if the GC is running.
This is a microoptimization that on its own changes very little, but
together with other optimizations and a breaking up of the various
malloc paths will matter all together ("death by a thousand cuts").
Change-Id: I4cfac7f3e8e873ba66ff3b553072737a4707e2c2
Reviewed-on: https://go-review.googlesource.com/c/go/+/617876
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
This is an allocator microoptimization. There's no reason to check
gcphase in general, since it's mostly for debugging anyway.
writeBarrier.enabled is set in all the same cases here, and we force one
fewer cache line (probably) to be touched during malloc.
Conceptually, it also makes a bit more sense. The allocate-black policy
is partly informed by the write barrier design.
Change-Id: Ia5ff593d64c29cf7f4d1bced3204056566444a98
Reviewed-on: https://go-review.googlesource.com/c/go/+/617875
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Checking whether the current allocation needs to be profiled is
currently branch-y and weirdly a lot of code. The branches are
generally predictable, but it's a surprising number of instructions.
Part of the problem is that MemProfileRate is just a global that can be
set at any time, so we need to load it and check certain settings
explicitly. In an ideal world, we would just always subtract from
nextSample and have a single branch to take the slow path if we
subtract below zero.
If MemProfileRate were a function, we could trash all the nextSample
values intentionally in each mcache. This would be slow, but
MemProfileRate changes rarely while the malloc hot path is well, hot.
Unfortunate...
Although this ideal world is, AFAICT, impossible, we can still get
close. If we cache the value of MemProfileRate in each mcache, then we
can force malloc to take the slow path whenever MemProfileRate changes.
This does require two additional loads, but crucially, these loads are
independent of everything else in mallocgc. Furthermore, the branch
dependent on those loads is incredibly predictable in practice.
This CL on its own has little-to-no impact on mallocgc. But this
codepath is going to be duplicated in several places in the next CL, so
it'll pay to simplify it. Also, we're very much trying to remedy a
death-by-a-thousand-cuts situation, and malloc is currently still kind
of a monster -- it will not help if mallocgc isn't really streamlined
itself.
Lastly, there's a nice property now that all nextSample values get
immediately re-sampled when MemProfileRate changes.
Change-Id: I6443d0cf9bd7861595584442b675ac1be8ea3455
Reviewed-on: https://go-review.googlesource.com/c/go/+/615815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Fixes#69941
Change-Id: Icc3c88d57c14fa9ca203c693d67f144686fed8cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/621076
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
