Previously, handleState.prefix was nil if and only if the length of
the prefix was zero. Now, prefix is never nil.
Fix the nil check in the code by also checking if the length is non-zero.
Change-Id: I9f69c0029cb1c73fe6c2919c78fee7d4085bfd85
Reviewed-on: https://go-review.googlesource.com/c/go/+/495977
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
It doesn't make sense to call Logger.WithGroup with the empty string.
Make it a no-op by returning the receiver.
This relieves handlers of the burden of detecting that case themselves.
Less importantly, but for consistency, if Logger.With is called with
no args, make it a no-op by returning the receiver.
Along the way, fix obsolete mentions of "the Logger's context" in the
doc.
Change-Id: Ia6caa4f1ca70c1c4b0cab3e222b2fda48be73fef
Reviewed-on: https://go-review.googlesource.com/c/go/+/496175
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Add the slog static analysis pass to `go vet`.
Vendor in golang.org/x/tools@master to pick up the pass.
Tweak a test in slog to avoid triggering the vet check.
Change-Id: I55ceac9a4e6876c8385897784542761ea0af2481
Reviewed-on: https://go-review.googlesource.com/c/go/+/496156
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The existing implementation clears and recreates Javascript
timeouts when Go is called from js, leading to excessive
load on the js scheduler. Instead, we should remove redundant
calls to clearTimeout and refrain from creating new timeouts
if the previous event's timestamp is within 1 millisecond of
our target (the js scheduler's max precision)
Fixes#56100
Change-Id: I42bbed4c2f1fa6579c1f3aa519b6ed8fc003a20c
Reviewed-on: https://go-review.googlesource.com/c/go/+/442995
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Change-Id: Ibcd919afcb7ff4db79036ef427d088097362a574
Reviewed-on: https://go-review.googlesource.com/c/go/+/496695
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
We will soon have PGO specialization. It doesn't make sense for the
debug flag to have inline in the name, so rename it to pgodebug.
pgoinline is now a flag that can be used to disable PGO inlining.
Devirtualization will have a similar debug flag.
For #59959.
Change-Id: I9770ff1f0d132dfa3cd417018a887a1bd5555bba
Reviewed-on: https://go-review.googlesource.com/c/go/+/494716
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Provide an exported version of implements to easily check if a type
implements an interface. This will be use for PGO devirtualization.
Even within the package, other callers can make use of this simpler API
to reduce duplication.
For #59959.
Change-Id: If4eb86f197ca32abc7634561e36498a247b5070f
Reviewed-on: https://go-review.googlesource.com/c/go/+/495915
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
The documentation for Assignop specifies that if the assignment is not
valid, the reason for the failure is returned via a reason string
without failing the build.
A few cases in Assignop1 -> implements -> ifacelookdot directly call
base.Errorf rather than plumbing through the reason string as they
should. Drop these calls. Since error messages are mostly unreachable
here (it only applies to generated code), don't maintain them and allow
them to just fallthrough to the generic "missing method" message.
This is important for PGO specialization, which opportunistically checks
if candidate interface call targets implement the interface. Many of
these will fail, which should not break the build.
For #59959.
Change-Id: I1891ca0ebebc1c1f51a0d0285035bbe8753036bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/494959
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
This CL fixes two problems:
- NewContextStub initialize a context with the wrong FP. That
function should dereference the FP returned by getcallerfp, as it
returns the callers's FP instead of the caller's caller FP.
CL 494857 will rename getcallerfp to getfp to make this fact clearer.
- sehCallers skips the bottom frame when it should.
Fixes#60053
Change-Id: I7d59b0175fc95281fcc7dd565ced9293064df3a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/496140
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
The previous name was wrong due to the mistaken assumption that calling
f->g->getcallerpc and f->g->getcallersp would respectively return the
pc/sp at g. However, they are actually referring to their caller's
caller, i.e. f.
Rename getcallerfp to getfp in order to stay consistent with this
naming convention.
Also see discussion on CL 463835.
For #16638
This is a redo of CL 481617 that became necessary because CL 461738
added another call site for getcallerfp().
Change-Id: If0b536e85a6c26061b65e7b5c2859fc31385d025
Reviewed-on: https://go-review.googlesource.com/c/go/+/494857
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Before this CL, instructions such as SHA1SU0, AESD and AESE are encoded
in case 1 together with FMOV/ADD, and some error checking is missing,
for example:
SHA1SU0 V1.B16, V2.B16, V3.B16 // wrong data arrangement
SHA1SU0 V1.4S, V2.S4, V3.S4 // correct
Both will be accepted by the assembler, but the first one is totally
incorrect.
This CL fixes these potential encoding issues by moving them into
separate cases, adds some error tests, and also fixes a wrong encoding
operand for ASHA1C.
Change-Id: Ic778321a567735d48bc34a1247ee005c4ed9e11f
Reviewed-on: https://go-review.googlesource.com/c/go/+/493195
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Historically net.Conn.File.Fd has returned a descriptor in blocking mode.
That was broken by CL 495079, which changed the behavior for os.OpenFile
and os.NewFile without intending to affect net.Conn.File.Fd.
Use a hidden os entry point to preserve the historical behavior,
to ensure backward compatibility.
Change-Id: I8d14b9296070ddd52bb8940cb88c6a8b2dc28c27
Reviewed-on: https://go-review.googlesource.com/c/go/+/496080
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TestDialCancel assumes that packets sent to the private IP addresses
198.18.0.254 and 2001:2::254 will be routed to /dev/null.
Not all systems are configured that way. We already ignore one
error case in the test; ignore a couple more than have appeared
on the builders. The test is still valid as long as some builders
discard the packets as expected.
Fixes#52579Fixes#57364
Change-Id: Ibe9ed73b8b3b498623f1d18203dadf9207a0467e
Reviewed-on: https://go-review.googlesource.com/c/go/+/496037
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Now that we implement fcntl on all Unix systems, we can
write closeonexec that uses it. This lets us remove a bunch
of assembler code.
Change-Id: If35591df535ccfc67292086a9492f0a8920e3681
Reviewed-on: https://go-review.googlesource.com/c/go/+/496081
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Fixes#50335
Change-Id: I44b9dc6afa8c70b5cc8c79fb3ebddc3f45d3cef8
Reviewed-on: https://go-review.googlesource.com/c/go/+/475695
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Separate the result and the errno value, rather than assuming
that the result can never be negative.
Change-Id: Ib01a70a3d46285aa77e95371cdde74e1504e7c12
Reviewed-on: https://go-review.googlesource.com/c/go/+/496416
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
The documentation is yet to be written (more work in the go
command remains first). This CL implements the toolchain
selection described in
https://go.dev/design/57001-gotoolchain#the-and-lines-in-in-the-work-module
with these changes based on the issue discussion:
1. GOTOOLCHAIN=auto looks for a go1.19.1 binary in $PATH
and if found uses it instead of downloading Go 1.19.1 as a module.
2. GOTOOLCHAIN=path is like GOTOOLCHAIN=auto, with
downloading disabled.
3. GOTOOLCHAIN=auto+version and GOTOOLCHAIN=path+version
set a different minimum version of Go to use during the version
selection. The default is to use the newer of what's on the go line
or the current toolchain. If you are have Go 1.22 installed locally
and want to switch to a minimum of Go 1.25 with go.mod files
allowed to bump even further, you would set GOTOOLCHAIN=auto+go1.25.
The minimum is also important when there is no go.mod involved,
such as when you write a tiny x.go program and run "go run x.go".
That would get Go 1.25 in this example, instead of falling back to
the local Go 1.22.
Change-Id: I286625a24420424c313d1082b9949a463b2fe14a
Reviewed-on: https://go-review.googlesource.com/c/go/+/494436
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
We don't want to permit writing before the start of an OffsetWriter.
One of the goals of OffsetWriter is to restrict where data
can be written.
However, this rule can be violated by WriteAt() method of OffsetWriter
as the following code shows:
f, _ := os.Create("file.txt")
owr := io.NewOffsetWriter(f, 10)
owr.Write([]byte("world"))
owr.WriteAt([]byte("hello"), -10)
Change-Id: I6c7519fea68daefa641f25130cdd9803dc8aae22
GitHub-Last-Rev: a29d890d6f
GitHub-Pull-Request: golang/go#60222
Reviewed-on: https://go-review.googlesource.com/c/go/+/495155
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jabar Asadi <jasadi@d2iq.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Fixes#59452
Change-Id: Ia0b5a03565f663190c480ef9e26309fa85ff192c
Reviewed-on: https://go-review.googlesource.com/c/go/+/496144
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
This change replaces the statically sized pinnerBits with gcBits
based ones, that are copied in each GC cycle if they exist. The
pinnerBits now include a second bit per object, that indicates if a
pinner counter for multi-pins exists, in order to avoid unnecessary
specials iterations.
This is a follow-up to CL 367296.
Change-Id: I82e38cecd535e18c3b3ae54b5cc67d3aeeaafcfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/493275
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The work to add the -json flag to the 'dist test' command also cleaned
how dist tests are tracked and registered. By now, a pair of (import
path, variant) strings is sufficient to uniquely identify every dist
test that exists. Some of the custom dist test names have been improved
along the way. And since the names are already changing a little anyway,
we use this opportunity to make them more uniform and predictable.
The mapping from the old dist test names to the new is as follows:
- "go_test:pkg" → "pkg" (this is the most common case)
- "go_test_bench:pkg" → "pkg:racebench"
- all other custom names are now called "pkg:variant", where variant
is a description of their test configuration and pkg is the import
path of the Go package under test
CL 495016 introduced test variants and used variant names for rewriting
the Package field in JSON events, and now that same name starts to also
be used as the dist test name.
Like previously done in CL 494496, registering a test variant involving
multiple Go packages creates a "pkg:variant" dist test name for each.
In the future we may combine their 'go test' invocation purely as an
optimization.
We can do this with the support of CL 496190 that keeps the coordinator
happy and capable of working with both new and old names.
In the end, all dist tests now have a consistent "pkg[:variant]" name.
For #37486.
For #59990.
Change-Id: I7eb02a42792a9831a2f3eeab583ff635d24269e8
Co-authored-by: Austin Clements <austin@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/496181
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
The moved_goroot test was the last user of the goroot functionality.
Now that it's been deleted, drop this and clean up loose ends.
Change-Id: Ie5e95644022dab76b1c06cf37f7729ee6616311f
Reviewed-on: https://go-review.googlesource.com/c/go/+/496520
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
This test is largely obviated by the goroot_executable and
list_goroot_symlink cmd/go script tests. It's the last user of several
special-case features in cmd/dist and runs only under a fairly
constrained set of conditions (including only running on builders, not
locally). Delete it.
Change-Id: Icc744e3f9f04813bfd0cad2ef3e88e42617ecf5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/496519
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
This adds the loop type to the json/LSP logging, to help with
studies of how many loops of which kind were modified.
Change-Id: I637a630cd275b413259601c0070b963f3c6d2185
Reviewed-on: https://go-review.googlesource.com/c/go/+/496515
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently trace.lock is reentrant in a few cases. AFAICT, this was
necessary a long time ago when the trace reader would goparkunlock, and
might flush a trace buffer while parking the goroutine. Today, that's no
longer true, since that always happens without the trace.lock held.
However, traceReadCPU does still rely on this behavior, since it could
get called either with trace.lock held, or without it held. The silver
lining here is that it doesn't *need* trace.lock to be held, so the
trace reader can just drop the lock to call traceReadCPU (this is
probably also nice for letting other goroutines flush while the trace
reader is reading from the CPU log).
Stress-tested with
$ stress ./trace.test -test.run="TestTraceCPUProfile|TestTraceStress|TestTraceStressStartStop"
...
42m0s: 24520 runs so far, 0 failures
Change-Id: I2016292c17fe7384050fcc0c446f5797c4e46437
Reviewed-on: https://go-review.googlesource.com/c/go/+/496296
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
This change adds traceBlockReason which leaks fewer implementation
details of the tracer to the runtime. Currently, gopark is called with
an explicit trace event, but this leaks details about trace internals
throughout the runtime.
This change will make it easier to change out the trace implementation.
Change-Id: Id633e1704d2c8838c6abd1214d9695537c4ac7db
Reviewed-on: https://go-review.googlesource.com/c/go/+/494185
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
On Windows when connecting to an unavailable port, ConnectEx() will
retry for 2s, even on loopback devices.
This CL uses a call to WSAIoctl to make the ConnectEx() call fail
faster on local connections.
Fixes#23366
Change-Id: Iafeca8ea0053f01116b2504c45d88120f84d05e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/495875
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently, if a test prints an incomplete line and then exits, in JSON
mode, the filter we use to rewrite Package lines will keep the last
incomplete line in an internal buffer and never print it. In theory
this should never happen anyway because the test should only write
JSON to stdout, but we try pretty hard to pass through any non-JSON,
so it seems inconsistent to swallow incomplete lines.
Fix this by adding a testJSONFilter.Flush method and calling it in the
right places. Unfortunately this is a bit tricky because the filter is
constructed pretty far from where we run the exec.Cmd, so we return
the flush function through the various layers in order to route it to
the place where we call Cmd.Run.
Updates #37486.
Change-Id: I38af67e8ad23458598a32fd428779bb0ec21ac3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/496516
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
It's called from exitsyscall, which is nosplit.
Change-Id: I3f5f92e044497a88a72c1870beb2bdd15c4263c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/496517
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>
Retrying the original CL with a small modification. The original CL
did not handle the case of reading an itab out of a dictionary
correctly. When we read an itab out of a dictionary, we must treat
the type inside that itab as maybe being put in an interface.
Original CL: 486895
Revert CL: 490156
Change-Id: Id2dc1699d184cd8c63dac83986a70b60b4e6cbd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/491495
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Currently the trace clock is cputicks() with comments sprinkled in
different places as to which clock to use. Since the execution tracer
redesign will use a different clock, it seems like a good time to clean
that up.
Also, rename the start/end timestamps to be more readable (i.e.
startTime vs. timeStart).
Change-Id: If43533eddd0e5f68885bb75cdbadb38da42e7584
Reviewed-on: https://go-review.googlesource.com/c/go/+/494775
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The new cmp package provides types and functions related to
comparing ordered values.
For #59488
Change-Id: I43f4b2e6036f63b87c2152672d2b6fa18235cbeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/496356
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Eli Bendersky <eliben@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
The implementation of SetDeadline in Plan 9 begins by calculating
d = the offset of the requested deadline from time.Now(). If d > 0,
a timer is set to interrupt future I/O. If d < 0, the channel is
flagged to prevent future I/O and any current I/O is cancelled.
But if d = 0, nothing happens and the deadline isn't set.
The d = 0 case should be handled the same as d < 0.
Fixes#60282Fixes#52896
Change-Id: Id8167db3604db1c129d99376fa78a3da75417d20
Reviewed-on: https://go-review.googlesource.com/c/go/+/496137
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: David du Colombier <0intro@gmail.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Currently STW events are only emitted for GC STWs. There's little reason
why the trace can't contain events for every STW: they're rare so don't
take up much space in the trace, yet being able to see when the world
was stopped is often critical to debugging certain latency issues,
especially when they stem from user-level APIs.
This change adds new "kinds" to the EvGCSTWStart event, renames the
GCSTW events to just "STW," and lets the parser deal with unknown STW
kinds for future backwards compatibility.
But, this change must break trace compatibility, so it bumps the trace
version to Go 1.21.
This change also includes a small cleanup in the trace command, which
previously checked for STW events when deciding whether user tasks
overlapped with a GC. Looking at the source, I don't see a way for STW
events to ever enter the stream that that code looks at, so that
condition has been deleted.
Change-Id: I9a5dc144092c53e92eb6950e9a5504a790ac00cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/494495
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
This was an oversight, which might cause accounted-for idle time to be
lost. Noticed this while working on #60276.
Change-Id: Ic743785d6dc82555e660f2c9b6aaa9dedef56ed8
Reviewed-on: https://go-review.googlesource.com/c/go/+/496117
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
The new Pinner API's implementation imposes some partial-orders that are
safe but previously did not exist between a mspanSpecial, mheapSpecial,
and mheap. Fix that up in the lock ranking.
For #46787.
Change-Id: I51cc8f7f069240caeb44d749bed43515634f4814
Reviewed-on: https://go-review.googlesource.com/c/go/+/496193
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Some C APIs require the use or structures that contain pointers to
buffers (iovec, io_uring, ...). The pointer passing rules would
require that these buffers are allocated in C memory and to process
this data with Go libraries it would need to be copied.
In order to provide a zero-copy way to use these C APIs, this CL
implements a Pinner API that allows to pin Go objects, which
guarantees that the garbage collector does not move these objects
while pinned. This allows to relax the pointer passing rules so that
pinned pointers can be stored in C allocated memory or can be
contained in Go memory that is passed to C functions.
The Pin() method accepts pointers to objects of any type and
unsafe.Pointer. Slices and arrays can be pinned by calling Pin()
with the pointer to the first element. Pinning of maps is not
supported.
If the GC collects unreachable Pinner holding pinned objects it
panics. If Pin() is called with the other non-pointer types it
panics as well.
Performance considerations: This change has no impact on execution
time on existing code, because checks are only done in code paths,
that would panic otherwise. The memory footprint on existing code is
one pointer per memory span.
Fixes: #46787
Signed-off-by: Sven Anderson <sven@anderson.de>
Change-Id: I110031fe789b92277ae45a9455624687bd1c54f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/367296
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Currently if GOGC=off and GOMEMLIMIT is set, then the synchronous
scavenger is likely to work fairly often to maintain the limit, since
the heap goal goes right up to the edge of the memory limit (minus a
fixed 1 MiB of headroom).
If the application's allocation rate is high, and page-level
fragmentation is high, then most allocations will scavenge.
This change mitigates this problem by adding a proportional component
to constant headroom added to the memory-limit-based heap goal. This
means the runtime will have much more headroom before fragmentation
forces memory to be eagerly scavenged.
The proportional headroom in this case is 3%, or ~30 MiB for a 1 GiB
heap. This technically will increase GC frequency in the GOGC=off case
by a tiny amount, but will likely have a positive impact on both
allocation throughput and latency that outweighs this difference.
I wrote a small program to reproduce this issue and confirmed that the
issue is resolved by this patch:
https://github.com/golang/go/issues/57069#issuecomment-1551746565
This value of 3% is chosen as it seems to be a inflection point in this
particular small program. 2% still resulted in quite a bit of eager
scavenging work. I confirmed this results in a GC frequency increase of
about 3%.
This choice is still somewhat arbitrary because the program is
arbitrary, so perhaps worth revisiting in the future. Still, it should
help a good number of programs.
Fixes#57069.
Change-Id: Icb9829db0dfefb4fe42a0cabc5aa8d35970dd7d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/460375
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
runtime.fcntl returns the error value as a negative value, so it needs
to be inverted before being converted to syscall.Errno.
Change-Id: I43cd0b035150424ac59e623b17a9396c7d62c186
Reviewed-on: https://go-review.googlesource.com/c/go/+/495675
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
