Mutex contention measurements work with two clocks: nanotime for use in
runtime/metrics, and cputicks for the runtime/pprof profile. They're
subject to different sampling rates: the runtime/metrics view is always
enabled, but the profile is adjustable and is turned off by default.
They have different levels of overhead: it can take as little as one
instruction to read cputicks while nanotime calls are more elaborate
(although some platforms implement cputicks as a nanotime call). The use
of the timestamps is also different: the profile's view needs to attach
the delay in some Ms' lock2 calls to another M's unlock2 call stack, but
the metric's view is only an int64.
Treat them differently. Don't bother threading the nanotime clock
through to the unlock2 call, measure and report it directly within
lock2. Sample nanotime at a constant gTrackingPeriod.
Don't consult any clocks unless the mutex is actually contended.
Continue liberal use of cputicks for now.
For #66999
Change-Id: I1c2085ea0e695bfa90c30fadedc99ced9eb1f69e
Reviewed-on: https://go-review.googlesource.com/c/go/+/586796
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Run-TryBot: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
IncNonDefault panics if Value was not called. That's too much DoS risk
in crypto/tls, when the call to Value is distant from the call to
IncNonDefault (see #65991). Value is cheap, though, so we can just call
it before each isolated IncNonDefault.
Change-Id: I6dbed345381e60e029b0a5ef2232e846aa089736
Reviewed-on: https://go-review.googlesource.com/c/go/+/586755
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
I initially thought the logic was broken, but writing the test I
realized it was actually very clever (derogative). It was relying on the
outer loop continuing after a supported match without a key share,
allowing a later key share to override it (but not a later supported
match because of the "if selectedGroup != 0 { continue }").
Replaced the clever loop with two hopefully more understandable loops,
and added a test (which was already passing).
We were however not checking that the selected group is in the supported
list if we found it in key shares first. (This was only a MAY.) Fixed.
Fixes#65686
Change-Id: I09ea44f90167ffa36809deb78255ed039a217b6d
Reviewed-on: https://go-review.googlesource.com/c/go/+/586655
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Fixes#67537.
Change-Id: Ic567f7d19d621a17d2a00aba5b9f927001195ea9
GitHub-Last-Rev: 9eec790ab6
GitHub-Pull-Request: golang/go#67539
Reviewed-on: https://go-review.googlesource.com/c/go/+/587015
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Change-Id: Id63ede5d50a8b287bc0b96382f9f3ee6c2e0b834
Reviewed-on: https://go-review.googlesource.com/c/go/+/586856
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This is just looking at a command-line flag. Look directly.
For #67401.
Change-Id: I7a1c3fc2d9cc85e5ffc5731444bf6db87abf6901
Reviewed-on: https://go-review.googlesource.com/c/go/+/585916
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Move code so that basic imports work instead
of //go:linkname for metadata lists.
For #67401.
Change-Id: Id02075570befc45a9426559aad2137ab540928b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/585915
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Centralizing some repetitive code, which would have prevented #45990.
This also fixes the deprecated Certificate.CreateCRL for RSA-PSS, not
that anyone cared, probably.
This has two other minor observable behavior changes: MD2 is now treated
as a completely unknown algorithm (why did we even have that!? removing
lets us treat hash == 0 as always meaning no prehash); and we now do the
signature verification self-check for all signing operations.
Change-Id: I3b34fe0c3b6eb6181d2145b0704834225cd45a27
Reviewed-on: https://go-review.googlesource.com/c/go/+/586015
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Just a cleanup.
Change-Id: Ibeb2c7d447c793086280e612fe5f0f7eeb863f71
Reviewed-on: https://go-review.googlesource.com/c/go/+/582875
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Jorropo <jorropo.pgm@gmail.com>
The AM* atomic access instruction performs a sequence of “read-modify-write”
operations on a memory cell atomically. Specifically, it retrieves the old
value at the specified address in memory and writes it to the general register
rd, performs some simple operations on the old value in memory and the value
in the general register rk, and then write the result of the operation back
to the memory address pointed to by general register rj.
Go asm syntax:
AM{SWAP/ADD/AND/OR/XOR/MAX/MIN}[DB]{W/V} RK, (RJ), RD
AM{MAX/MIN}[DB]{WU/VU} RK, (RJ), RD
Equivalent platform assembler syntax:
am{swap/add/and/or/xor/max/min}[_db].{w/d} rd, rk, rj
am{max/min}[_db].{wu/du} rd, rk, rj
Ref: https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html
Change-Id: I99ea4553ae731675180d63691c19ef334e7e7817
Reviewed-on: https://go-review.googlesource.com/c/go/+/481577
Reviewed-by: Meidan Li <limeidan@loongson.cn>
Reviewed-by: sophie zhao <zhaoxiaolin@loongson.cn>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Qiqi Huang <huangqiqi@loongson.cn>
Reviewed-by: WANG Xuerui <git@xen0n.name>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Fixes regression from Go 1.22.
For #67547.
Change-Id: I012681c7b8b01b02018b313dd3804690bc7aeed1
Reviewed-on: https://go-review.googlesource.com/c/go/+/587158
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fixes regression from Go 1.22.
For #67547.
Change-Id: Idd319b9d2a73c824caa2c821df0e2fcd4f58cb08
Reviewed-on: https://go-review.googlesource.com/c/go/+/587176
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For #67547.
Change-Id: I1b2118a311dce906327ae6e29e582da539c60b2b
Reviewed-on: https://go-review.googlesource.com/c/go/+/587157
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This change adds an Unalias call in applyTypeFunc and arrayPtrDeref.
At the moment this doesn't change anything or fix any bugs because
of the way these two functions are invoked, but that could change
in the future.
Also, manually reviewed all type assertions to Type types.
Excluding assertions to type parameters, no obvious issues
were found except for #67540 for which a separate fix is pending.
There are potential issues with assertions type parameters
which will be addressed in a follow-up CL.
For #67547.
Change-Id: I312268dc5e104f95b68f115f00aec3ec4c82e41f
Reviewed-on: https://go-review.googlesource.com/c/go/+/587156
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Disable three 100-continue tests that aren't exercising the
intended behavior because they don't set ExpectContinueTimeout.
The tests are flaky right now; setting ExpectContinueTimeout
makes them consistently fail.
Set ExpectContinueTimeout and t.Skip the tests for now.
Fixes#67382
For #67555
Change-Id: I459a19a927e14af03881e89c73d20c93cf0da43e
Reviewed-on: https://go-review.googlesource.com/c/go/+/587155
Auto-Submit: Damien Neil <dneil@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>
Modify rangefunc #next protocol to make it more robust
Extra-terrible nests of rangefunc iterators caused the
prior implementation to misbehave non-locally (in outer loops).
Add more rangefunc exit flag tests, parallel and tricky
This tests the assertion that a rangefunc iterator running
in parallel can trigger the race detector if any of the
parallel goroutines attempts an early exit. It also
verifies that if everything else is carefully written,
that it does NOT trigger the race detector if all the
parts run time completion.
Another test tries to rerun a yield function within a loop,
so that any per-line shared checking would be fooled.
Added all the use-of-body/yield-function checking.
These checks handle pathological cases that would cause
rangefunc for loops to behave in surprising ways (compared
to "regular" for loops). For example, a rangefunc iterator
might defer-recover a panic thrown in the syntactic body
of a loop; this notices the fault and panics with an
explanation
Modified closure naming to ID rangefunc bodies
Add a "-range<N>" suffix to the name of any closure generated for
a rangefunc loop body, as provided in Alessandro Arzilli's CL
(which is merged into this one).
Fix return values for panicky range functions
This removes the delayed implementation of "return x" by
ensuring that return values (in rangefunc-return-containing
functions) always have names and translating the "return x"
into "#rv1 = x" where #rv1 is the synthesized name of the
first result.
Updates #61405.
Change-Id: I933299ecce04ceabcf1c0c2de8e610b2ecd1cfd8
Reviewed-on: https://go-review.googlesource.com/c/go/+/584596
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Tim King <taking@google.com>
unsafe.Sizeof() can return a different value than
reflect.TypeOf(x).Size() for a variable of an interface static type.
This change points out the difference in behavior, by emphasizing that
unsafe.Sizeof() only returns the size of the interface value itself,
rather than the size of the value stored in the interface.
Fixes#67465.
Change-Id: Ia6a809debb7970be171b0fc186209e5d161784e7
Reviewed-on: https://go-review.googlesource.com/c/go/+/586275
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Felix Geisendörfer <felix.geisendoerfer@datadoghq.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Commit-Queue: Ian Lance Taylor <iant@golang.org>
Move code to internal/coverage/cfile, making it possible to
access directly from testing/internal/testdeps, so that we can
avoid needing //go:linkname hacks.
For #67401.
Change-Id: I10b23a9970164afd2165e718ef3b2d9e86783883
Reviewed-on: https://go-review.googlesource.com/c/go/+/585820
Auto-Submit: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Re-enable the build_plugin_reproducible script test now that CL 586079
(more linker changes to work around xcode problems on Darwin with
build reproducibility) is in.
Fixes#64947.
Cq-Include-Trybots: luci.golang.try:gotip-darwin-amd64-longtest
Change-Id: Ice5bc5b809fa7fee689b78fcb874049493bc2c5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/585356
TryBot-Bypass: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
The rotate value was not correctly converted from a 64 bit to 32
bit rotate. This caused a miscompile of
golang.org/x/text/unicode/runenames.Names.
Fixes#67526
Change-Id: Ief56fbab27ccc71cd4c01117909bfee7f60a2ea1
Reviewed-on: https://go-review.googlesource.com/c/go/+/586915
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
When building Go binaries using external linking, rewrite the LC_UUID
Macho load command to replace the content placed there by the external
linker, so as to ensure that we get reproducible builds.
Updates #64947.
Change-Id: I263a89d1a067807404febbc801d4dade33bc3288
Reviewed-on: https://go-review.googlesource.com/c/go/+/586079
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Have the test use the same clock (cputicks) as the profiler, and use the
test's own measurements as hard bounds on the magnitude to expect in the
profile.
Compare the depiction of two users of the same lock: one where the
critical section is fast, one where it is slow. Confirm that the profile
shows the slow critical section as a large source of delay (with #66999
fixed), rather than showing the fast critical section as a large
recipient of delay.
For #64253
For #66999
Change-Id: I784c8beedc39de564dc8cee42060a5d5ce55de39
Reviewed-on: https://go-review.googlesource.com/c/go/+/586237
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Go 1.22 promised to remove the setting in a future release once the
semantics of runtime-internal lock contention matched that of
sync.Mutex. That work is done, remove the setting.
For #66999
Change-Id: I3c4894148385adf2756d8754e44d7317305ad758
Reviewed-on: https://go-review.googlesource.com/c/go/+/585639
Reviewed-by: Carlos Amedee <carlos@golang.org>
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>
When an M's use of a lock causes delays in other Ms, capture the stack
of the unlock call that caused the delay. This makes the meaning of the
mutex profile for runtime-internal mutexes match the behavior for
sync.Mutex: the profile points to the end of the critical section that
is responsible for delaying other work.
Fixes#66999
Change-Id: I4abc4a1df00a48765d29c07776481a1cbd539ff8
Reviewed-on: https://go-review.googlesource.com/c/go/+/585638
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
When an M unlocks a contended mutex, it needs to consult a list of the
Ms that had to wait during its critical section. This allows the M to
attribute the appropriate amount of blame to the unlocking call stack.
Mirroring the implementation for users' sync.Mutex contention (via
sudog), we can (in a future commit) use the time that the head and tail
of the wait list started waiting, and the number of waiters, to estimate
the sum of the Ms' delays.
When an M acquires the mutex, it needs to remove itself from the list of
waiters. Since the futex-based lock implementation leaves the OS in
control of the order of M wakeups, we need to be prepared for quickly
(constant time) removing any M from the list.
First, have each M add itself to a singly-linked wait list when it finds
that its lock call will need to sleep. This case is safe against
live-lock, since any delay to one M adding itself to the list would be
due to another M making durable progress.
Second, have the M that holds the lock (either right before releasing,
or right after acquiring) update metadata on the list of waiting Ms to
double-link the list and maintain a tail pointer and waiter count. That
work is amortized-constant: we'll avoid contended locks becoming
proportionally more contended and undergoing performance collapse.
For #66999
Change-Id: If75cdea915afb59ccec47294e0b52c466aac8736
Reviewed-on: https://go-review.googlesource.com/c/go/+/585637
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Prepare the futex-based implementation of lock2 to maintain a list of
waiting Ms. Beyond storing an muintptr in the mutex's key field, we now
must never overwrite that field (even for a moment) without taking its
current value into account.
The semaphore-based implementation of lock2 already has that behavior.
Reuse that structure.
For #66999
Change-Id: I23b6f6bacb276fe33c6aed5c0571161a7e71fe6c
Reviewed-on: https://go-review.googlesource.com/c/go/+/585636
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Move the nextwaitm field into a small struct, in preparation for
additional metadata to track how long Ms need to wait for locks.
For #66999
Change-Id: Ib40e43c15cde22f7e35922641107973d99439ecd
Reviewed-on: https://go-review.googlesource.com/c/go/+/585635
Reviewed-by: Michael Pratt <mpratt@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: Dmitri Shuralyov <dmitshur@google.com>
For #46477.
Change-Id: Ia3558f9d2bf43fdd9e3618bd9f800d268e13b367
Reviewed-on: https://go-review.googlesource.com/c/go/+/586956
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
This is a continuation of CL 570036.
Amend FindProcess to use pidfdFind, and make it return a special
Process with Pid of pidDone (-2) if the process is not found.
Amend Wait and Signal to return ErrProcessDone if pid == pidDone.
The alternative to the above would be to make FindProcess return
ErrProcessDone, but this is unexpected and incompatible API change,
as discussed in #65866 and #51246.
For #62654.
Rework of CL 542699 (which got reverted in CL 566476).
Change-Id: Ifb4cd3ad1433152fd72ee685d0b85d20377f8723
Reviewed-on: https://go-review.googlesource.com/c/go/+/570681
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Kirill Kolyshkin <kolyshkin@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Allow users to decrease the profiling stack depth back to 32 in case
they experience any problems with the new default of 128.
Users may also use this option to increase the depth up to 1024.
Change-Id: Ieaab2513024915a223239278dd97a6e161dde1cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/581917
Reviewed-by: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@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>
CL 577935 enabled hot block alignment on 386
and amd64 architecture.
However, this change broke the plan9/386 build.
This change disables AlignHot on plan9/386.
Updates #67502.
Change-Id: If73b066824c7218a9408c6e8f06aec5908b7a64f
Reviewed-on: https://go-review.googlesource.com/c/go/+/586835
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: David du Colombier <0intro@gmail.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>
This is for the proposal, plus a few bug fixes
that would/will be necessary when this is put into
actual use.
Fixes#66408.
Updates #63131.
Change-Id: I3a66e09d707dd579c59f155e7f53367f41214c30
Reviewed-on: https://go-review.googlesource.com/c/go/+/578355
Reviewed-by: Austin Clements <austin@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: David Chase <drchase@google.com>
Commands run
go get golang.org/x/telemetry@bf80d56
go mod tidy
go mod vendor
Pulls in CL 586195 and CL 586098
Change-Id: I21ba2ddce0e9c355e9f1c06961ba6cba5475375c
Reviewed-on: https://go-review.googlesource.com/c/go/+/586877
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Matloob <matloob@golang.org>
Factor out file base computation into a method.
Change-Id: Ia6de100459b6df2919f2320872890320aa88866d
Reviewed-on: https://go-review.googlesource.com/c/go/+/586156
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
The posBaseMap is used to identify a file's syntax tree node
given a source position. The position is mapped to the file
base which is then used to look up the file node in posBaseMap.
When posBaseMap is initialized, the file position base
is not the file base if there's a line directive before
the package clause. This can happen in cgo-generated files,
for instance due to an import "C" declaration.
If the wrong file position base is used during initialization,
looking up a file given a position will not find the file.
If a version error occurs and the corresponding file is
not found, the old code panicked with a null pointer exception.
Make sure to consistently initialize the posBaseMap by factoring
out the code computing the file base from a given position.
While at it, check for a nil file pointer. This should not happen
anymore, but don't crash if it happens (at the cost of a slightly
less informative error message).
Fixes#67141.
Change-Id: I4a6af88699c32ad01fffce124b06bb7f9e06f43d
Reviewed-on: https://go-review.googlesource.com/c/go/+/586238
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
This is for the changes in CL 577935.
Change-Id: I8f70dd36c89aaede11a37744c0f279439e01eb13
Reviewed-on: https://go-review.googlesource.com/c/go/+/586895
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>
Use stringslite and bytealg to simplify the code and to remove redundent
helper functions.
Change-Id: I665a8313d9b91201b147b255290058f162cf0894
GitHub-Last-Rev: e9e3bebdb4
GitHub-Pull-Request: golang/go#67515
Reviewed-on: https://go-review.googlesource.com/c/go/+/586715
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Refactor compare function to use cmp.Compare for various types.
Remove redundant floatCompare and isNaN functions.
Change-Id: I146a7af9158538499d07daee930aad8cf89b7e8f
GitHub-Last-Rev: d98365c044
GitHub-Pull-Request: golang/go#67454
Reviewed-on: https://go-review.googlesource.com/c/go/+/586239
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Size() is currently not called from the fast path, since the package
handles the buffer sizing for Read and Write internally. This will change
when adding Append() because callers can use Size to avoid allocations when
writing into bytes.Buffer via AvailableBuffer for example.
Add a fast path for simple types and extend the existing struct size cache
to arrays of structs.
Change-Id: I3af16a2b6c9e2dbe6166a2f8c96bcd2e936719e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/584358
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Austin Clements <austin@google.com>
For #58894
Change-Id: Ia30a3a1a9c7b611f55701956c08caa967634cd5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/586138
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Sam Thanawalla <samthanawalla@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Avoid downloading modules into a modcache in the GOROOT directory. That
creates read only files in GOROOT, which is undesirable.
Fixes#67463
Change-Id: I6d10c605be5eba967648d823ee8fc915716b4b5b
Reviewed-on: https://go-review.googlesource.com/c/go/+/586078
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
