This addresses comments from CL 410356.
For #48409.
For #51400.
Change-Id: I03560e820a06c0745700ac997b02d13bc03adfc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/410735
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Chris Hines <chris.cs.guy@gmail.com>
Reviewed-by: Russ Cox <rsc@golang.org>
For #51400
Change-Id: I964e52e0a36e7bbe77175670e93ce8c99e7dab6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/410367
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Fill in the details of outstanding TODO items for go/types changes.
For #51400
Change-Id: Ib40d75fa1018aa164022cb49b293795dd597d49d
Reviewed-on: https://go-review.googlesource.com/c/go/+/410815
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Use the program counter to compute the address of the first instruction
of the ret sled. The ret sled is located after 5 instructions from the
MOVD instruction saving the value of the program counter.
Change-Id: Ie7ae7a0807785d6fea035cf7a770dba7f37de0ec
GitHub-Last-Rev: 2719208c6a
GitHub-Pull-Request: golang/go#53039
Reviewed-on: https://go-review.googlesource.com/c/go/+/407895
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Both GNU and LLVM linkers de facto accept `-zPARAM`, and Go sometimes
does it. Inconsistently: there are more uses of `-z PARAM` than
`-zPARAM`:
$ git grep -E -- '-Wl,-z[^,]' master | wc -l
4
$ git grep -E -- '-Wl,-z,' master | wc -l
7
However, not adding a space between `-z` and the param is not
documented:
llvm-13:
$ man ld.lld-13 | grep -E -A1 -w -- "^ +-z"
-z option
Linker option extensions.
gnu ld:
$ man ld | grep -E -A1 -w -- "^ +-z"
-z keyword
The recognized keywords are:
--
-z defs
Report unresolved symbol references from regular object files. This is done even if the linker is creating a non-symbolic
--
-z muldefs
Normally when a symbol is defined multiple times, the linker will report a fatal error. These options allow multiple definitions
--
-z
--imagic
... and thus should be avoided.
`zig cc`, when used as the C compiler (`CC="zig cc" go build ...`), will
bark, because `zig cc` accepts only `-z PARAM`, as documented.
Closesziglang/zig#11669
Change-Id: I758054ecaa3ce01a72600bf65d7f7b5c3ec46d09
GitHub-Last-Rev: e068e007da
GitHub-Pull-Request: golang/go#53030
Reviewed-on: https://go-review.googlesource.com/c/go/+/407834
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Boringcrypto has never been officially supported and it remains unsupported.
It need not be mentioned in the release notes.
Change-Id: I24a08d424982615244d51c1d250035d85a602023
Reviewed-on: https://go-review.googlesource.com/c/go/+/410362
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
CL 410244 changes relnote to look for api file changes as well
as references to proposal issues, finding various things that
were missing from the release notes.
This CL adds the TODOs that the updated relnote found.
For #51400.
Change-Id: I512a9b8f1349a6c68c8a6979f55a07964d630175
Reviewed-on: https://go-review.googlesource.com/c/go/+/410361
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change resolves some TODOs in the release notes, and while we're
here, also clarifies how CPU profile samples are represented in runtime
traces.
Change-Id: Idaa36ccf65b03fd5463b2d5da682d3fa578d2f46
Reviewed-on: https://go-review.googlesource.com/c/go/+/410356
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
A few of these are copied from the memory model doc.
Many are entirely new, following discussion on #47141.
See https://research.swtch.com/gomm for background.
The rule we are establishing is that each type that is meant
to help synchronize a Go program should document its
happens-before guarantees.
For #50859.
Change-Id: I947c40639b263abe67499fa74f68711a97873a39
Reviewed-on: https://go-review.googlesource.com/c/go/+/381316
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
In a set of 55M Go doc comments drawn from the latest version of
all public Go modules known to the module proxy in spring 2020,
the current Go 1.19 gofmt reformats about 1.57M of them.
Out of those 1.57M comments, inspection of random samples
shows that around 5% of the changed comments contain
unindented code snippets, multiline shell commands, or lists.
For example:
// Here is a greeting:
//
// func main() {
// fmt.Println("hello, world")
// }
// Run this command:
//
// path/to/your/program -flag1=longargument1 \
// -flag2=longargument2 \
// -flag3
// There are three possibilities:
//
// - Unindented code snippets (or JSON objects)
// in which the first and last line are unindented
// but end in { and start with }, respectively.
// - Unindented multiline shell commands
// in which the lines end in \
// - Unindented lists, in which wrapped lines are indented.
All three of these cases involve unindented lines next to indented
lines that would according to the usual rules begin a pre block.
Before this CL, they'd be reformatted to:
// Here is a greeting:
//
// func main() {
//
// fmt.Println("hello, world")
//
// }
// Run this command:
//
// path/to/your/program -flag1=longargument1 \
//
// -flag2=longargument2 \
// -flag3
// There are three possibilities:
//
// - Unindented code snippets (or JSON objects)
//
// in which the first and last line are unindented
// but end in { and start with }, respectively.
//
// - Unindented multiline shell commands
//
// in which the lines end in \
//
// - Unindented lists, in which wrapped lines are indented.
The fact that they are not already in canonical format gives us
a signal that they might not mean what the usual rules would say.
This CL takes advantage of that opening to apply a few heuristics
to better handle these cases:
1. If an indented code block immediately follows (without a blank line)
an unindented line ending in { or \, include the unindented line
in the code block.
2. If an indented code block immediately precedes (without a blank line)
an unindented line beginning with }, include the unindented line
in the code block.
3. If an indented line immediately follows (without a blank line)
an unindented line that starts with a list marker, assume this is
an unindented list with a wrapped indented line, and treat all
adjacent unindented lines starting with list markers as part of
the list, stopping at any surrounding blank lines.
This raises the fraction of “correctly” reformatted doc comments
in the corpus from approximately 87% to approximately 93%.
Change-Id: I7ac542eb085032d607a7caf3ba9020787b2978b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/410360
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 381315 added major revisions but neglected to update the date.
For #50859.
Change-Id: I086a55f0c80579c479bca5268109c9f3ae680adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/410675
Reviewed-by: Roland Shoemaker <roland@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
If we're trying to search in a module in the module cache, instead
iterate over the packages in the index.
Change-Id: Ia94cbe6e9690110c28b93dbb33810680e3010381
Reviewed-on: https://go-review.googlesource.com/c/go/+/403756
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Peter Weinberger <pjw@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
This CL addresses the comments on CL 403154.
For #51940.
Change-Id: I99bb3530916d469077bfbd53095bfcd1d2aa82ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/403976
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
``` is Markdown, not Go doc comment, but some small fraction of users get confused.
In a set of 55M Go doc comments drawn from the latest version of
all public Go modules known to the module proxy in spring 2020,
the current Go 1.19 gofmt reformats about 1.57M of them.
Out of those 1.57M comments, 8k of them (about 0.5%) contain ```.
Instead of rewriting ``` to “`, leave it alone.
For #51082.
Change-Id: I1c8c88aac7ef75ec03e1a396b84ffe711c46f941
Reviewed-on: https://go-review.googlesource.com/c/go/+/410359
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
A CL in the website repo will add go.dev/doc/comment.
One of the final steps for #51082.
Change-Id: I419b4f6dbb424a8a93a8d09db30f7321af9ae976
Reviewed-on: https://go-review.googlesource.com/c/go/+/410358
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
This appeases Go 1.4, making it possible to bootstrap GOARCH=riscv64 with
a Go 1.4 compiler.
Fixes#52583
Change-Id: Ib13c2afeb095b2bb1464dcd7f1502574209bc7ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/409974
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
strings for 'NaN' -> string for 'NaN'
Change-Id: Ia415644a1b651e6ef9996ad24dd9708a60e57dfc
GitHub-Last-Rev: 877f1c3eb1
GitHub-Pull-Request: golang/go#53246
Reviewed-on: https://go-review.googlesource.com/c/go/+/410494
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
When building CGO internal linking on windows 386, make sure to avoid
rewriting references to "_main" to "main" when reading symbols during
host object loading; the main routine defined by the Go runtime is
still named "_main" (not "main"). If we don't do this, we wind up with
an SXREF symbol named "main", which can then cause the loader to pull
an actual "main" symbol out of a host archive, which is undesirable.
Updates #35006.
Change-Id: I3768e3617b560552f4522e9e72af879c6adf7705
Reviewed-on: https://go-review.googlesource.com/c/go/+/410124
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Now that validType is using the correct type nest (CL 409694),
the top entry of the type nest corresponds to the instantiated
type. Thus we can use that type instance to look up the value
of type parameters, there's no need anymore to create an environment
to look up type arguments.
Remove the need to pass around the environment and remove all
associated types and functions.
Updates #52698.
Change-Id: Ie37eace88896386e667ef93c77a4fc3cd0be6eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/410294
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
validType was using a global type info map to detect invalid recursive
types, which was incorrect. Instead, change the algorithm as follows:
- Rather than using a "seen" (or typeInfo) map which is cumbersome to
update correctly, use the stack of embedding types (the type nest)
to check whether a type is embedded within itself, directly or
indirectly.
- Use Identical for type comparisons which correctly considers identity
of instantiated generic types.
- As before, maintain the full path of types leading to a cycle. But
unlike before, track the named types rather than their objects, for
a smaller slice ([]*Named rather than []Object), and convert to an
object list only when needed for error reporting.
- As an optimization, keep track of valid *Named types (Checker.valids).
This prevents pathological cases from consuming excessive computation
time.
- Add clarifying comments and document invariants.
Based on earlier insights by David Chase (see also CL 408818).
Fixes#52698.
Change-Id: I5e4598c58afcf4ab987a426c5c4b7b28bdfcf5ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/409694
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Add TODO items for significant changes to go/types: the inclusion of
Origin methods for Var and Func, and a re-working of Named types to
ensure finiteness of reachable types via their API.
Updates #51400
Change-Id: I0f2a972023a5d5f995de3c33e9e2b0a4213e900a
Reviewed-on: https://go-review.googlesource.com/c/go/+/410614
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently runtime.Breakpoint generates a SIGSEGV in ppc64.
The solution is an unconditional trap similar to what clang and gcc do. It is documented in the section C.6 of the ABI Book 3.
Fixes#52101
Change-Id: I071d2f2679b695ef268445b04c9222bd74e1f9af
GitHub-Last-Rev: fff4e5e8ff
GitHub-Pull-Request: golang/go#52102
Reviewed-on: https://go-review.googlesource.com/c/go/+/397554
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Paul Murphy <murp@ibm.com>
Run-TryBot: Paul Murphy <murp@ibm.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Before this change, `startParse` would write `lex.breakOK` and `lex.continueOK` when the lexer goroutine is already running, which is a potential race condition.
Makes `breakOK` and `continueOK` configuration flags passed when `lexer` is created, similarly to how `emitComment` works.
Fixes#53234
Change-Id: Ia65f6135509a758cd4c5a453b249a174f4fb3e21
Reviewed-on: https://go-review.googlesource.com/c/go/+/410414
Reviewed-by: Eli Bendersky <eliben@google.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
During type-checking, newly created instances share a type checking
Context which de-duplicates identical instances. However, when
unexpanded types escape the type-checking pass or are created via calls
to Instantiate, they lack this shared context. As reported in #52728,
this may lead to infinitely many identical but distinct types that are
reachable via the API.
This CL introduces a new invariant that ensures we don't create such
infinitely expanding chains: instances created during expansion share a
context with the type that led to their creation. During expansion, the
expanding type passes its Context to any newly created instances.
This ensures that cycles will eventually terminate with a previously
seen instance. For example, if we have an instantiation chain
T1[P]->T2[P]->T3[P]->T1[P], by virtue of this Context passing the
expansion of T3[P] will find the instantiation T1[P].
In general, storing a Context in a Named type could lead to pinning
types in memory unnecessarily, but in this case the Context pins only
those types that are reachable from the original instance. This seems
like a reasonable compromise between lazy and eager expansion.
Our treatment of Context was a little haphazard: Checker.bestContext
made it easy to get a context at any point, but made it harder to reason
about which context is being used. To fix this, replace bestContext with
Checker.context, which returns the type-checking context and panics on a
nil receiver. Update all call-sites to verify that the Checker is
non-nil when context is called.
Also make it a panic to call subst with a nil context. Instead, update
subst to explicitly accept a local (=instance) context along with a
global context, and require that one of them is non-nil. Thread this
through to the call to Checker.instance, and handle context updating
there.
Fixes#52728
Change-Id: Ib7f26eb8c406290325bc3212fda25421a37a1e8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/404885
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Separate instance information into an instance struct, to reduce memory
footprint for non-instance Named types. This may induce a sense of
deja-vu: we had a similar construct in the past that was removed as
unnecessary. With additional new fields being added that only apply to
instances, having a separate struct makes sense again.
Updates #52728
Change-Id: I0bb5982d71c27e6b574bfb4f7b886a6aeb9c5390
Reviewed-on: https://go-review.googlesource.com/c/go/+/404884
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In order to clean up context after fully expanding a type (in subsequent
CLs), we must use a common mutex. Eliminate the lazy methodList type,
which keeps a sync.Once per method, in favor of Named.mu.
Updates #52728
Change-Id: I2d13319276df1330ee53046ef1823b0167a258d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/404883
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Introduce a monotonic state variable to track the lifecycle of a named
type, replacing the existing sync.Once. Having a single guard for the
state of underlying and methods will allow for cleaning-up when the type
is fully expanded. In the future, this state may also be used for
detecting access to information such as underlying or methods before the
type is fully set-up, though that will require rethinking our
type-checking of invalid cyclic types.
Also remove support for type-type inference. If we ever support this
feature in the future (inference of missing type arguments for named
type instances), it will likely involve additional machinery that does
not yet exist. Remove the current partial support to simplify our
internal APIs. In particular, this means that Named.resolver is only
used for lazy loading. As a result, we can revert the lazy loader
signature to its previous form.
A lot of exposition is added for how Named types work. Along the way,
the terminology we use to describe them is refined.
Some microbenchmarks are added that were useful in evaluating the
tradeoffs between synchronization mechanisms.
Updates #52728
Change-Id: I4e147360bc6e5d8cd4f37e32e86fece0530a6480
Reviewed-on: https://go-review.googlesource.com/c/go/+/404875
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This resolves legacy go binaries crashing the buildid tool when the -w flag is specified.
Fixes#50809
Change-Id: I55a866f285a3c2cebcf2cdbb9cc30e5078e1d18f
GitHub-Last-Rev: 7169a58fd7
GitHub-Pull-Request: golang/go#53163
Reviewed-on: https://go-review.googlesource.com/c/go/+/409535
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The documentation for strconv.ParseFloat mentions that it "accepts
decimal and hexadecimal floating-point number syntax", but it doesn't
specify what those formats entail. For example, "0x10" is not allowed;
you need an explicit exponent, as in "0x10p0".
This clarifies that ParseFloat accepts the Go syntax for floating-point
literals, and links to that spec section. I've also linked to the
relevant spec section for ParseInt's doc comment, which already said
"as defined by the Go syntax for integer literals".
Change-Id: Ib5d2b408bdd01ea0b9f69381a9dbe858f6d1d424
Reviewed-on: https://go-review.googlesource.com/c/go/+/410335
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
statment -> statement
Change-Id: Ia93a466fdc20157a7d6048903e359fe8717ecb8f
GitHub-Last-Rev: 0a9bc5cab0
GitHub-Pull-Request: golang/go#53231
Reviewed-on: https://go-review.googlesource.com/c/go/+/410374
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Following discussion on #47141, make the following changes:
- Document Go's overall approach.
- Document that multiword races can cause crashes.
- Document happens-before for runtime.SetFinalizer.
- Document (or link to) happens-before for more sync types.
- Document happens-before for sync/atomic.
- Document disallowed compiler optimizations.
See also https://research.swtch.com/gomm for background.
Fixes#50859.
Change-Id: I17d837756a77f4d8569f263489c2c45de20a8778
Reviewed-on: https://go-review.googlesource.com/c/go/+/381315
Reviewed-by: Ian Lance Taylor <iant@google.com>
Rolled back in CL 410133.
For #51115.
Change-Id: I009c557acf98a98a9e5648fa82d998d41974ae60
Reviewed-on: https://go-review.googlesource.com/c/go/+/410357
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
We are having a hard time deciding the exact semantics
of the Err field, and we need to ship the beta.
So revert the Err field change; it can wait for Go 1.20.
For #51115.
This reverts CL 396215.
Change-Id: I7719386567d3da10a614058a11f19dbccf304b4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/410133
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
As of this CL, release notes for all packages owned by @golang/runtime
on https://dev.golang.org/owners are either complete or have explicit
TODOs.
For #51400
Change-Id: I5b6affd43883991a3b8a065b4aa211efce7427f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/410118
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
This documents most of the changes in runtime packages, which the major
exception of GC changes, which will be documented in a future CL.
For #51400
Change-Id: Ibcf501e1b4f7caa3397db6b9136daec07aac5a65
Reviewed-on: https://go-review.googlesource.com/c/go/+/410117
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently the GC CPU limiter only tracks idle GC work time. However, in
very undersubscribed situations, it's possible that all this extra idle
time prevents the enabling of the limiter, since it all gets account for
as mutator time. Fix this by tracking all idle time via pidleget and
pidleput. To support this, pidleget and pidleput also accept and return
"now" parameters like the timer code.
While we're here, let's clean up some incorrect assumptions that some of
the scheduling code makes about "now."
Fixes#52890.
Change-Id: I4a97893d2e5ad1e8c821f8773c2a1d449267c951
Reviewed-on: https://go-review.googlesource.com/c/go/+/410122
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Either due to a new nowritebarrierrec annotation or a change in escape
analysis, printDebuglog can't be called from sighandler anymore.
Fix this by avoiding a string allocation that's the primary culprit.
Change-Id: Ic84873a453f45852b0443a46597ed3ab8c9443fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/410121
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently the GC CPU limiter consumes CPU time from a few pools, but
because the events that flush to those pools may overlap, rather than be
strictly contained within, the update window for the GC CPU limiter, the
limiter's accounting is ultimately sloppy.
This sloppiness complicates accounting for idle time more completely,
and makes reasoning about the transient behavior of the GC CPU limiter
much more difficult.
To remedy this, this CL adds a field to the P struct that tracks the
start time of any in-flight event the limiter might care about, along
with information about the nature of that event. This timestamp is
managed atomically so that the GC CPU limiter can come in and perform a
read of the partial CPU time consumed by a given event. The limiter also
updates the timestamp so that only what's left over is flushed by the
event itself when it completes.
The end result of this change is that, since the GC CPU limiter is aware
of all past completed events, and all in-flight events, it can much more
accurately collect the CPU time of events since the last update. There's
still the possibility for skew, but any leftover time will be captured
in the following update, and the magnitude of this leftover time is
effectively bounded by the update period of the GC CPU limiter, which is
much easier to consider.
One caveat of managing this timestamp-type combo atomically is that they
need to be packed in 64 bits. So, this CL gives up the top 3 bits of the
timestamp and places the type information there. What this means is we
effectively have only a 61-bit resolution timestamp. This is fine when
the top 3 bits are the same between calls to nanotime, but becomes a
problem on boundaries when those 3 bits change. These cases may cause
hiccups in the GC CPU limiter by not accounting for some source of CPU
time correctly, but with 61 bits of resolution this should be extremely
rare. The rate of update is on the order of milliseconds, so at worst
the runtime will be off of any given measurement by only a few
CPU-milliseconds (and this is directly bounded by the rate of update).
We're probably more inaccurate from the fact that we don't measure real
CPU time but only approximate it.
For #52890.
Change-Id: I347f30ac9e2ba6061806c21dfe0193ef2ab3bbe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/410120
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
After CL 381317 there exist values that may have an alignment greater
than the pointer size for that platform. Specifically, atomic.{Ui|I}nt64
may be aligned to 8 bytes on a 32-bit platform. If such a value, or
a container for the value, gets stack-allocated, it's possible that it
won't be aligned correctly, because the maximum alignment we enforce on
stacks is governed by the pointer size. Changing that would be a
significant undertaking, so just escape these values to the heap
instead, where we're sure they'll actually be aligned correctly.
Change is by rsc@, I'm just shepherding it through code review.
For #50860.
Change-Id: I51669561c0a13ecb84f821020e144c58cb528418
Reviewed-on: https://go-review.googlesource.com/c/go/+/410131
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
