Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Feb 22, 2024

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

Mark Freeman and others added 29 commits October 23, 2025 13:16
This is a minor renaming to help differentiate the two kinds of loading
that can happen for named types (eager and lazy).

Use of the term "loaded" suggests that it might also apply to types
which are eagerly-loaded, which is not the case. This change uses
"lazyLoaded" instead to mark this difference.

Change-Id: Iee170024246d9adf3eed978bde2b0500f6d490b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/713282
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Named.resolve normalizes a named type's representation so that type
operations have a uniform surface to work with, regardless of how
the type was created.

This often gives the type a heavier footprint, such as when filling
in a lazy-loaded type from UIR, or expanding the RHS of an
instantiated type.

For that reason, it seems more appropriate to call this "unpacking"
the type, as it hints to this heavier form. The term "resolving"
is used in many contexts, and generally suggests that we are
trying to figure out what a name means.

Change-Id: Ia733fd68656380b2be4f7433b7b971b7c1422783
Reviewed-on: https://go-review.googlesource.com/c/go/+/713285
Auto-Submit: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Other panic messages in the file use the "unique.canonMap"
prefix, but this one used "internal/sync.HashTrieMap",
looks like it was copied from the "internal/sync" package.

Change-Id: Ic3e85154eb5a593d41c19013d5696afed2178b7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/713660
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
This function returns two values, one was missing in the example.

Change-Id: I738597f959011260f666c29b7d3ffad8e5cdc473
GitHub-Last-Rev: 5d99e04
GitHub-Pull-Request: #76032
Reviewed-on: https://go-review.googlesource.com/c/go/+/714420
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
This change adds a new field to the Builder struct to store a function
to retrieve the current vendor directory.  This allows us to delay the
determination of the vendor directory until later, which is currently
necessary to successful interaction with the module loader state.
This behavior will be changed in a future CL and the Builder field
will then be removed.

In addition, a new method to get the vendor dir from the module loader
state is added that will return the empty string instead of panicing.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: Ib0165edb9502d98ddfa986acf5579c1b746a026f
Reviewed-on: https://go-review.googlesource.com/c/go/+/711133
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: Iff90d83b440b39df3d598f5a999e270baa893e24
Reviewed-on: https://go-review.googlesource.com/c/go/+/711134
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This change modifies the type `QueryMatchesMainModulesError` to have
an additional field to store the current module loader state.  The
field is used to break the dependency on the global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: Ia2066fab9f3ec4ffdd3d7393dacdf65c0fd35b92
Reviewed-on: https://go-review.googlesource.com/c/go/+/711118
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change makes the following functions methods on the State:
  * CheckAllowed
  * CheckExclusions
  * CheckRetractions

Doing so allows us to reduce the use of the global state variable in
downstream function calls.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: I97147311d9de16ecac8c122c2b6bdde94bad9d8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/711119
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This change reworks the sentinel error value ErrNoModRoot and the type
noMainModulesError so that we can determine the appropriate error
message to display based on the loader state without depending on the
state directly.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: I64de433faca96ed90fad4c153766c50575a72157
Reviewed-on: https://go-review.googlesource.com/c/go/+/711120
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This change refactors the injection of the global
`modload.LoaderState` variable such that the injection can occur later
in the chain of function calls.  This allows us to keep the behavior
of the global PerPackageFlags (e.g. BuildGcFlags, etc) consistent and
avoid introducing a dependency on the module loader state there.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: I1a0cc07173a1804426392581ba8de4ae1e30cdef
Reviewed-on: https://go-review.googlesource.com/c/go/+/711115
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Use slices.DeleteFunc to remove chains with invalid policies and
incompatible key usage, instead of iterating over the chains and
reconstructing the slice.

Change-Id: I8ad2bc1ac2469d0d18b2c090e3d4f702b1b577cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/708415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: David Chase <drchase@google.com>
This is semantically identical and just a cleanup.

Prior to #63397, JSON object names were sorted according to UTF-16
to match the semantic of RFC 8785, but there were a number of
objections in the discussion to using that as the sorting order.

In go-json-experiment/json#121,
we switched to sorting by UTF-8, which matches the behavior
of v1 and avoids an option to toggle the behavior.
However, we should have deleted the stringSlice.Sort method
and just directly called slices.Sort.

From a principled perspective, both UTF-16 and UTF-8 are
reasonable ways to sort JSON object names.
RFC 8259 specifies that the entire JSON text is encoded as UTF-8.
However, the way JSON strings are encoded requires escaping
Unicode codepoints according to UTF-16 surragate halves
(a quirk of JavaScript inherited by JSON).
Thus, JSON is inconsistently both UTF-8 and UTF-16.

Change-Id: Id92b5cc20efe4201827e9d3fccf24ccf894d3e60
Reviewed-on: https://go-review.googlesource.com/c/go/+/713522
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Bypass: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Damien Neil <dneil@google.com>
Previously, we put a jsontext.Encoder (or Decoder)
back into a pool with minimal reset logic.
This was semantically safe since we always did a full reset
after obtaining an Encoder/Decoder back out of the pool.

However, this meant that so long as an Encoder/Decoder was
alive in the pool, any application data referenced by the coder
would be kept alive longer than necessary.
Explicitly, clear such fields so that application data
can be more aggressively garbage collected.

Performance:

	name               old time/op    new time/op    delta
	Unmarshal/Bool-32    52.0ns ± 3%    50.3ns ± 3%  -3.30%  (p=0.001 n=10+10)
	Marshal/Bool-32      55.4ns ± 3%    54.4ns ± 2%  -1.75%  (p=0.006 n=10+9)

This only impacts the performance of discrete Marshal/Unmarshal calls.
For the simplest possible call (i.e., to marsha/unmarshal a bool),
there is a 1-2ns slow down. This is an appropriate slowdown
for an improvement in memory utilization.

Change-Id: I5e7d7827473773e53a9dcb3d7fe9052a75481e9f
Reviewed-on: https://go-review.googlesource.com/c/go/+/713640
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
TryBot-Bypass: Damien Neil <dneil@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Auto-Submit: Damien Neil <dneil@google.com>
The standard approach to constraint checking involves checking the
constraints during chain building. This is typically done as most chain
building algorithms want to find a single chain. We don't do this, and
instead build every valid chain we can find. Because of this, we don't
_need_ to do constraint checking during the chain building stage, and
instead can defer it until we have built all of the potentially valid
chains (we already do this for EKU nesting and policy checking).

This allows us to limit the constraints we check to only chains issued
by trusted roots, which reduces the attack surface for constraint
checking, which is an annoyingly algorithmically complex process (for
now).

To maintain previous behavior, if we see an error during constraint
checking, and we end up with no valid chains, we return the first
constraint checking error, instead of a more verbose error indicating
if there were different problems during filtering. At some point we
probably should come up with a more unified error type for chain
building that can contain information about multiple failure modes.

Change-Id: I5780b3adce8538eb4c3b56ddec52f0723d39009e
Reviewed-on: https://go-review.googlesource.com/c/go/+/713240
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
This code path became useless after CL 231220.

Change-Id: I35c25368652eeb107350dcd9d1b283429ad3d5e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/714500
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
CL 708495 mass migrated the stdlib to use errors.AsType.
It rewrote the documentation, but uses the non-pointer type
of SemanticError or SyntacticError, which is invalid since
the Error method is declared on the pointer receiver.

Also, call errors.AsType on a separate line for readability.

Change-Id: I2eaf59614e2b58aa4bc8669ab81ce64168c54f13
Reviewed-on: https://go-review.googlesource.com/c/go/+/714660
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Johan Brandhorst-Satzkorn <johan.brandhorst@gmail.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently we use 64-bit hash calculations on Wasm. The 64-bit hash
calculation make intensive uses of 64x64->128 bit multiplications,
which on many 64-bit platforms are compiler intrinsics and can be
compiled to one or two instructions. This is not the case on Wasm,
so it is not very performant.

This CL makes it use 32-bit hashes on Wasm, just like other 32-bit
architectures. The 32-bit hash calculation only uses 32x32->64 bit
multiplications, which can be compiled efficiently on Wasm.

Using 32-bit hashes may increase the chance of collisions. But it
is the same as 32-bit architectures like 386. And our Wasm port
supports only 32-bit address space (like 386), so this is not too
bad.

Runtime Hash benchmark results
goos: js
goarch: wasm
pkg: runtime
                     │   0h.txt    │               1h.txt                │
                     │   sec/op    │   sec/op     vs base                │
Hash5                  20.45n ± 9%   14.06n ± 2%  -31.21% (p=0.000 n=10)
Hash16                 22.34n ± 7%   17.52n ± 1%  -21.62% (p=0.000 n=10)
Hash64                 47.47n ± 3%   28.68n ± 1%  -39.59% (p=0.000 n=10)
Hash1024               475.4n ± 1%   271.4n ± 0%  -42.92% (p=0.000 n=10)
Hash65536              28.42µ ± 1%   16.66µ ± 0%  -41.40% (p=0.000 n=10)
HashStringSpeed        40.07n ± 7%   29.23n ± 1%  -27.05% (p=0.000 n=10)
HashBytesSpeed         62.01n ± 3%   46.11n ± 4%  -25.64% (p=0.000 n=10)
HashInt32Speed         24.31n ± 2%   20.39n ± 1%  -16.13% (p=0.000 n=10)
HashInt64Speed         25.48n ± 7%   20.81n ± 7%  -18.29% (p=0.000 n=10)
HashStringArraySpeed   87.69n ± 4%   76.65n ± 2%  -12.58% (p=0.000 n=10)
FastrandHashiter       87.65n ± 1%   87.65n ± 1%        ~ (p=0.896 n=10)
geomean                90.82n        67.03n       -26.19%

Map benchmarks are too many to post here. The speedups are around
0-40%.

Change-Id: I2f7a68cfc446ab5a547fdb6a40aea07854516d51
Reviewed-on: https://go-review.googlesource.com/c/go/+/714600
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
The debug_gdb_scripts sectino may or may not be compressed. Check
for both.

For #76022.

Change-Id: I7541e0aa2b90f6b3b694e02d3dfa99f9019a9e5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/714461
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change adds fields to the type `ImportMissingError` so
that the `Error()` method can be called without accessing the global
`LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: Ib313faeb27ae44e9ac68086313633ce742f596dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/711121
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
This change modifies the type `mvsReqs` to have an additional field to
store the current module loader state.  The field is used to break the
dependency on the global `modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

Change-Id: Id6bd96bc5de68bf327f9e78a778173634e1d15d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/711122
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit modifies `run.runRun` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/run
rf '
  add run.go:/func runRun\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runRun://+0 moduleLoaderState := modload.NewState()
  rm run.go:/var moduleLoaderState \*modload.State/
'

Change-Id: I76e56798b2e0d23a0fefe65d997740e3e411d99d
Reviewed-on: https://go-review.googlesource.com/c/go/+/711116
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit modifies `bug.runBug` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/bug
rf '
  add bug.go:/func runBug\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runBug://+0 moduleLoaderState := modload.NewState()
  rm bug.go:/var moduleLoaderState \*modload.State/
'

Change-Id: Ic4f74d2127f667491136ee4bad4388b4d606821e
Reviewed-on: https://go-review.googlesource.com/c/go/+/711123
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This commit modifies `list.runList` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/list
rf '
  add list.go:/func runList\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runList://+0 moduleLoaderState := modload.NewState()
  rm list.go:/var moduleLoaderState \*modload.State/
'

Change-Id: I7f45205c1c946189eb05c6178d14ce74ae259547
Reviewed-on: https://go-review.googlesource.com/c/go/+/711124
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit modifies `modcmd.runDownload` to construct a new
modload.State object using the new constructor instead of the current
global `modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/modcmd
rf '
  add download.go:/func runDownload\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runDownload://+0 moduleLoaderState := modload.NewState()
  add runEdit://+0 moduleLoaderState := modload.NewState()
  add runGraph://+0 moduleLoaderState := modload.NewState()
  add runInit://+0 moduleLoaderState := modload.NewState()
  add runTidy://+0 moduleLoaderState := modload.NewState()
  add runVendor://+0 moduleLoaderState := modload.NewState()
  add runVerify://+0 moduleLoaderState := modload.NewState()
  add runWhy://+0 moduleLoaderState := modload.NewState()
  rm download.go:/var moduleLoaderState \*modload.State/
'

Change-Id: Id69deba173032a4d6da228eae28e38bd87176db5
Reviewed-on: https://go-review.googlesource.com/c/go/+/711125
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit modifies `modget.runGet` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/modget
rf '
  add get.go:/func runGet\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runGet://+0 moduleLoaderState := modload.NewState()
  rm get.go:/var moduleLoaderState \*modload.State/
'

Change-Id: I977c3f57c00b60d383ffd2c2c0d7bc90813c7988
Reviewed-on: https://go-review.googlesource.com/c/go/+/711126
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This commit modifies `test.runTest` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/test
rf '
  add test.go:/func runTest\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runTest://+0 moduleLoaderState := modload.NewState()
  rm test.go:/var moduleLoaderState \*modload.State/
'

Change-Id: Ia387160f30818714ab578c5b78713e532cd394df
Reviewed-on: https://go-review.googlesource.com/c/go/+/711127
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
This commit modifies `tool.runTool` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/tool
rf '
  add tool.go:/func runTool\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runTool://+0 moduleLoaderState := modload.NewState()
  rm tool.go:/var moduleLoaderState \*modload.State/
'

Change-Id: I0aff1bc2b57d973c258510dc52fb877fa824355c
Reviewed-on: https://go-review.googlesource.com/c/go/+/711128
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This commit modifies package `workcmd` to construct a new
modload.State object using the new constructor instead of the current
global `modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/workcmd
rf '
  inject modload.LoaderState runUse
  add sync.go:/func runSync\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add runSync://+0 moduleLoaderState := modload.NewState()
  add runEditwork://+0 moduleLoaderState := modload.NewState()
  add runInit://+0 moduleLoaderState := modload.NewState()
  add runUse://+0 moduleLoaderState := modload.NewState()
  add runVendor://+0 moduleLoaderState := modload.NewState()
  rm sync.go:/var moduleLoaderState \*modload.State/
'

Change-Id: Iadc4ffd19d15f80a694285c86adfc01f0d26bac7
Reviewed-on: https://go-review.googlesource.com/c/go/+/711129
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit modifies `vet.runVet` to construct a new modload.State
object using the new constructor instead of the current global
`modload.LoaderState` variable.

This commit is part of the overall effort to eliminate global
modloader state.

[git-generate]
cd src/cmd/go/internal/vet
rf '
  add vet.go:/func run\(/-0 var moduleLoaderState *modload.State
  ex {
    import "cmd/go/internal/modload";
    modload.LoaderState -> moduleLoaderState
  }
  add run://+0 moduleLoaderState := modload.NewState()
  rm vet.go:/var moduleLoaderState \*modload.State/
'

Change-Id: I31c7f3ea8d301b9210f5afeb632afb5b53e93e46
Reviewed-on: https://go-review.googlesource.com/c/go/+/711130
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Mark Freeman and others added 30 commits November 13, 2025 13:14
Change-Id: Ia315cef2022197ec740023b67827bc810219b868
Reviewed-on: https://go-review.googlesource.com/c/go/+/715361
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The existing protocol check for fcgi/cgi requests did not properly
account for Apache SSI (Server-Side Includes) SERVER_PROTOCOL value of
INCLUDED.

Added check for well-known INCLUDED value for proper implementation of
the CGI Spec as specified in RFC 3875 - section 4.1.16.

The SERVER_PROTOCOL section of the specification is outlined at
https://www.rfc-editor.org/rfc/rfc3875.html#section-4.1.16

Fixes #70416

Change-Id: I129e606147e16d1daefb49ed6c13a561a88ddeb6
Reviewed-on: https://go-review.googlesource.com/c/go/+/715680
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
Auto-Submit: Sean Liao <sean@liao.dev>
Use our local git server instead of cloud.google.com/go,
which may go down or otherwise reject our traffic.

I built and installed git 2.23.0 and confirmed that this
updated test still fails if CL 196961 is rolled back.
So the test is still accurate at detecting the problem.

Change-Id: I58011f6cc62af2f21fbbcc526ba5401f4186eeaf
Reviewed-on: https://go-review.googlesource.com/c/go/+/720322
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
isLitOrSingle and isNotToken are private and unused.

Change-Id: I07718d4496e92d5f75ed74986e174a8aa1f70a88
GitHub-Last-Rev: 722c4dc
GitHub-Pull-Request: #76216
Reviewed-on: https://go-review.googlesource.com/c/go/+/718700
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Sean Liao <sean@liao.dev>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In the proc view, the thread ID is useful. In the thread view, the proc
ID is useful. Add both in both cases forever more.

Change-Id: I9cb7bd67a21ee17d865c25d73b2049b3da7aefbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/720402
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Cq-Include-Trybots: luci.golang.try:gotip-linux-386-nosizespecializedmalloc,gotip-linux-amd64-nosizespecializedmalloc,gotip-linux-arm64-nosizespecializedmalloc
Change-Id: I6a6a696465004b939c989afc058c4c3e1fb7134f
Reviewed-on: https://go-review.googlesource.com/c/go/+/720401
Auto-Submit: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Specifically if there are experiments but no nonstandard toolchain
suffix such as "-devel", the go version will not be valid according to
go/version.IsValid. To fix that, always put the X: part into the suffix,
resulting in, for example, go1.25.0-X:foo.

Fixes #75953

Change-Id: I6a6a696468f3ba9b82b6a410fb88831428e93b58
Reviewed-on: https://go-review.googlesource.com/c/go/+/719701
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The type checker assigns types to objects. An object can be in
1 of 3 states, which are mapped to colors. This is stored as a
field on the object.

 - white : type not known, awaiting processing
 - grey  : type pending, in processing
 - black : type known, done processing

With the addition of Checker.objPathIdx, which maps objects to
a path index, presence in the map could signal grey coloring.
White and black coloring can be signaled by presence of
object.typ (a simple nil check).

This change removes the object.color field and its associated
methods, replacing it with an equivalent use of Checker.objPathIdx.
Checker.objPathIdx is integrated into the existing push and pop
methods, while slightly simplifying their signatures.

Note that the concept of object coloring remains the same - we
are merely changing and simplifying the mechanism which represents
the colors.

Change-Id: I91fb5e9a59dcb34c08ffc5b4ebc3f20a400094b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/715840
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Mark Freeman <markfreeman@google.com>
This change shouldn't have any impact on codegen. It removes some
unnecessary int8 and int64 casts from the riscv64 rewrite rules.

It also removes explicit typing where the types already match:
`(OldOp <t>) => (NewOp <t>)` is the same as `(OldOp) => (NewOp)`.

Change-Id: Ic02b65da8f548c8b9ad9ccb6627a03b7bf6f050f
Reviewed-on: https://go-review.googlesource.com/c/go/+/719220
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Commit-Queue: Junyang Shao <shaojunyang@google.com>
Optimize comparisons with constants that only differ by 1 bit (i.e.
a power of 2). For example:

    x == 4 || x == 6 -> x|2 == 6
    x != 1 && x != 5 -> x|4 != 5

Change-Id: Ic61719e5118446d21cf15652d9da22f7d95b2a15
Reviewed-on: https://go-review.googlesource.com/c/go/+/719420
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
This change fixed the overlooked offset in bytes.Buffer.Peek.
Otherwise, it will either return wrong result or panic with
"runtime error: slice bounds out of range".

Change-Id: Ic42fd8a27fb9703c51430f298933b91cf0d45451
GitHub-Last-Rev: fb97ebc
GitHub-Pull-Request: #76165
Reviewed-on: https://go-review.googlesource.com/c/go/+/717640
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
goos: linux
goarch: riscv64
pkg: cmd/compile/internal/test
cpu: Spacemit(R) X60
        │ /root/mul.base.log │          /root/mul.new.log          │
        │       sec/op       │   sec/op     vs base                │
MulNeg           6.426µ ± 0%   4.501µ ± 0%  -29.96% (p=0.000 n=10)
Mul2Neg          9.000µ ± 0%   6.431µ ± 0%  -28.54% (p=0.000 n=10)
Mul2             1.263µ ± 0%   1.263µ ± 0%        ~ (p=1.000 n=10)
MulNeg2          1.577µ ± 0%   1.577µ ± 0%        ~ (p=0.211 n=10)
geomean          3.276µ        2.756µ       -15.89%

goos: linux
goarch: amd64
pkg: cmd/compile/internal/test
cpu: AMD EPYC 7532 32-Core Processor
        │ /root/base  │              /root/new              │
        │   sec/op    │   sec/op     vs base                │
MulNeg    691.9n ± 1%   319.4n ± 0%  -53.83% (p=0.000 n=10)
Mul2Neg   630.0n ± 0%   629.6n ± 0%   -0.07% (p=0.000 n=10)
Mul2      438.1n ± 0%   438.1n ± 0%        ~ (p=0.728 n=10)
MulNeg2   439.3n ± 0%   439.4n ± 0%        ~ (p=0.656 n=10)
geomean   538.2n        443.6n       -17.58%

Change-Id: Ice8e6c8d1e8e3009ba8a0b1b689205174e199019
Reviewed-on: https://go-review.googlesource.com/c/go/+/720180
Reviewed-by: abner chenc <chenguoqi@loongson.cn>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
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>
The rule

(SLTI  [x] (ORI  [y] _)) && y >= 0 && int64(y) >= int64(x) => (MOVDconst [0])

is incorrect as it only generates correct code if the unknown value
being compared is >= 0. If the unknown value is < 0 the rule will
incorrectly produce a constant value of 0, whereas the code optimized
away by the rule would have produced a value of 1.

A new test that causes the faulty rule to generate incorrect code
is also added to ensure that the error does not return.

Change-Id: I69224e0776596f1b9538acf9dacf9009d305f966
Reviewed-on: https://go-review.googlesource.com/c/go/+/720220
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Add documentation for the riscv64 assembler with a link to the
documentation from asm.html. Architecture specific assembler
documentation is provided for the other architectures but has
been missing for riscv64 until now.

Change-Id: I62ed7e6a2a4b52e0720d869e964b29e2a980223a
Reviewed-on: https://go-review.googlesource.com/c/go/+/652717
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Meng Zhuo <mengzhuo1203@gmail.com>
Auto-Submit: Joel Sing <joel@sing.id.au>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: I0ada7fa02eb5f18a78da17bdcfc63333abbd8450
Reviewed-on: https://go-review.googlesource.com/c/go/+/713284
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
This was suggested in CL 646198, and I tested with it, I forgot to push
it so it wasn't merged. I think it might be worth keeping.

Change-Id: Ibf97d969fe7d0eeb365f5f7b1fbeadea3a1076ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/716580
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Currently weak handles are atomic.Uintptr values that may end up in
a tiny block which can cause all sorts of surprising leaks. See #76007
for one example.

This change pads out the underlying allocation of the atomic.Uintptr to
16 bytes to ensure we bypass the tiny allocator, and it gets its own
block. This wastes 8 bytes per weak handle. We could potentially do
better by using the 8 byte noscan size class, but this can be a
follow-up change.

For #76007.

Change-Id: I3ab74dda9bf312ea0007f167093052de28134944
Reviewed-on: https://go-review.googlesource.com/c/go/+/719960
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Change-Id: Ia463a9b3b5980670bcf9297b4bddb60980ebfde5
Reviewed-on: https://go-review.googlesource.com/c/go/+/720320
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Currently, AddCleanup just creates a simple closure that calls
`cleanup(arg)` as the actual cleanup function tracked internally.
However, the argument ends up getting its own allocation. If it's tiny,
then it can also end up sharing a tiny allocation slot with the object
we're adding the cleanup to. Since the closure is a GC root, we can end
up with cleanups that never fire.

This change refactors the AddCleanup machinery to make the storage for
the argument separate and explicit. With that in place, it explicitly
allocates 16 bytes of storage for tiny arguments to side-step the tiny
allocator.

One would think this would cause an increase in memory use and more
bytes allocated, but that's actually wrong! It turns out that the
current "simple closure" actually creates _two_ closures. By making the
argument passing explicit, we eliminate one layer of closures, so this
actually results in a slightly faster AddCleanup overall and 16 bytes
less memory allocated.

goos: linux
goarch: amd64
pkg: runtime
cpu: AMD EPYC 7B13
                     │ before.bench │            after.bench             │
                     │    sec/op    │   sec/op     vs base               │
AddCleanupAndStop-64    124.5n ± 2%   103.7n ± 2%  -16.71% (p=0.002 n=6)

                     │ before.bench │            after.bench            │
                     │     B/op     │    B/op     vs base               │
AddCleanupAndStop-64     48.00 ± 0%   32.00 ± 0%  -33.33% (p=0.002 n=6)

                     │ before.bench │            after.bench            │
                     │  allocs/op   │ allocs/op   vs base               │
AddCleanupAndStop-64     3.000 ± 0%   2.000 ± 0%  -33.33% (p=0.002 n=6)

This change does, however, does add 16 bytes of overhead to the cleanup
special itself, and makes each cleanup block entry 24 bytes instead of 8
bytes. This means the overall memory overhead delta with this change is
neutral, and we just have a faster cleanup. (Cleanup block entries are
transient, so I suspect any increase in memory overhead there is
negligible.)

Together with CL 719960, fixes #76007.

Change-Id: I81bf3e44339e71c016c30d80bb4ee151c8263d5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/720321
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
…faces

If the struct is a bunch of 0-sized fields and one pointer field.

Merged revert-of-revert for 4 CLs.

original  revert
681937    695016
693415    694996
693615    695015
694195    694995

Fixes #74092
Update #74888
Update #74908
Update #74935
(updated issues are bugs in the last attempt at this)

Change-Id: I32246d49b8bac3bb080972dc06ab432a5480d560
Reviewed-on: https://go-review.googlesource.com/c/go/+/714421
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
…handlers

(*body).Close() internally tries to discard the content of a request
body up to 256 KB. We rely on this behavior to allow connection re-use,
by calling (*body).Close() when our request handler exits.

Unfortunately, this causes an unfortunate side-effect where we would
prematurely try to discard a body content when (*body).Close() is called
from within a request handler.

There should not be a good reason for (*body).Close() to do this when
called from within a request handler. As such, this CL modifies
(*body).Close() to not discard body contents when called from within a
request handler. Note that when a request handler exits, it will still
try to discard the body content for connection re-use.

For #75933

Change-Id: I71d2431a540579184066dd35d3da49d6c85c3daf
Reviewed-on: https://go-review.googlesource.com/c/go/+/720380
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
…rfree=1

When run with GODEBUG=clobberfree=1, three out of seven of the top-level
tests in runtime/arena_test.go fail with a SIGSEGV inside the
clobberfree function where it is overwriting freed memory
with 0xdeadbeef.

This is not a new problem. For example, this crashes in Go 1.20:

  GODEBUG=clobberfree=1 go test runtime -run=TestUserArena

It would be nice for all.bash to pass with GODEBUG=clobberfree=1,
including it is useful for testing the automatic reclaiming of
dead memory via runtime.freegc in #74299.

Given the GOEXPERIMENT=arenas in #51317 is not planned to move forward
(and is hopefully slated to be replace by regions before too long),
for now we just skip those three tests in order to get all.bash
passing with GODEBUG=clobberfree=1.

Updates #74299

Change-Id: I384d96791157b30c73457d582a45dd74c5607ee0
Reviewed-on: https://go-review.googlesource.com/c/go/+/715080
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
This CL is part of a set of CLs that attempt to reduce how much work the
GC must do. See the design in https://go.dev/design/74299-runtime-freegc

The plan has been for GOEXPERIMENT=runtimefreegc to be disabled
by default for Go 1.26, so here we disable it.

Also, we update the name of the GOEXPERIMENT to reflect the latest name.

Updates #74299

Change-Id: I94a34784700152e13ca93ef6711ee9b7f1769d9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/720120
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This CL is part of a set of CLs that attempt to reduce how much work the
GC must do. See the design in https://go.dev/design/74299-runtime-freegc

This CL adds runtime.freegc:

 func freegc(ptr unsafe.Pointer, uintptr size, noscan bool)

Memory freed via runtime.freegc is made immediately reusable for
the next allocation in the same size class, without waiting for a
GC cycle, and hence can dramatically reduce pressure on the GC. A sample
microbenchmark included below shows strings.Builder operating roughly
2x faster.

An experimental modification to reflect to use runtime.freegc
and then using that reflect with json/v2 gave reported memory
allocation reductions of -43.7%, -32.9%, -21.9%, -22.0%, -1.0%
for the 5 official real-world unmarshalling benchmarks from
go-json-experiment/jsonbench by the authors of json/v2, covering
the CanadaGeometry through TwitterStatus datasets.

Note: there is no intent to modify the standard library to have
explicit calls to runtime.freegc, and of course such an ability
would never be exposed to end-user code.

Later CLs in this stack teach the compiler how to automatically
insert runtime.freegc calls when it can prove it is safe to do so.

(The reflect modification and other experimental changes to
the standard library were just that -- experiments. It was
very helpful while initially developing runtime.freegc to see
more complex uses and closer-to-real-world benchmark results
prior to updating the compiler.)

This CL only addresses noscan span classes (heap objects without
pointers), such as the backing memory for a []byte or string. A
follow-on CL adds support for heap objects with pointers.

If we update strings.Builder to explicitly call runtime.freegc on its
internal buf after a resize operation (but without freeing the usually
final incarnation of buf that will be returned to the user as a string),
we can see some nice benchmark results on the existing strings
benchmarks that call Builder.Write N times and then call Builder.String.
Here, the (uncommon) case of a single Builder.Write is not helped (given
it never resizes after first alloc if there is only one Write), but the
impact grows such that it is up to ~2x faster as there are more resize
operations due to more strings.Builder.Write calls:

                                               │     disabled.out │      new-free-20.txt                │
                                               │      sec/op      │   sec/op     vs base                │
BuildString_Builder/1Write_36Bytes_NoGrow-4           55.82n ± 2%   55.86n ± 2%        ~ (p=0.794 n=20)
BuildString_Builder/2Write_36Bytes_NoGrow-4           125.2n ± 2%   115.4n ± 1%   -7.86% (p=0.000 n=20)
BuildString_Builder/3Write_36Bytes_NoGrow-4           224.0n ± 1%   188.2n ± 2%  -16.00% (p=0.000 n=20)
BuildString_Builder/5Write_36Bytes_NoGrow-4           239.1n ± 9%   205.1n ± 1%  -14.20% (p=0.000 n=20)
BuildString_Builder/8Write_36Bytes_NoGrow-4           422.8n ± 3%   325.4n ± 1%  -23.04% (p=0.000 n=20)
BuildString_Builder/10Write_36Bytes_NoGrow-4          436.9n ± 2%   342.3n ± 1%  -21.64% (p=0.000 n=20)
BuildString_Builder/100Write_36Bytes_NoGrow-4         4.403µ ± 1%   2.381µ ± 2%  -45.91% (p=0.000 n=20)
BuildString_Builder/1000Write_36Bytes_NoGrow-4        48.28µ ± 2%   21.38µ ± 2%  -55.71% (p=0.000 n=20)

See the design document for more discussion of the strings.Builder case.

For testing, we add tests that attempt to exercise different aspects
of the underlying freegc and mallocgc behavior on the reuse path.
Validating the assist credit manipulations turned out to be subtle,
so a test for that is added in the next CL. There are also
invariant checks added, controlled by consts (primarily the
doubleCheckReusable const currently).

This CL also adds support in runtime.freegc for GODEBUG=clobberfree=1
to immediately overwrite freed memory with 0xdeadbeef, which
can help a higher-level test fail faster in the event of a bug,
and also the GC specifically looks for that pattern and throws
a fatal error if it unexpectedly finds it.

A later CL (currently experimental) adds GODEBUG=clobberfree=2,
which uses mprotect (or VirtualProtect on Windows) to set
freed memory to fault if read or written, until the runtime
later unprotects the memory on the mallocgc reuse path.

For the cases where a normal allocation is happening without any reuse,
some initial microbenchmarks suggest the impact of these changes could
be small to negligible (at least with GOAMD64=v3):

goos: linux
goarch: amd64
pkg: runtime
cpu: AMD EPYC 7B13
                        │ base-512M-v3.bench │   ps16-512M-goamd64-v3.bench     │
                        │        sec/op      │ sec/op     vs base               │
Malloc8-16                      11.01n ± 1%   10.94n ± 1%  -0.68% (p=0.038 n=20)
Malloc16-16                     17.15n ± 1%   17.05n ± 0%  -0.55% (p=0.007 n=20)
Malloc32-16                     18.65n ± 1%   18.42n ± 0%  -1.26% (p=0.000 n=20)
MallocTypeInfo8-16              18.63n ± 0%   18.36n ± 0%  -1.45% (p=0.000 n=20)
MallocTypeInfo16-16             22.32n ± 0%   22.65n ± 0%  +1.50% (p=0.000 n=20)
MallocTypeInfo32-16             23.37n ± 0%   23.89n ± 0%  +2.23% (p=0.000 n=20)
geomean                         18.02n        18.01n       -0.05%

These last benchmark results include the runtime updates to support
span classes with pointers (which was originally part of this CL,
but later split out for ease of review).

Updates #74299

Change-Id: Icceaa0f79f85c70cd1a718f9a4e7f0cf3d77803c
Reviewed-on: https://go-review.googlesource.com/c/go/+/673695
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
…freegc

This CL is part of a set of CLs that attempt to reduce how much work the
GC must do. See the design in https://go.dev/design/74299-runtime-freegc

This CL adds a better test of assist credit handling when heap objects
are being reused after a runtime.freegc call.

The main approach is bracketing alloc/free pairs with measurements
of the assist credit before and after, and hoping to see a net zero
change in the assist credit.

However, validating the desired behavior is perhaps a bit subtle.
To help stabilize the measurements, we do acquirem in the test code
to avoid being preempted during the measurements to reduce other code's
ability to adjust the assist credit while we are measuring, and
we also reduce GOMAXPROCS to 1.

This test currently does fail if we deliberately introduce bugs
in the runtime.freegc implementation such as if we:
- never adjust the assist credit when reusing an object, or
- always adjust the assist credit when reusing an object, or
- deliberately mishandle internal fragmentation.

The two main cases of current interest for testing runtime.freegc
are when over the course of our bracketed measurements gcBlackenEnable
is either true or false. The test attempts to exercise both of those
case by running the GC continually in the background (which we can see
seems effective based on logging and by how our deliberate bugs fail).

This passes ~10K test executions locally via stress.

A small note to the future: a previous incarnation of this test (circa
patchset 11 of this CL) did not do acquirem but had an approach of
ignoring certain measurements, which also was able to pass ~10K runs
via stress. The current version in this CL is simpler, but
recording the existence of the prior version here in case it is
useful in the future. (Hopefully not.)

Updates #74299

Change-Id: I46c7e0295d125f5884fee0cc3d3d31aedc7e5ff4
Reviewed-on: https://go-review.googlesource.com/c/go/+/717520
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For #72850

Change-Id: I07e64f05c82a34b1dadb9a72e16f5045e68cbd24
Reviewed-on: https://go-review.googlesource.com/c/go/+/720642
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Follow-up cleanup on go.dev/cl/698835. Clarify the tests by replacing
git-min-vers with a more-puposeful git-sha256 verb in the cmd tests.

Updates: #73704

Change-Id: I4361356431d567a6a3bb3a50eadfaa9e3ba37d90
Reviewed-on: https://go-review.googlesource.com/c/go/+/720481
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
…n objects

This CL is part of a set of CLs that attempt to reduce how much work the
GC must do. See the design in https://go.dev/design/74299-runtime-freegc

This CL updates the smallNoScanStub stub in malloc_stubs.go to reuse
heap objects that have been freed by runtime.freegc calls, and generates
the corresponding size-specialized code in malloc_generated.go.

This CL only adds support in the specialized mallocs for noscan
heap objects (objects without pointers). A later CL handles objects
with pointers.

While we are here, we leave a couple of breadcrumbs in mkmalloc.go on
how to do the generation.

Updates #74299

Change-Id: I2657622601a27211554ee862fce057e101767a70
Reviewed-on: https://go-review.googlesource.com/c/go/+/715761
Reviewed-by: Junyang Shao <shaojunyang@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
	$ go get golang.org/x/tools@59ff18c
	$ GOWORK=off go mod tidy
	$ GOWORK=off go mod vendor

This implies golang.org/x/sys@v0.38.0, for which I have also
updated src/go.mod for consistency.

I also upgraded x/mod@3f03020 to bring in some fixes to
code that go vet would otherwise have flagged in this CL.

This brings in a number of fixes and improvements to the analysis
tools within cmd/fix, which I will apply to std and cmd presently.

For #71859

Change-Id: I56c49e7ab28eb6ba55408a3721e9a898ee3067a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/719760
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
In the scheduler's steal path, we usleep(3) before stealing a _Prunning
P's runnext slot. Before CL 646198, we would not call usleep(3) if the P
was in _Psyscall. After CL 646198, Ps with Gs in syscalls stay in
_Prunning until stolen, meaning we might unnecessarily usleep(3) where
we didn't before. This probably isn't a huge deal in most cases, but can
cause some apparent slowdowns in microbenchmarks that frequently take
the steal path while there are syscalling goroutines.

Change-Id: I5bf3df10fe61cf8d7f0e9fe9522102de66faf344
Reviewed-on: https://go-review.googlesource.com/c/go/+/720441
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.