| Age | Commit message (Collapse) | Author |
|---|
|
Issue #76540 reports failures in this test from the leaked goroutine
count being too small. The test makes an effort to wait for the
goroutines to leak, but there's no guarantee.
This change instead changes TestGoroutineLeakProfileConcurrency to wait
for the number of leaked goroutines to reach at least the minimum
expected before proceeding. This deflakes this particular issue.
The downside of this change is that a failure to detect leaked
goroutines due to a bug will lead to a timeout instead of an instant
failure, but this change makes an effort to log and report that it was
waiting for the goroutines to leak and timed out for that reason, so at
least the failure is more obvious.
Overall, this is still better than random flakes.
While we're here, I also make some minor stylistic changes and document
the helper functions a little more. I also noticed that checkFrames was
using the wrong *testing.T, so this change fixes that too.
Fixes #76540.
Change-Id: I0508855dc39b91f8c6b72d059ce88dbfc68fe748
Reviewed-on: https://go-review.googlesource.com/c/go/+/729280
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Copy LabelSet to an internal package as label.Set, and include (escaped)
labels within goroutine stack dumps.
Labels are added to the goroutine header as quoted key:value pairs, so
the line may get long if there are a lot of labels.
To handle escaping, we add a printescaped function to the
runtime and hook it up to the print function in the compiler with a new
runtime.quoted type that's a sibling to runtime.hex. (in fact, we
leverage some of the machinery from printhex to generate escape
sequences).
The escaping can be improved for printable runes outside basic ASCII
(particularly for languages using non-latin stripts). Additionally,
invalid UTF-8 can be improved.
So we can experiment with the output format make this opt-in via a
a new tracebacklabels GODEBUG var.
Updates #23458
Updates #76349
Change-Id: I08e78a40c55839a809236fff593ef2090c13c036
Reviewed-on: https://go-review.googlesource.com/c/go/+/694119
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
|
|
The goroutine leak profile tests currently rely on a function being
inlined, which results in a slightly different representation in the
pprof proto. This function is not inlined on the noopt builder.
Disable inlining of the one function which could be inlined to align but
the regular and noopt builder versions of this test. We're not
interested in testing the inlining functionality of profiles with this
test, we care about certain stack frames appearing in a certain order.
This also simplifies a bunch of the checking in the test.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-noopt
Change-Id: I28902cc4c9fae32d1e3fa41b93b00c3be4d6074a
Reviewed-on: https://go-review.googlesource.com/c/go/+/720100
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Cleaned up goroutine leak profile extraction:
- removed the acquisition of goroutineProfile semaphore
- inlined the call to saveg when recording stacks instead of using
doRecordGoroutineProfile, which had side-effects over
goroutineProfile fields.
Added regression tests for goroutine leak profiling frontend for binary
and debug=1 profile formats.
Added stress tests for concurrent goroutine and goroutine leak profile requests.
Change-Id: I55c1bcef11e9a7fb7699b4c5a2353e594d3e7173
GitHub-Last-Rev: 5e9eb3b1d80c4d2d9b668a01f6b39a7b42f7bb45
GitHub-Pull-Request: golang/go#76045
Reviewed-on: https://go-review.googlesource.com/c/go/+/714580
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
|
|
This change mechanically replaces all occurrences of interface{}
by 'any' (where deemed safe by the 'any' modernizer) throughout
std and cmd, minus their vendor trees.
Since this fix is relatively numerous, it gets its own CL.
Also, 'go generate go/types'.
Change-Id: I14a6b52856c3291c1d27935409bca8d5fd4242a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/719702
Commit-Queue: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
|
|
I think the original depth-1 argument to allocDeep was correct.
Reverted that, and also the change to maxSkip in mprof.go, which was
also incorrect. I think before we were usually passing accidentally in
the loop over matched stacks when we really should usually have been
passing in the previous loop.
Change-Id: I6a6a696463e2baf045b66f418d7afbfcb49258e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/712100
Reviewed-by: Michael Matloob <matloob@google.com>
TryBot-Bypass: Michael Matloob <matloob@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
The test has been causing a lot of flakes on the builders. Skip it while
I'm debugging it.
For #74029
Change-Id: I6a6a696450c23f65bc310a2d0ab61b22dba88f00
Reviewed-on: https://go-review.googlesource.com/c/go/+/712060
TryBot-Bypass: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
This change creates calls to size-specialized malloc functions instead
of calls to newObject when we know the size of the allocation at
compilation time. Most of it is a matter of calling the newObject
function (which will create calls to the size-specialized functions)
rather then the newObjectNonSpecialized function (which won't). In the
newHeapaddr, small, non-pointer case, we'll create a non specialized
newObject and transform that into the appropriate size-specialized
function when we produce the mallocgc in flushPendingHeapAllocations.
We have to update some of the rewrites in generic.rules to also apply to
the size-specialized functions when they apply to newObject.
The messiest thing is we have to adjust the offset we use to save the
memory profiler stack, because the depth of the call to profilealloc is
two frames fewer in the size-specialized malloc functions compared to
when newObject calls mallocgc. A bunch of tests have been adjusted to
account for that.
Change-Id: I6a6a6964c9037fb6719e392c4a498ed700b617d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/707856
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Matloob <matloob@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
This change improves the concrete type analysis in the devirtualizer,
it not longer relies on ir.Reassigned, it now statically tries to
determine the concrete type of an interface, even when assigned
multiple times, following type assertions and iface conversions.
Alternative to CL 649195
Updates #69521
Fixes #64824
Change-Id: Ib1656e19f3619ab2e1e6b2c78346cc320490b2af
GitHub-Last-Rev: e8fa0b12f0a7b1d7ae00e5edb54ce04d1f702c09
GitHub-Pull-Request: golang/go#71935
Reviewed-on: https://go-review.googlesource.com/c/go/+/652036
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
|
|
There are a few places in the code which checks that the running kernel
is greater than or equal to x.y. The check takes a few lines and the
checking code is somewhat distracting.
Let's abstract this check into a simple function, KernelVersionGE,
and convert the users accordingly.
Add a test case (I'm not sure it has much value, can be dropped).
Change-Id: I8ec91dcc7452363361f95e46794701c0ae57d956
Reviewed-on: https://go-review.googlesource.com/c/go/+/700796
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
|
|
Also CL 690655 for golang.org/x/sys.
For #71671
Change-Id: Iceb369dec5affb944a39d07cdabfd7add6f1f319
Reviewed-on: https://go-review.googlesource.com/c/go/+/648795
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Before CL 650697, there was only one system goroutine that could
dynamically change between being a user goroutine and a system
goroutine, and that was the finalizer/cleanup goroutine. In goroutine
profiles, it was handled explicitly. It's status would be checked during
the first STW, and its stack would be recorded. This let the goroutine
profiler completely ignore system goroutines once the world was started
again.
CL 650697 added dedicated cleanup goroutines (there may be more than
one), and with this, the logic for finalizer goroutines no longer
scaled. In that CL, I let the isSystemGoroutine check be dynamic and
dropped the special case, but this was based on incorrect assumptions.
Namely, it's possible for the scheduler to observe, for example, the
finalizer goroutine as a system goroutine and ignore it, but then later
the goroutine profiler itself sees it as a user goroutine. At that point
it's too late and already running. This violates the invariant of the
goroutine profile that all goroutines are handled by the profiler before
they start executing. In practice, the result is that the goroutine
profiler can crash when it checks this invariant (not checking the
invariant means racily reading goroutine stack memory).
The root cause of the problem is that these system goroutines do not
participate in the goroutine profiler's state machine. Normally, when
profiling, goroutines transition from 'absent' to 'in-progress' to
'satisfied'. However with system goroutines, the state machine is
ignored entirely. They always stay in the 'absent' state. This means
that if a goroutine transitions from system to user, it is eligible for
a profile record when it shouldn't be. That transition shouldn't be
allowed to occur with respect to the goroutine profiler, because the
goroutine profiler is trying to snapshot the state of every goroutine.
The fix to this problem is simple: don't ignore system goroutines. Let
them participate in the goroutine profile state machine. Instead, decide
whether or not to record the stack after the goroutine has been acquired
for goroutine profiling. This means if the scheduler observes the
finalizer goroutine as a system goroutine, it will get promoted in the
goroutine profiler's state machine, and no other part of the goroutine
profiler will observe the goroutine again. Simultaneously, the
stack record for the goroutine will be correctly skipped.
Fixes #74090.
Change-Id: Icb9a164a033be22aaa942d19e828e895f700ca74
Reviewed-on: https://go-review.googlesource.com/c/go/+/680477
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This CL fixes a number of (all true positive) findings of vet's
copylock analyzer patched to treat the Bu{ff,uild}er types
as non-copyable after first use.
This does require imposing an additional indirection
between noder.writer and Encoder since the field is
embedded by value but its constructor now returns a pointer.
Updates golang/go#25907
Updates golang/go#47276
Change-Id: I0b4d77ac12bcecadf06a91709e695365da10766c
Reviewed-on: https://go-review.googlesource.com/c/go/+/635339
Reviewed-by: Robert Findley <rfindley@google.com>
Commit-Queue: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
|
|
TestMutexBlockFullAggregation aggregates stacks by function, file, and
line number. But there can be multiple function calls on the same line,
giving us different sequences of PCs. This causes the test to spuriously
fail in some cases. Include PCs in the stacks for this test.
Also pick up a small "range over int" modernize suggestion while we're
looking at the test.
Fixes #73641
Change-Id: I50489e19fcf920e27b9eebd9d4b35feb89981cbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/673115
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This change splits the finalizer and cleanup queues and implements a new
lock-free blocking queue for cleanups. The basic design is as follows:
The cleanup queue is organized in fixed-sized blocks. Individual cleanup
functions are queued, but only whole blocks are dequeued.
Enqueuing cleanups places them in P-local cleanup blocks. These are
flushed to the full list as they get full. Cleanups can only be enqueued
by an active sweeper.
Dequeuing cleanups always dequeues entire blocks from the full list.
Cleanup blocks can be dequeued and executed at any time.
The very last active sweeper in the sweep phase is responsible for
flushing all local cleanup blocks to the full list. It can do this
without any synchronization because the next GC can't start yet, so we
can be very certain that nobody else will be accessing the local blocks.
Cleanup blocks are stored off-heap because the need to be allocated by
the sweeper, which is called from heap allocation paths. As a result,
the GC treats cleanup blocks as roots, just like finalizer blocks.
Flushes to the full list signal to the scheduler that cleanup goroutines
should be awoken. Every time the scheduler goes to wake up a cleanup
goroutine and there were more signals than goroutines to wake, it then
forwards this signal to runtime.AddCleanup, so that it creates another
goroutine the next time it is called, up to gomaxprocs goroutines.
The signals here are a little convoluted, but exist because the sweeper
and the scheduler cannot safely create new goroutines.
For #71772.
For #71825.
Change-Id: Ie839fde2b67e1b79ac1426be0ea29a8d923a62cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/650697
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: Ib3be7cee6ca6dce899805aac176ca789eb2fd0f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/661738
Reviewed-by: Carlos Amedee <carlos@golang.org>
Auto-Submit: Sean Liao <sean@liao.dev>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Users see this frame in tracebacks and it serves as a hint that what is
running here is a finalizer or cleanup. But runfinq is a rather dense
name. We can give it a more obvious name to help users realize what it
is.
For #73011.
Change-Id: I6a6a636ce9a493fd00d4b4c60c23f2b1c96d3568
Reviewed-on: https://go-review.googlesource.com/c/go/+/660296
Auto-Submit: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
CL 658035 added TestCgoCallbackPprof, which is consistently failing on
solaris. runtime/pprof maintains a list of platforms where CPU profiling
does not work properly. Since this test requires CPU profiling, skip the
this test on those platforms.
For #72870.
Fixes #72876.
Change-Id: I6a6a636cbf6b16abcbba8771178fe1d001be9d9b
Reviewed-on: https://go-review.googlesource.com/c/go/+/658415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: I07e7c8eaa5bd4bac0d576b2f2f4cd3f81b0b77a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/630055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
|
|
Switch labelMap from map[string]string to use LabelSet as a data
structure. Optimize Labels() for the case where the keys are given in
sorted order without duplicates.
This is primarily motivated by reducing the overhead of distributed
tracing systems that use pprof labels. We have encountered cases where
users complained about the overhead relative to the rest of our
distributed tracing library code. Additionally, we see this as an
opportunity to free up hundreds of CPU cores across our fleet.
A secondary motivation is eBPF profilers that try to access pprof
labels. The current map[string]string requires them to implement Go map
access in eBPF, which is non-trivial. With the enablement of swiss maps,
this complexity is only increasing. The slice data structure introduced
in this CL will greatly lower the implementation complexity for eBPF
profilers in the future. But to be clear: This change does not imply
that the pprof label mechanism is now a stable ABI. They are still an
implementation detail and may change again in the future.
goos: darwin
goarch: arm64
pkg: runtime/pprof
cpu: Apple M1 Max
│ baseline.txt │ patch1.txt │
│ sec/op │ sec/op vs base │
Labels/set-one-10 153.50n ± 3% 75.00n ± 1% -51.14% (p=0.000 n=10)
Labels/merge-one-10 187.8n ± 1% 128.8n ± 1% -31.42% (p=0.000 n=10)
Labels/overwrite-one-10 193.1n ± 2% 102.0n ± 1% -47.18% (p=0.000 n=10)
Labels/ordered/set-many-10 502.6n ± 4% 146.1n ± 2% -70.94% (p=0.000 n=10)
Labels/ordered/merge-many-10 516.3n ± 2% 238.1n ± 1% -53.89% (p=0.000 n=10)
Labels/ordered/overwrite-many-10 569.3n ± 4% 247.6n ± 2% -56.51% (p=0.000 n=10)
Labels/unordered/set-many-10 488.9n ± 2% 308.3n ± 3% -36.94% (p=0.000 n=10)
Labels/unordered/merge-many-10 523.6n ± 1% 258.5n ± 1% -50.64% (p=0.000 n=10)
Labels/unordered/overwrite-many-10 571.4n ± 1% 412.1n ± 2% -27.89% (p=0.000 n=10)
geomean 366.8n 186.9n -49.05%
│ baseline.txt │ patch1b.txt │
│ B/op │ B/op vs base │
Labels/set-one-10 424.0 ± 0% 104.0 ± 0% -75.47% (p=0.000 n=10)
Labels/merge-one-10 424.0 ± 0% 200.0 ± 0% -52.83% (p=0.000 n=10)
Labels/overwrite-one-10 424.0 ± 0% 136.0 ± 0% -67.92% (p=0.000 n=10)
Labels/ordered/set-many-10 1344.0 ± 0% 392.0 ± 0% -70.83% (p=0.000 n=10)
Labels/ordered/merge-many-10 1184.0 ± 0% 712.0 ± 0% -39.86% (p=0.000 n=10)
Labels/ordered/overwrite-many-10 1056.0 ± 0% 712.0 ± 0% -32.58% (p=0.000 n=10)
Labels/unordered/set-many-10 1344.0 ± 0% 712.0 ± 0% -47.02% (p=0.000 n=10)
Labels/unordered/merge-many-10 1184.0 ± 0% 712.0 ± 0% -39.86% (p=0.000 n=10)
Labels/unordered/overwrite-many-10 1.031Ki ± 0% 1.008Ki ± 0% -2.27% (p=0.000 n=10)
geomean 843.1 405.1 -51.95%
│ baseline.txt │ patch1b.txt │
│ allocs/op │ allocs/op vs base │
Labels/set-one-10 5.000 ± 0% 3.000 ± 0% -40.00% (p=0.000 n=10)
Labels/merge-one-10 5.000 ± 0% 5.000 ± 0% ~ (p=1.000 n=10) ¹
Labels/overwrite-one-10 5.000 ± 0% 4.000 ± 0% -20.00% (p=0.000 n=10)
Labels/ordered/set-many-10 8.000 ± 0% 3.000 ± 0% -62.50% (p=0.000 n=10)
Labels/ordered/merge-many-10 8.000 ± 0% 5.000 ± 0% -37.50% (p=0.000 n=10)
Labels/ordered/overwrite-many-10 7.000 ± 0% 4.000 ± 0% -42.86% (p=0.000 n=10)
Labels/unordered/set-many-10 8.000 ± 0% 4.000 ± 0% -50.00% (p=0.000 n=10)
Labels/unordered/merge-many-10 8.000 ± 0% 5.000 ± 0% -37.50% (p=0.000 n=10)
Labels/unordered/overwrite-many-10 7.000 ± 0% 5.000 ± 0% -28.57% (p=0.000 n=10)
geomean 6.640 4.143 -37.60%
¹ all samples are equal
Change-Id: Ie68e960a25c2d97bcfb6239dc481832fa8a39754
Reviewed-on: https://go-review.googlesource.com/c/go/+/574516
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
The TestProfilerStackDepth/heap test can spuriously fail if the profiler
happens to capture a stack with an allocation several frames deep into
runtime code. The pprof API hides runtime frames at the leaf-end of
stacks, but those frames still count against the profiler's stack depth
limit. The test checks only the first stack it finds with the desired
prefix and fails if it's not deep enough or doesn't have the right root
frame. So it can fail in that scenario, even though the implementation
isn't really broken.
Relax the test to check that there is at least one stack with desired
prefix, depth, and root frame.
Fixes #70112
Change-Id: I337fb3cccd1ddde76530b03aa1ec0f9608aa4112
Reviewed-on: https://go-review.googlesource.com/c/go/+/623998
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Goroutine profiles require checking in with the profiler before any
goroutine starts running. coroswitch is a place where a goroutine may
start running, but where we do not check in with the profiler, which
leads to crashes. Fix this by checking in with the profiler the same way
execute does.
Fixes #69998.
Change-Id: Idef6dd31b70a73dd1c967b56c307c7a46a26ba73
Reviewed-on: https://go-review.googlesource.com/c/go/+/622016
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
This reverts commit 5b0f8596b766afae9dd1f117a4a5dcfbbf1b80f1.
Reason for revert: This CL breaks gotip-linux-amd64-noopt builder.
Change-Id: I3950211f05c90e4955c0785409b796987741a9f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/617715
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
While working on CL 611241 and CL 616375, I introduced a bug that wasn't
caught by any test. CL 611241 added more inline expansion at sample time
for block/mutex profile stacks collected via frame pointer unwinding.
CL 616375 then changed how inline expansion for those stacks is done at
reporting time. So some frames passed through multiple rounds of inline
expansion, and this lead to duplicate stack frames in some cases. The
stacks from TestBlockMutexProfileInlineExpansion looked like
sync.(*Mutex).Unlock
runtime/pprof.inlineF
runtime/pprof.inlineE
runtime/pprof.inlineD
runtime/pprof.inlineD
runtime.goexit
after those two CLs, and in particular after CL 616375. Note the extra
inlineD frame. The test didn't catch that since it was only looking for
a few frames in the stacks rather than checking the entire stacks.
This CL makes that test stricter by checking the entire expected stacks
rather than just a portion of the stacks.
Change-Id: I0acc739d826586e9a63a081bb98ef512d72cdc9a
Reviewed-on: https://go-review.googlesource.com/c/go/+/617235
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Fix a regression introduced in CL 572396 causing goroutine stacks not
getting null terminated.
This bug impacts callers that reuse the []StackRecord slice for multiple
calls to GoroutineProfile. See https://github.com/felixge/fgprof/issues/33
for an example of the problem.
Add a test case to prevent similar regressions in the future. Use null
padding instead of null termination to be consistent with other profile
types and because it's less code to implement. Also fix the
ThreadCreateProfile code path.
Fixes #69243
Change-Id: I0b9414f6c694c304bc03a5682586f619e9bf0588
Reviewed-on: https://go-review.googlesource.com/c/go/+/609815
Reviewed-by: Tim King <taking@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Fix a regression introduced in CL 598515 causing runtime.MutexProfile
stack traces to omit their root frames.
In most cases this was merely causing the `runtime.goexit` frame to go
missing. But in the case of runtime._LostContendedRuntimeLock, an empty
stack trace was being produced.
Add a test that catches this regression by checking for a stack trace
with the `runtime.goexit` frame.
Also fix a separate problem in expandFrame that could cause
out-of-bounds panics when profstackdepth is set to a value below 32.
There is no test for this fix because profstackdepth can't be changed at
runtime right now.
Fixes #69335
Change-Id: I1600fe62548ea84981df0916d25072c3ddf1ea1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/611615
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Nick Ripley <nick.ripley@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Change-Id: I013aae68f47d7a37deb44097f80a213d8c7976bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/612655
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Tobias Klauser <tobias.klauser@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
Mutex contention events with delay of 0 need more than CL 604355 added:
When deciding which event to store in the M's single available slot,
always choose to drop the zero-delay event. Store an explicit flag for
whether we have an event to store, rather than relying on a non-zero
delay.
And, fix a test of sync.Mutex contention that expects those events to
have non-zero delay. The reporting of non-runtime contention like this
has long allowed zero-delay events, which we see when cputicks has low
resolution.
Fixes #68892
Fixes #68906
Change-Id: Id412141e4eb09724f3ce195899a20d59c92d7b78
Reviewed-on: https://go-review.googlesource.com/c/go/+/606115
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
When using frame pointer unwinding, we defer frame skipping and inline
expansion for call stacks until profile reporting time. We can end up
with records which have different stacks if no frames are skipped, but
identical stacks once skipping is taken into account. Returning multiple
records with the same stack (but different values) has broken programs
which rely on the records already being fully aggregated by call stack
when returned from runtime.MutexProfile. This CL addresses the problem
by handling skipping at recording time. We do full inline expansion to
correctly skip the desired number of frames when recording the call
stack, and then handle the rest of inline expansion when reporting the
profile.
The regression test in this CL is adapted from the reproducer in
https://github.com/grafana/pyroscope-go/issues/103, authored by Tolya
Korniltsev.
Fixes #67548
This reapplies CL 595966.
The original version of this CL introduced a bounds error in
MutexProfile and failed to correctly expand inlined frames from that
call. This CL applies the original CL, fixing the bounds error and
adding a test for the output of MutexProfile to ensure the frames are
expanded properly.
Change-Id: I5ef8aafb9f88152a704034065c0742ba767c4dbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/598515
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
This reverts CL 595966.
Reason for revert: This CL contains a bug. See the comment in https://go-review.googlesource.com/c/go/+/595966/8#message-57f4c1f9570b5fe912e06f4ae3b52817962533c0
Change-Id: I48030907ded173ae20a8965bf1b41a713dd06059
Reviewed-on: https://go-review.googlesource.com/c/go/+/598219
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
|
|
When using frame pointer unwinding, we defer frame skipping and inline
expansion for call stacks until profile reporting time. We can end up
with records which have different stacks if no frames are skipped, but
identical stacks once skipping is taken into account. Returning multiple
records with the same stack (but different values) has broken programs
which rely on the records already being fully aggregated by call stack
when returned from runtime.MutexProfile. This CL addresses the problem
by handling skipping at recording time. We do full inline expansion to
correctly skip the desired number of frames when recording the call
stack, and then handle the rest of inline expansion when reporting the
profile.
The regression test in this CL is adapted from the reproducer in
https://github.com/grafana/pyroscope-go/issues/103, authored by Tolya
Korniltsev.
Fixes #67548
Co-Authored-By: Tolya Korniltsev <korniltsev.anatoly@gmail.com>
Change-Id: I6a42ce612377f235b2c8c0cec9ba8e9331224b84
Reviewed-on: https://go-review.googlesource.com/c/go/+/595966
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Carlos Amedee <carlos@golang.org>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
|
|
In all cases the intent was not to interpret s as a format string.
In one case (go/types), this was a latent bug in production.
(These were uncovered by a new check in vet's printf analyzer.)
Updates #60529
Change-Id: I3e17af7e589be9aec1580783a1b1011c52ec494b
Reviewed-on: https://go-review.googlesource.com/c/go/+/587855
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Russ Cox <rsc@golang.org>
|
|
Tests of the mutex profile focus on sync.Mutex, which is easy to
control. But since those tests still use the runtime, and contention on
internal runtime.mutex values is now also part of the mutex profile, we
have to filter out those samples before examining the profile. Otherwise
the test may be confused by stray contention on sched.lock (or other
runtime-internal locks) as a natural consequence of using goroutines.
Fixes #67563
Change-Id: I066a24674d8b719dbeca4a5c0f76b53bc07498c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/586957
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
The current stack depth limit for alloc, mutex, block, threadcreate and
goroutine profiles of 32 frequently leads to truncated stack traces in
production applications. Increase the limit to 128 which is the same
size used by the execution tracer.
Create internal/profilerecord to define variants of the runtime's
StackRecord, MemProfileRecord and BlockProfileRecord types that can hold
arbitrarily big stack traces. Implement internal profiling APIs based on
these new types and use them for creating protobuf profiles and to act
as shims for the public profiling APIs using the old types.
This will lead to an increase in memory usage for applications that
use the impacted profile types and have stack traces exceeding the
current limit of 32. Those applications will also experience a slight
increase in CPU usage, but this will hopefully soon be mitigated via CL
540476 and 533258 which introduce frame pointer unwinding for the
relevant profile types.
For #43669.
Change-Id: Ie53762e65d0f6295f5d4c7d3c87172d5a052164e
Reviewed-on: https://go-review.googlesource.com/c/go/+/572396
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
Previously it was possible for mutex and block profile stack traces to
contain up to 32 frames in Stack0 or the resulting pprof profiles.
CL 533258 changed this behavior by using some of the space to
record skipped frames that are discarded when performing delayed inline
expansion. This has lowered the effective maximum stack size from 32 to
27 (the max skip value is 5), which can be seen as a small regression.
Add TestProfilerStackDepth to demonstrate the issue and protect all
profile types from similar regressions in the future. Fix the issue by
increasing the internal maxStack limit to take the maxSkip value into
account. Assert that the maxSkip value is never exceeded when recording
mutex and block profile stack traces.
Three alternative solutions to the problem were considered and
discarded:
1) Revert CL 533258 and give up on frame pointer unwinding. This seems
unappealing as we would lose the performance benefits of frame
pointer unwinding.
2) Discard skipped frames when recording the initial stack trace. This
would require eager inline expansion for up to maxSkip frames and
partially negate the performance benefits of frame pointer
unwinding.
3) Accept and document the new behavior. This would simplify the
implementation, but seems more confusing from a user perspective. It
also complicates the creation of test cases that make assertions
about the maximum profiling stack depth.
The execution tracer still has the same issue due to CL 463835. This
should be addressed in a follow-up CL.
Co-authored-by: Nick Ripley <nick.ripley@datadoghq.com>
Change-Id: Ibf4dbf08a5166c9cb32470068c69f58bc5f98d2c
Reviewed-on: https://go-review.googlesource.com/c/go/+/586657
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
|
|
A few tests rely on finalizers running, but are doing tiny allocations.
These tests will break if, for example, the testing package does is own
tiny allocations before calling the test function (see CL 478955). The
tiny allocator will group these allocations together and the ones done
for the tests themselves will live longer than desired. Use types which
have/are pointers for these tests so they won't be allocated by the tiny
allocator.
While here, pick up a small refactor suggested by Michael Knyszek to use
the BlockUntilEmptyFinalizerQueue helper to wait for the finalizers to
run in TestFinalizerRegisterABI.
Change-Id: I39f477d61f81dc76c87fae215339f8a38979cf94
Reviewed-on: https://go-review.googlesource.com/c/go/+/529555
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
The goroutine profile has close to three code paths for adding a
goroutine record to the goroutine profile: one for the goroutine that
requested the profile, one for every other goroutine, plus some special
handling for the finalizer goroutine. The first of those captured the
goroutine stack, but neglected to include that goroutine's labels.
Update the tests to check for the inclusion of labels for all three
types of goroutines, and include labels for the creator of the goroutine
profile.
For #63712
Change-Id: Id5387a5f536d3c37268c240e0b6db3d329a3d632
Reviewed-on: https://go-review.googlesource.com/c/go/+/537515
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Rhys Hiltner <rhys@justin.tv>
Reviewed-by: David Chase <drchase@google.com>
|
|
Currently TestMutexProfile expects contention to reported as somewhere
between 0.9x and 2.0x the expected amount introduced. While bounding
from below is fine (especially since the goroutine holding the mutex
doesn't even start to sleep until the required number of goroutines are
blocked on a mutex), bounding from above can easily lead to flakiness.
Delays and non-determinism can come from anywhere in the system,
and nevertheless clocks keep ticking. The result is that goroutines
could easily appear to be blocked on a mutex much longer than just the
sleep time.
However, the contention upper bound is still useful, especially for
identifying wildly incorrect values. Set the contention total to be
proportional to the total wall-time spent in the actual sampling mutex
block sampling portion of the code. This should be a generous
upper-bound on how much contention there could be, because it should in
theory capture any delays from the environment in it as well.
Still, rounding errors could be an issue, and on Windows the time
granularity is quite low (~15ms, or 15% of what each goroutine is
supposed to add to the mutex profile), so getting unlucky with where
time measurements fall within each tick could also be a problem. Add an
extra 10%, which seems to make it much less likely to fail in a Windows
gomote.
Fixes #62094.
Change-Id: I59a10a73affd077185dada8474b91d0bc43b4a43
Reviewed-on: https://go-review.googlesource.com/c/go/+/520635
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
|
|
The pprof mutex profile was meant to match the Google C++ (now Abseil)
mutex profiler, originally designed and implemented by Mike Burrows.
When we worked on the Go version, pjw and I missed that C++ counts the
time each thread is blocked, even if multiple threads are blocked on a
mutex. That is, if 100 threads are blocked on the same mutex for the
same 10ms, that still counts as 1000ms of contention in C++. In Go, to
date, /debug/pprof/mutex has counted that as only 10ms of contention.
If 100 goroutines are blocked on one mutex and only 1 goroutine is
blocked on another mutex, we probably do want to see the first mutex
as being more contended, so the Abseil approach is the more useful one.
This CL adopts "contention scales with number of goroutines blocked",
to better match Abseil [1]. However, it still makes sure to attribute the
time to the unlock that caused the backup, not subsequent innocent
unlocks that were affected by the congestion. In this way it still gives
more accurate profiles than Abseil does.
[1] https://github.com/abseil/abseil-cpp/blob/lts_2023_01_25/absl/synchronization/mutex.cc#L2390
Fixes #61015.
Change-Id: I7eb9e706867ffa8c0abb5b26a1b448f6eba49331
Reviewed-on: https://go-review.googlesource.com/c/go/+/506415
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
If the test is about to time out, testenv.Command sends SIGQUIT to the
child process. The runtime's SIGQUIT goroutine dump should help us to
determine whether the hangs observed in TestCPUProfileWithFork are a
symptom of #60108 or a separate bug.
For #59995.
Updates #60108.
Change-Id: I26342ca262b2b0772795c8be142cfcad8d90db30
Reviewed-on: https://go-review.googlesource.com/c/go/+/507356
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
|
|
Implements OS interactions and memory management.
For #58141
Co-authored-by: Richard Musiol <neelance@gmail.com>
Co-authored-by: Achille Roussel <achille.roussel@gmail.com>
Co-authored-by: Julien Fabre <ju.pryz@gmail.com>
Co-authored-by: Evan Phoenix <evan@phx.io>
Change-Id: I876e7b033090c2fe2d76d2535bb63d52efa36185
Reviewed-on: https://go-review.googlesource.com/c/go/+/479618
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
|
|
The existing runtime_expandFinalInlineFrame implementation doesn't skip trailing wrappers, but
gentraceback does skip wrapper functions.
This change makes runtime_expandFinalInlineFrame handling wrapper functions consistent to gentraceback.
Fixes #58288
Change-Id: I1b0e2c10b0a89bcb1e787b98d27730cb40a34406
Reviewed-on: https://go-review.googlesource.com/c/go/+/465097
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
The current output of TestLabelSystemstack is a bit cryptic. This CL
improves various messages and hopefully simplifies the logic in the
test.
Simplifying the logic leads to three changes in possible outcomes,
which I verified by running the logic before and after this change
through all 2^4 possibilities (https://go.dev/play/p/bnfb-OQCT4j):
1. If a sample both must be labeled and must not be labeled, the test
now reports that explicitly rather than giving other confusing output.
2. If a sample must not be labeled but is, the current logic will
print two identical error messages. The new logic prints only one.
3. If the test finds no frames at all that it recognizes, but the
sample is labeled, it will currently print a confusing "Sample labeled
got true want false" message. The new logic prints nothing. We've seen
this triggered by empty stacks in profiles.
Fixes #51550. This bug was caused by case 3 above, where it was
triggered by a profile label on an empty stack. It's valid for empty
stacks to appear in a profile if we sample a goroutine just as it's
exiting (and that goroutine may have a profile label), so the test
shouldn't fail in this case.
Change-Id: I1593ec4ac33eced5bb89572a3ba7623e56f2fb3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/460516
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This reverts CL 455358, commit 98da0fb43fb481a25b3b4399cd9f517fe94d9f3f.
Reason for revert: still failing https://build.golang.org/log/c9f13a76069f523b5b4a37a75ec52b30a1f3427a
Change-Id: I8246d233c4fb86781b882f19dea82065cc21bc26
Reviewed-on: https://go-review.googlesource.com/c/go/+/455696
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
It is possible that CL 455166 fixes this. Try unskipping the test
and see. If it fails again we can skip it again.
Fixes #48655.
Change-Id: Ia81b06cb7608f74adb276bc018e8fc840285bc11
Reviewed-on: https://go-review.googlesource.com/c/go/+/455358
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Samples in the mutex profile have their count and duration scaled
according to the probability they were sampled. This is done when the
profile is actually requested. The adjustment is done using to the
current configured sampling rate. However, if the sample rate is changed
after a specific sample is recorded, then the sample will be scaled
incorrectly. In particular, if the sampling rate is changed to 0, all of
the samples in the encoded profile will have 0 count and duration. This
means the profile will be "empty", even if it should have had samples.
This CL scales the samples in the profile when they are recorded, rather
than when the profile is requested. This matches what is currently done
for the block profile.
With this change, neither the block profile nor mutex profile are scaled
when they are encoded, so the logic for scaling the samples can be
removed.
Change-Id: If228cf39284385aa8fb9a2d62492d839e02f027f
Reviewed-on: https://go-review.googlesource.com/c/go/+/443056
Auto-Submit: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
This change adds 3 new waitReasons that correspond to sync.Mutex.Lock,
sync.RWMutex.RLock, and sync.RWMutex.Lock that are plumbed down into
semacquire1 by exporting new functions to the sync package from the
runtime.
Currently these three functions show up as "semacquire" in backtraces
which isn't very clear, though the stack trace itself should reveal
what's really going on. This represents a minor improvement to backtrace
readability, though blocking on an RWMutex.w.Lock will still show up as
blocking on a regular mutex (I suppose technically it is).
This is a step toward helping the runtime identify when a goroutine is
blocked on a mutex of some kind.
For #49881.
Change-Id: Ia409b4d27e117fe4bfdc25fa541e9c58d6d587b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/427616
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
|
|
Change-Id: I0407d96e2ba1376cc33fe91b52b6a8d7e81f59ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/428277
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Currently, there are 3 functions returning Linux kernel version numbers.
Two of them are identical:
- in net, initially added by commit 0a9dd47dd817904e;
- in internal/poll, initially added by commit 1c7650aa93bd53;
(both were later fixed by commit 66c02645062561a).
The third one is a more complex, regexp-based implementation in
runtime/pprof, which is only used for a test.
Instead of adding one more, let's consolidate existing ones.
Remove the complex implementation, and move the simple one into
internal/syscall/unix. Use it from all the three places mentioned above.
Change-Id: I4a34d9ca47257743c16def30e4dd634e36056091
Reviewed-on: https://go-review.googlesource.com/c/go/+/424896
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
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>
|
