forked from golang/go
-
-
Notifications
You must be signed in to change notification settings - Fork 1
[pull] master from golang:master #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
pull
wants to merge
5,284
commits into
golangFame:master
Choose a base branch
from
golang:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )