| Age | Commit message (Collapse) | Author |
|---|
|
Change-Id: Ibffe46bad7d30df9380ba18d49eeb6782406a1aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/463115
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Extra Ms may lead to the "no consistent ordering of events possible" error when parsing trace file with cgo enabled, since:
1. The gs in the extra Ms may be in `_Gdead` status while starting trace by invoking `runtime.StartTrace`,
2. and these gs will trigger `traceEvGoSysExit` events in `runtime.exitsyscall` when invoking go functions from c,
3. then, the events of those gs are under non-consistent ordering, due to missing the previous events.
Add two events, `traceEvGoCreate` and `traceEvGoInSyscall`, in `runtime.StartTrace`, will make the trace parser happy.
Fixes #29707
Change-Id: I2fd9d1713cda22f0ddb36efe1ab351f88da10881
GitHub-Last-Rev: 7bbfddb81b70041250e3c59ce53bea44f7afd2c3
GitHub-Pull-Request: golang/go#54974
Reviewed-on: https://go-review.googlesource.com/c/go/+/429858
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: xie cui <523516579@qq.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
This lock is acquired under trace.lock, which as of CL 418956
(6c2e327e35b) must be acquired on the system stack, so this lock must
be, too.
Fixes #54553.
Change-Id: I4fb0c0c2dfc3cb94b76673e842ad416305a31238
Reviewed-on: https://go-review.googlesource.com/c/go/+/425097
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Updates #46731
Change-Id: Ic2208c8bb639aa1e390be0d62e2bd799ecf20654
Reviewed-on: https://go-review.googlesource.com/c/go/+/421878
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
This reverts commit ea9c3fd42d94182ce6f87104b68a51ea92f1a571.
Reason for revert: break linux/ricsv64, openbsd/arm, illumos/amd64 builders
Change-Id: I98479a8f63e76eed89a0e8846acf2c73e8441377
Reviewed-on: https://go-review.googlesource.com/c/go/+/423437
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Extra Ms may lead to the "no consistent ordering of events possible" error when parsing trace file with cgo enabled, since:
1. The gs in the extra Ms may be in `_Gdead` status while starting trace by invoking `runtime.StartTrace`,
2. and these gs will trigger `traceEvGoSysExit` events in `runtime.exitsyscall` when invoking go functions from c,
3. then, the events of those gs are under non-consistent ordering, due to missing the previous events.
Add two events, `traceEvGoCreate` and `traceEvGoInSyscall`, in `runtime.StartTrace`, will make the trace parser happy.
Fixes #29707
Change-Id: I7cc4b80822d2c46591304a59c9da2c9fc470f1d0
GitHub-Last-Rev: 445de8eaf3fb54e12795ac31e26650f821c5efbc
GitHub-Pull-Request: golang/go#53284
Reviewed-on: https://go-review.googlesource.com/c/go/+/411034
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Change-Id: I7eb3de35d1f1f0237962735450b37d738966f30c
Reviewed-on: https://go-review.googlesource.com/c/go/+/423254
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
schedt.goidgen and p.goidcache are already uint64, this makes all cases
consistent.
The only oddball here is schedtrace which prints -1 as an equivalent for
N/A or nil. A future CL will make this more explicit.
Change-Id: I489626f3232799f6ca333d0d103b71d9d3aa7494
Reviewed-on: https://go-review.googlesource.com/c/go/+/419440
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Now that we've moved the trace locks to the leaf of the lock graph, we
can safely annotate that any trace event may acquire trace.lock even
if dynamically it turns out a particular event doesn't need to flush
and acquire this lock.
This reveals a new edge where we can trace while holding the mheap
lock, so we add this to the lock graph.
For #53789.
Updates #53979.
Change-Id: I13e2f6cd1b621cca4bed0cc13ef12e64d05c89a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/418720
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Currently, trace.lock can be acquired while on a user G and stack
splits can happen while holding trace.lock. That means every lock used
by the stack allocator must be okay to acquire while holding
trace.lock, including various locks related to span allocation. In
turn, we cannot safely emit trace events while holding any
allocation-related locks because this would cause a cycle in the lock
rank graph.
To fix this, require that trace.lock only be acquired on the system
stack, like mheap.lock. This pushes it into the "bottom half" and
eliminates the lock rank relationship between tracing and stack
allocation, making it safe to emit trace events in many more places.
One subtlety is that the trace code has race annotations and uses
maps, which have race annotations. By default, we can't have race
annotations on the system stack, so we borrow the user race context
for these situations.
We'll update the lock graph itself in the next CL.
For #53979. This CL technically fixes the problem, but the lock rank
checker doesn't know that yet.
Change-Id: I9f5187a9c52a67bee4f7064db124b1ad53e5178f
Reviewed-on: https://go-review.googlesource.com/c/go/+/418956
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
We're about to require that all uses of trace.lock be on the system
stack. That's mostly easy, except that it's involving parking the
trace reader. Fix this by changing that parking protocol so it instead
synchronizes through an atomic.
For #53979.
Change-Id: Icd6db8678dd01094029d7ad1c612029f571b4cbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/418955
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Writing out the trace footer currently manages trace buffers
differently from the rest of trace code. Rearrange it so it looks like
the rest of the code. In particular, we now write the frequency event
out to the trace buffer rather than returning it in a special byte
slice, and (*traceStackTable).dump threads a traceBufPtr like most
other functions that write to the trace buffers.
Change-Id: I3d0e108e56df884e7bd19823310dfbc0e21af9a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/422974
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Following up on the previous CL, this CL removes a unnecessary stack
copy of a large object in a range loop. This drops another 64 KiB from
(*traceStackTable).dump's stack frame so it is now roughly 80 bytes
depending on architecture, which will easily fit on the system stack.
For #53979.
Change-Id: I16f642f6f1982d0ed0a62371bf2e19379e5870eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/422955
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
|
|
Currently, the stack frame of (*traceStackTable).dump is 68KiB. We're
about to move (*traceStackTable).dump to the system stack, where we
often don't have this much room.
5140 bytes of this is an on-stack temporary buffer for constructing
potentially large trace events before copying these out to the actual
trace buffer.
Reduce the stack frame size by writing these events directly to the
trace buffer rather than temporary space. This introduces a couple
complications:
- The trace event starts with a varint encoding the event payload's
length in bytes. These events are large and somewhat complicated, so
it's hard to know the size ahead of time. That's not a problem with
the temporary buffer because we can just construct the event and see
how long it is. In order to support writing directly to the trace
buffer, we reserve enough bytes for a maximum size varint and add
support for populating a reserved space after the fact.
- Emitting a stack event calls traceFrameForPC, which can itself emit
string events. If these were emitted in the middle of the stack
event, it would corrupt the stream. We already allocate a []Frame to
convert the PC slice to frames, and then convert each Frame into a
traceFrame with trace string IDs, so we address this by combining
these two steps into one so that all trace string events are emitted
before we start constructing the stack event.
For #53979.
Change-Id: Ie60704be95199559c426b551f8e119b14e06ddac
Reviewed-on: https://go-review.googlesource.com/c/go/+/422954
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Atomic operations are used even during STW for consistency.
For #53821.
Change-Id: Ibe7afe5cf893b1288ce24fc96b7691b1f81754ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/417775
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: I24d299b345bda1c9d6fa7876d4f03c05b8c1156d
Reviewed-on: https://go-review.googlesource.com/c/go/+/418587
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Like previous CLs, cases where the getg() G is used only to access the M
are replaced with direct uses of mp.
Change-Id: I4740c80d6b4997d051a52afcfa8c087e0317dab3
Reviewed-on: https://go-review.googlesource.com/c/go/+/418579
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
|
|
_g_, _p_, and _m_ are primarily vestiges of the C version of the
runtime, while today we prefer Go-style variable names (generally gp,
pp, and mp).
This change replaces all remaining uses of _p_ with pp. These are all
trivial replacements (i.e., no conflicts). That said, there are several
functions that refer to two different Ps at once. There the naming
convention is generally that pp refers to the local P, and p2 refers to
the other P we are accessing.
Change-Id: I205b801be839216972e7644b1fbeacdbf2612859
Reviewed-on: https://go-review.googlesource.com/c/go/+/306674
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Fixes #52704
Change-Id: Ia2104c62d7ea9d67469144948b2ceb5d9f1313b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/404054
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
|
|
CL 400795, which uses the runtime/internal/atomic package in trace.go,
raced against CL 397014 removing that import. Re-add the import.
Change-Id: If847ec23f9a0fdff91dab07e93d9fb1b2efed85b
Reviewed-on: https://go-review.googlesource.com/c/go/+/403845
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
When the CPU profiler and execution tracer are both active, report the
CPU profile samples in the execution trace data stream.
Include only samples that arrive on the threads known to the runtime,
but include them even when running g0 (such as near the scheduler) or if
there's no P (such as near syscalls).
Render them in "go tool trace" as instantaneous events.
For #16895
Change-Id: I0aa501a7b450c971e510961c0290838729033f7f
Reviewed-on: https://go-review.googlesource.com/c/go/+/400795
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
As it stands, the heap goal and the trigger are set once by
gcController.commit, and then read out of gcController. However with the
coming memory limit we need the GC to be able to respond to changes in
non-heap memory. The simplest way of achieving this is to compute the
heap goal and its associated trigger dynamically.
In order to make this easier to implement, the GC trigger is now based
on the heap goal, as opposed to the status quo of computing both
simultaneously. In many cases we just want the heap goal anyway, not
both, but we definitely need the goal to compute the trigger, because
the trigger's bounds are entirely based on the goal (the initial runway
is not). A consequence of this is that we can't rely on the trigger to
enforce a minimum heap size anymore, and we need to lift that up
directly to the goal. Specifically, we need to lift up any part of the
calculation that *could* put the trigger ahead of the goal. Luckily this
is just the heap minimum and minimum sweep distance. In the first case,
the pacer may behave slightly differently, as the heap minimum is no
longer the minimum trigger, but the actual minimum heap goal. In the
second case it should be the same, as we ensure the additional runway
for sweeping is added to both the goal *and* the trigger, as before, by
computing that in gcControllerState.commit.
There's also another place we update the heap goal: if a GC starts and
we triggered beyond the goal, we always ensure there's some runway.
That calculation uses the current trigger, which violates the rule of
keeping the goal based on the trigger. Notice, however, that using the
precomputed trigger for this isn't even quite correct: due to a bug, or
something else, we might trigger a GC beyond the precomputed trigger.
So this change also adds a "triggered" field to gcControllerState that
tracks the point at which a GC actually triggered. This is independent
of the precomputed trigger, so it's fine for the heap goal calculation
to rely on it. It also turns out, there's more than just that one place
where we really should be using the actual trigger point, so this change
fixes those up too.
Also, because the heap minimum is set by the goal and not the trigger,
the maximum trigger calculation now happens *after* the goal is set, so
the maximum trigger actually does what I originally intended (and what
the comment says): at small heaps, the pacer picks 95% of the runway as
the maximum trigger. Currently, the pacer picks a small trigger based
on a not-yet-rounded-up heap goal, so the trigger gets rounded up to the
goal, and as per the "ensure there's some runway" check, the runway ends
up at always being 64 KiB. That check is supposed to be for exceptional
circumstances, not the status quo. There's a test introduced in the last
CL that needs to be updated to accomodate this slight change in
behavior.
So, this all sounds like a lot that changed, but what we're talking about
here are really, really tight corner cases that arise from situations
outside of our control, like pathologically bad behavior on the part of
an OS or CPU. Even in these corner cases, it's very unlikely that users
will notice any difference at all. What's more important, I think, is
that the pacer behaves more closely to what all the comments describe,
and what the original intent was.
Another note: at first, one might think that computing the heap goal and
trigger dynamically introduces some raciness, but not in this CL: the heap
goal and trigger are completely static.
Allocation outside of a GC cycle may now be a bit slower than before, as
the GC trigger check is now significantly more complex. However, note
that this executes basically just as often as gcController.revise, and
that makes up for a vanishingly small part of any CPU profile. The next
CL cleans up the floating point multiplications on this path
nonetheless, just to be safe.
For #48409.
Change-Id: I280f5ad607a86756d33fb8449ad08555cbee93f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/397014
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This change moves several scheduling decisions made by schedule into
findrunnable. The main motivation behind this change is the fact that
stopped Ms can't become dedicated or fractional GC workers. The main
reason for this is that when a stopped M wakes up, it stays in
findrunnable until it finds work, which means it will never consider GC
work. On that note, it'll also never consider becoming the trace reader,
either.
Another way of looking at it is that this change tries to make
findrunnable aware of more sources of work than it was before. With this
change, any M in findrunnable should be capable of becoming a GC worker,
resolving #44313. While we're here, let's also make more sources of
work, such as the trace reader, visible to handoffp, which should really
be checking all sources of work. With that, we also now correctly handle
the case where StopTrace is called from the last live M that is also
locked (#39004). stoplockedm calls handoffp to start a new M and handle
the work it cannot, and once we include the trace reader in that, we
ensure that the trace reader gets scheduled.
This change attempts to preserve the exact same ordering of work
checking to reduce its impact.
One consequence of this change is that upon entering schedule, some
sources of work won't be checked twice (i.e. the local and global
runqs, and timers) as they do now, which in some sense gives them a
lower priority than they had before.
Fixes #39004.
Fixes #44313.
Change-Id: I5d8b7f63839db8d9a3e47cdda604baac1fe615ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/393880
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
With the switch to the register ABI, we now generate wrapper
functions for go statements in many cases. A new goroutine's start
PC now points to the wrapper function. This does not affect
execution, but the runtime tracer uses the start PC and the
function name as the name/label of that goroutine. If the start
function is a named function, using the name of the wrapper loses
that information. Furthur, the tracer's goroutine view groups
goroutines by start PC. For multiple go statements with the same
callee, they are grouped together. With the wrappers, which is
context-dependent as it is a closure, they are no longer grouped.
This CL fixes the problem by providing the underlying unwrapped
PC for tracing. The compiler emits metadata to link the unwrapped
PC to the wrapper function. And the runtime reads that metadata
and record that unwrapped PC for tracing.
(This doesn't work for shared buildmode. Unfortunate.)
TODO: is there a way to test?
Fixes #50622.
Change-Id: Iaa20e1b544111c0255eb0fc04427aab7a5e3b877
Reviewed-on: https://go-review.googlesource.com/c/go/+/384158
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
The riscv64 Hifive Unmatched is the only platform that
failed on testcase TestAnalyzeAnnotations occasionally
after CL 332954 had merged. The failure happens when
ticks per second (freq) is over 1e12 which causing the timestamps
of two events are same.
There are 2 reasons causing big frequency:
1. RDCYCLE is HART based according to the riscv manual which makes
negative ticks delta
2. negative float64 -> uint64 is undefined and "lucky" negative float
is too big to handle for trace
For #46737
Change-Id: I1f3c1ac31aae249969000c719c32aaf5a66d29a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/373034
Trust: Zhuo Meng <mzh@golangcn.org>
Run-TryBot: Zhuo Meng <mzh@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
ticks might be same after tick division, although the real cputicks
is linear growth
Fixes #46737
Change-Id: I1d98866fbf21b426c6c1c96cc9cf802d7f440f18
Reviewed-on: https://go-review.googlesource.com/c/go/+/330849
Trust: Meng Zhuo <mzh@golangcn.org>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Meng Zhuo <mzh@golangcn.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
versions [generated]
[git-generate]
cd src/runtime
gofmt -w -r "sys.Goarch386 -> goarch.Is386" .
gofmt -w -r "sys.GoarchAmd64 -> goarch.IsAmd64" .
gofmt -w -r "sys.GoarchAmd64p32 -> goarch.IsAmd64p32" .
gofmt -w -r "sys.GoarchArm -> goarch.IsArm" .
gofmt -w -r "sys.GoarchArmbe -> goarch.IsArmbe" .
gofmt -w -r "sys.GoarchArm64 -> goarch.IsArm64" .
gofmt -w -r "sys.GoarchArm64be -> goarch.IsArm64be" .
gofmt -w -r "sys.GoarchPpc64 -> goarch.IsPpc64" .
gofmt -w -r "sys.GoarchPpc64le -> goarch.IsPpc64le" .
gofmt -w -r "sys.GoarchMips -> goarch.IsMips" .
gofmt -w -r "sys.GoarchMipsle -> goarch.IsMipsle" .
gofmt -w -r "sys.GoarchMips64 -> goarch.IsMips64" .
gofmt -w -r "sys.GoarchMips64le -> goarch.IsMips64le" .
gofmt -w -r "sys.GoarchMips64p32 -> goarch.IsMips64p32" .
gofmt -w -r "sys.GoarchMips64p32le -> goarch.IsMips64p32le" .
gofmt -w -r "sys.GoarchPpc -> goarch.IsPpc" .
gofmt -w -r "sys.GoarchRiscv -> goarch.IsRiscv" .
gofmt -w -r "sys.GoarchRiscv64 -> goarch.IsRiscv64" .
gofmt -w -r "sys.GoarchS390 -> goarch.IsS390" .
gofmt -w -r "sys.GoarchS390x -> goarch.IsS390x" .
gofmt -w -r "sys.GoarchSparc -> goarch.IsSparc" .
gofmt -w -r "sys.GoarchSparc64 -> goarch.IsSparc64" .
gofmt -w -r "sys.GoarchWasm -> goarch.IsWasm" .
goimports -w *.go
Change-Id: I9d88e1284efabaeb0ee3733cba6286247d078c85
Reviewed-on: https://go-review.googlesource.com/c/go/+/328345
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
[git-generate]
cd src/runtime
goimports -w *.go
Change-Id: I1387af0f2fd1a213dc2f4c122e83a8db0fcb15f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/329189
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
|
|
internal/goarch.PtrSize [generated]
[git-generate]
cd src/runtime/internal/math
gofmt -w -r "sys.PtrSize -> goarch.PtrSize" .
goimports -w *.go
cd ../..
gofmt -w -r "sys.PtrSize -> goarch.PtrSize" .
goimports -w *.go
Change-Id: I43491cdd54d2e06d4d04152b3d213851b7d6d423
Reviewed-on: https://go-review.googlesource.com/c/go/+/328337
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
This change moves next_gc and last_next_gc into gcControllerState under
the names heapGoal and lastHeapGoal respectively. These are
fundamentally GC pacer related values, and so it makes sense for them to
live here.
Partially generated by
rf '
ex . {
memstats.next_gc -> gcController.heapGoal
memstats.last_next_gc -> gcController.lastHeapGoal
}
'
except for updates to comments and gcControllerState methods, where
they're accessed through the receiver, and trace-related renames of
NextGC -> HeapGoal, while we're here.
For #44167.
Change-Id: I1e871ad78a57b01be8d9f71bd662530c84853bed
Reviewed-on: https://go-review.googlesource.com/c/go/+/306603
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
This change moves certain important but internal-only GC statistics from
memstats into gcController. These statistics are mainly used in pacing
the GC, so it makes sense to keep them in the pacer's state.
This CL was mostly generated via
rf '
ex . {
memstats.gc_trigger -> gcController.trigger
memstats.triggerRatio -> gcController.triggerRatio
memstats.heap_marked -> gcController.heapMarked
memstats.heap_live -> gcController.heapLive
memstats.heap_scan -> gcController.heapScan
}
'
except for a few special cases, like updating names in comments and when
these fields are used within gcControllerState methods (at which point
they're accessed through the reciever).
For #44167.
Change-Id: I6bd1602585aeeb80818ded24c07d8e6fec992b93
Reviewed-on: https://go-review.googlesource.com/c/go/+/306598
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Correctly accessing allgs is a bit hairy. Some paths need to lock
allglock, some don't. Those that don't are safest using atomicAllG, but
usage is not consistent.
Rather than doing this ad-hoc, move all access* through forEachG /
forEachGRace, the locking and atomic versions, respectively. This will
make it easier to ensure safe access.
* markroot is the only exception, as it has a far-removed guarantee of
safe access via an atomic load of allglen far before actual use.
Change-Id: Ie1c7a8243e155ae2b4bc3143577380c695680e89
Reviewed-on: https://go-review.googlesource.com/c/go/+/279994
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Background mark workers perform per-P marking work. Currently each
worker is assigned a P at creation time. The worker "attaches" to the P
via p.gcBgMarkWorker, making itself (usually) available to
findRunnableGCWorker for scheduling GC work.
While running gcMarkDone, the worker "detaches" from the P (by clearing
p.gcBgMarkWorker), since it may park for other reasons and should not be
scheduled by findRunnableGCWorker.
Unfortunately, this design is complex and difficult to reason about. We
simplify things by changing the design to eliminate the hard P
attachment. Rather than workers always performing work from the same P,
workers perform work for whichever P they find themselves on. On park,
the workers are placed in a pool of free workers, which each P's
findRunnableGCWorker can use to run a worker for its P.
Now if a worker parks in gcMarkDone, a P may simply use another worker
from the pool to complete its own work.
The P's GC worker mode is used to communicate the mode to run to the
selected worker. It is also used to emit the appropriate worker
EvGoStart tracepoint. This is a slight change, as this G may be
preempted (e.g., in gcMarkDone). When it is rescheduled, the trace
viewer will show it as a normal goroutine again. It is currently a bit
difficult to connect to the original worker tracepoint, as the viewer
does not display the goid for the original worker (though the data is in
the trace file).
Change-Id: Id7bd3a364dc18a4d2b1c99c4dc4810fae1293c1b
Reviewed-on: https://go-review.googlesource.com/c/go/+/262348
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>
|
|
next_gc is mostly updated only during a STW, but may occasionally be
updated by calls to e.g. debug.SetGCPercent. In this case the update is
supposed to be protected by the heap lock, but in reality it's accessed
by gcController.revise which may be called without the heap lock held
(despite its documentation, which will be updated in a later change).
Change the synchronization policy on next_gc so that it's atomically
accessed when the world is not stopped to aid in making revise safe for
concurrent use.
Change-Id: I79657a72f91563f3241aaeda66e8a7757d399529
Reviewed-on: https://go-review.googlesource.com/c/go/+/246962
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Currently sysmon is not stopped when the world is stopped, which is
in general a difficult thing to do. The result of this is that when
tracing starts and the value of trace.enabled changes, it's possible
for sysmon to fail to emit an event when it really should. This leads to
traces which the execution trace parser deems inconsistent.
Fix this by putting all of sysmon's work behind a new lock sysmonlock.
StartTrace and StopTrace both acquire this lock after stopping the world
but before performing any work in order to ensure sysmon sees the
required state change in tracing. This change is expected to slow down
StartTrace and StopTrace, but will help ensure consistent traces are
generated.
Updates #29707.
Fixes #38794.
Change-Id: I64c58e7c3fd173cd5281ffc208d6db24ff6c0284
Reviewed-on: https://go-review.googlesource.com/c/go/+/234617
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
I took some of the infrastructure from Austin's lock logging CR
https://go-review.googlesource.com/c/go/+/192704 (with deadlock
detection from the logs), and developed a setup to give static lock
ranking for runtime locks.
Static lock ranking establishes a documented total ordering among locks,
and then reports an error if the total order is violated. This can
happen if a deadlock happens (by acquiring a sequence of locks in
different orders), or if just one side of a possible deadlock happens.
Lock ordering deadlocks cannot happen as long as the lock ordering is
followed.
Along the way, I found a deadlock involving the new timer code, which Ian fixed
via https://go-review.googlesource.com/c/go/+/207348, as well as two other
potential deadlocks.
See the constants at the top of runtime/lockrank.go to show the static
lock ranking that I ended up with, along with some comments. This is
great documentation of the current intended lock ordering when acquiring
multiple locks in the runtime.
I also added an array lockPartialOrder[] which shows and enforces the
current partial ordering among locks (which is embedded within the total
ordering). This is more specific about the dependencies among locks.
I don't try to check the ranking within a lock class with multiple locks
that can be acquired at the same time (i.e. check the ranking when
multiple hchan locks are acquired).
Currently, I am doing a lockInit() call to set the lock rank of most
locks. Any lock that is not otherwise initialized is assumed to be a
leaf lock (a very high rank lock), so that eliminates the need to do
anything for a bunch of locks (including all architecture-dependent
locks). For two locks, root.lock and notifyList.lock (only in the
runtime/sema.go file), it is not as easy to do lock initialization, so
instead, I am passing the lock rank with the lock calls.
For Windows compilation, I needed to increase the StackGuard size from
896 to 928 because of the new lock-rank checking functions.
Checking of the static lock ranking is enabled by setting
GOEXPERIMENT=staticlockranking before doing a run.
To make sure that the static lock ranking code has no overhead in memory
or CPU when not enabled by GOEXPERIMENT, I changed 'go build/install' so
that it defines a build tag (with the same name) whenever any experiment
has been baked into the toolchain (by checking Expstring()). This allows
me to avoid increasing the size of the 'mutex' type when static lock
ranking is not enabled.
Fixes #38029
Change-Id: I154217ff307c47051f8dae9c2a03b53081acd83a
Reviewed-on: https://go-review.googlesource.com/c/go/+/207619
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
This change makes it so that worldsema isn't held across the mark phase.
This means that various operations like ReadMemStats may now stop the
world during the mark phase, reducing latency on such operations.
Only three such operations are still no longer allowed to occur during
marking: GOMAXPROCS, StartTrace, and StopTrace.
For the former it's because any change to GOMAXPROCS impacts GC mark
background worker scheduling and the details there are tricky.
For the latter two it's because tracing needs to observe consistent GC
start and GC end events, and if StartTrace or StopTrace may stop the
world during marking, then it's possible for it to see a GC end event
without a start or GC start event without an end, respectively.
To ensure that GOMAXPROCS and StartTrace/StopTrace cannot proceed until
marking is complete, the runtime now holds a new semaphore, gcsema,
across the mark phase just like it used to with worldsema.
This change is being landed once more after being reverted in the Go
1.14 release cycle, since CL 215157 allows it to have a positive
effect on system performance.
For the benchmark BenchmarkReadMemStatsLatency in the runtime, which
measures ReadMemStats latencies while the GC is exercised, the tail of
these latencies reduced dramatically on an 8-core machine:
name old 50%tile-ns new 50%tile-ns delta
ReadMemStatsLatency-8 4.40M ±74% 0.12M ± 2% -97.35% (p=0.008 n=5+5)
name old 90%tile-ns new 90%tile-ns delta
ReadMemStatsLatency-8 102M ± 6% 0M ±14% -99.79% (p=0.008 n=5+5)
name old 99%tile-ns new 99%tile-ns delta
ReadMemStatsLatency-8 147M ±18% 4M ±57% -97.43% (p=0.008 n=5+5)
Fixes #19812.
Change-Id: If66c3c97d171524ae29f0e7af4bd33509d9fd0bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/216557
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
This reverts commit 7b294cdd8df0a9523010f6ffc80c59e64578f34b, CL 182657.
Reason for revert: This change may be causing latency problems
for applications which call ReadMemStats, because it may cause
all goroutines to stop until the GC completes.
https://golang.org/cl/215157 fixes this problem, but it's too
late in the cycle to land that.
Updates #19812.
Change-Id: Iaa26f4dec9b06b9db2a771a44e45f58d0aa8f26d
Reviewed-on: https://go-review.googlesource.com/c/go/+/216358
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Updates #6239
Updates #27707
Change-Id: I65e6471829c9de4677d3ac78ef6cd7aa0a1fc4cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/171884
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
|
|
This change renames the "round" function to the more appropriately named
"alignUp" which rounds an integer up to the next multiple of a power of
two.
This change also adds the alignDown function, which is almost like
alignUp but rounds down to the previous multiple of a power of two.
With these two functions, we also go and replace manual rounding code
with it where we can.
Change-Id: Ie1487366280484dcb2662972b01b4f7135f72fec
Reviewed-on: https://go-review.googlesource.com/c/go/+/190618
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
Part 1: CL 199499 (GOOS nacl)
Part 2: CL 200077 (amd64p32 files, toolchain)
Part 3: stuff that arguably should've been part of Part 2, but I forgot
one of my grep patterns when splitting the original CL up into
two parts.
This one might also have interesting stuff to resurrect for any future
x32 ABI support.
Updates #30439
Change-Id: I2b4143374a253a003666f3c69e776b7e456bdb9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/200318
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
This change makes it so that worldsema isn't held across the mark phase.
This means that various operations like ReadMemStats may now stop the
world during the mark phase, reducing latency on such operations.
Only three such operations are still no longer allowed to occur during
marking: GOMAXPROCS, StartTrace, and StopTrace.
For the former it's because any change to GOMAXPROCS impacts GC mark
background worker scheduling and the details there are tricky.
For the latter two it's because tracing needs to observe consistent GC
start and GC end events, and if StartTrace or StopTrace may stop the
world during marking, then it's possible for it to see a GC end event
without a start or GC start event without an end, respectively.
To ensure that GOMAXPROCS and StartTrace/StopTrace cannot proceed until
marking is complete, the runtime now holds a new semaphore, gcsema,
across the mark phase just like it used to with worldsema.
Fixes #19812.
Change-Id: I15d43ed184f711b3d104e8f267fb86e335f86bf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/182657
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
|
|
Replaces legacy Go syntax for pointer struct member access
with more modern auto-deref alternative.
Found using https://go-critic.github.io/overview#underef-ref
Change-Id: I71a3c424126c4ff5d89f9e4bacb6cc01c6fa2ddf
Reviewed-on: https://go-review.googlesource.com/122895
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Replace `x[:]` where x is a slice with just `x`.
Found using https://go-critic.github.io/overview.html#unslice-ref
Change-Id: Ib0ee16e1d49b2a875b6b92a770049acc33208362
Reviewed-on: https://go-review.googlesource.com/123375
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
This is a re-roll of CL 99078, which was rolled
back because of failures on s390x.
Those failures were apparently due to an old version of gdb.
Change-Id: Icc2c12f449b2934063fd61e272e06237625ed589
Reviewed-on: https://go-review.googlesource.com/111256
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
|
|
"Span" is a commonly used term in many distributed tracing systems
(Dapper, OpenCensus, OpenTracing, ...). They use it to refer to a
period of time, not necessarily tied into execution of underlying
processor, thread, or goroutine, unlike the "Span" of runtime/trace
package.
Since distributed tracing and go runtime execution tracing are
already similar enough to cause confusion, this CL attempts to avoid
using the same word if possible.
"Region" is being used in a certain tracing system to refer to a code
region which is pretty close to what runtime/trace.Span currently
refers to. So, replace that.
https://software.intel.com/en-us/itc-user-and-reference-guide-defining-and-recording-functions-or-regions
This CL also tweaks APIs a bit based on jbd and heschi's comments:
NewContext -> NewTask
and it now returns a Task object that exports End method.
StartSpan -> StartRegion
and it now returns a Region object that exports End method.
Also, changed WithSpan to WithRegion and it now takes func() with no
context. Another thought is to get rid of WithRegion. It is a nice
concept but in practice, it seems problematic (a lot of code churn,
and polluting stack trace). Already, the tracing concept is very low
level, and we hope this API to be used with great care.
Recommended usage will be
defer trace.StartRegion(ctx, "someRegion").End()
Left old APIs untouched in this CL. Once the usage of them are cleaned
up, they will be removed in a separate CL.
Change-Id: I73880635e437f3aad51314331a035dd1459b9f3a
Reviewed-on: https://go-review.googlesource.com/108296
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: JBD <jbd@google.com>
|
|
This reverts commit 4eea887fd477368653f6fcf8ad766030167936e5.
Reason for revert: broke s390x build
Change-Id: Id6c2b6a7319273c4d21f613d4cdd38b00d49f847
Reviewed-on: https://go-review.googlesource.com/100375
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
|
|
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
Change-Id: I7ffbd52fbc2a286931a2218038fda52ed6473cc9
Reviewed-on: https://go-review.googlesource.com/99078
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
This implements the annotation API proposed in golang.org/cl/63274.
traceString is updated to protect the string map with trace.stringsLock
because the assumption that traceString is called by a single goroutine
(either at the beginning of tracing and at the end of tracing when
dumping all the symbols and function names) is no longer true.
traceString is used by the annotation apis (NewContext, StartSpan, Log)
to register frequently appearing strings (task and span names, and log
keys) after this change.
NewContext -> one or two records (EvString, EvUserTaskCreate)
end function -> one record (EvUserTaskEnd)
StartSpan -> one or two records (EvString, EvUserSpan)
span end function -> one or two records (EvString, EvUserSpan)
Log -> one or two records (EvString, EvUserLog)
EvUserLog record is of the typical record format written by traceEvent
except that it is followed by bytes that represents the value string.
In addition to runtime/trace change, this change includes
corresponding changes in internal/trace to parse the new record types.
Future work to improve efficiency:
More efficient unique task id generation instead of atomic. (per-P
counter).
Instead of a centralized trace.stringsLock, consider using per-P
string cache or something more efficient.
R=go1.11
Change-Id: Iec9276c6c51e5be441ccd52dec270f1e3b153970
Reviewed-on: https://go-review.googlesource.com/71690
Reviewed-by: Austin Clements <austin@google.com>
|
|
This CL presents the proposed user annotation API skeleton.
This CL bumps up the trace version to 1.11.
Design doc https://goo.gl/iqJfJ3
Implementation CLs are followed.
The API introduces three basic building blocks. Log, Span, and Task.
Log is for basic logging. When called, the message will be recorded
to the trace along with timestamp, goroutine id, and stack info.
trace.Log(ctx, messageType message)
Span can be thought as an extension of log to record interesting
time interval during a goroutine's execution. A span is local to a
goroutine by definition.
trace.WithSpan(ctx, "doVeryExpensiveOp", func(ctx context) {
/* do something very expensive */
})
Task is higher-level concept that aids tracing of complex operations
that encompass multiple goroutines or are asynchronous.
For example, an RPC request, a HTTP request, a file write, or a
batch job can be traced with a Task.
Note we chose to design the API around context.Context so it allows
easier integration with other tracing tools, often designed around
context.Context as well. Log and WithSpan APIs recognize the task
information embedded in the context and record it in the trace as
well. That allows the Go execution tracer to associate and group
the spans and log messages based on the task information.
In order to create a Task,
ctx, end := trace.NewContext(ctx, "myTask")
defer end()
The Go execution tracer measures the time between the task created
and the task ended for the task latency.
More discussion history in golang.org/cl/59572.
Update #16619
R=go1.11
Change-Id: I59a937048294dafd23a75cf1723c6db461b193cd
Reviewed-on: https://go-review.googlesource.com/63274
Reviewed-by: Austin Clements <austin@google.com>
|
