mstart is the entry point for new threads, so it certainly can't
interact with GC enough to have write barriers. We move the one small
piece that is allowed to have write barriers out into its own
function.
Change-Id: Id9c31d6ffac31d0051fab7db15eb428c11cadbad
Reviewed-on: https://go-review.googlesource.com/46035
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Applications that need to manipulate kernel thread state are currently
on thin ice in Go: they can use LockOSThread to prevent other
goroutines from running on the manipulated thread, but Go may clone
this manipulated state into a new thread that's put into the runtime's
thread pool along with other threads.
Fix this by never starting a new thread from a locked thread or a
thread that may have been started by C. Instead, the runtime starts a
"template thread" with a known-good state. If it then needs to start a
new thread but doesn't know that the current thread is in a good
state, it forwards the thread creation to the template thread.
Fixes#20676.
Change-Id: I798137a56e04b7723d55997e9c5c085d1d910643
Reviewed-on: https://go-review.googlesource.com/46033
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
These have never had a use - not even going back to when they were added
in C.
Change-Id: I143b6902b3bacb1fa83c56c9070a8adb9f61a844
Reviewed-on: https://go-review.googlesource.com/69119
Reviewed-by: Dave Cheney <dave@cheney.net>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The //go:nosplit directive was visible in GoDoc because the function
that it preceeded (Gosched) is exported. This change moves the directive
above the documentation, hiding it from the output.
Change-Id: I281fd7573f11d977487809f74c9cc16b2af0dc88
Reviewed-on: https://go-review.googlesource.com/69120
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, there is a single bit for LockOSThread, so two calls to
LockOSThread followed by one call to UnlockOSThread will unlock the
thread. There's evidence (#20458) that this is almost never what
people want or expect and it makes these APIs very hard to use
correctly or reliably.
Change this so LockOSThread/UnlockOSThread can be nested and the
calling goroutine will not be unwired until UnlockOSThread has been
called as many times as LockOSThread has. This should fix the vast
majority of incorrect uses while having no effect on the vast majority
of correct uses.
Fixes#20458.
Change-Id: I1464e5e9a0ea4208fbb83638ee9847f929a2bacb
Reviewed-on: https://go-review.googlesource.com/45752
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
allp now has length gomaxprocs, which means none of allp[i] are nil or
in state _Pdead. This lets replace several different styles of loops
over allp with normal range loops.
for i := 0; i < gomaxprocs; i++ { ... } loops can simply range over
allp. Likewise, range loops over allp[:gomaxprocs] can just range over
allp.
Loops that check for p == nil || p.state == _Pdead don't need to check
this any more.
Loops that check for p == nil don't have to check this *if* dead Ps
don't affect them. I checked that all such loops are, in fact,
unaffected by dead Ps. One loop was potentially affected, which this
fixes by zeroing p.gcAssistTime in procresize.
Updates #15131.
Change-Id: Ifa1c2a86ed59892eca0610360a75bb613bc6dcee
Reviewed-on: https://go-review.googlesource.com/45575
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Now that allp is dynamically allocated, there's no need for a hard cap
on GOMAXPROCS.
Fixes#15131.
Change-Id: I53eee8e228a711a818f7ebce8d9fd915b3865eed
Reviewed-on: https://go-review.googlesource.com/45574
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes it possible to eliminate the hard cap on GOMAXPROCS.
Updates #15131.
Change-Id: I4c422b340791621584c118a6be1b38e8a44f8b70
Reviewed-on: https://go-review.googlesource.com/45573
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Now that getcallerpc is a compiler intrinsic on x86 and non-x86
platforms don't need the argument, we can drop it.
Sadly, this doesn't let us remove any dummy arguments since all of
those cases also use getcallersp, which still takes the argument
pointer, but this is at least an improvement.
Change-Id: I9c34a41cf2c18cba57f59938390bf9491efb22d2
Reviewed-on: https://go-review.googlesource.com/65474
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
For a trivial benchmark with a do-nothing cgo call:
name old time/op new time/op delta
Call-4 64.5ns ± 7% 63.0ns ± 6% -2.25% (p=0.027 n=20+16)
Because Windows uses the cgocall mechanism to make system calls,
and passes arguments in a struct held in the m,
we need to do the lockOSThread/unlockOSThread in that code.
Because deferreturn was getting a nosplit stack overflow error,
change it to avoid calling typedmemmove.
Updates #21827.
Change-Id: I9b1d61434c44faeb29805b46b409c812c9acadc2
Reviewed-on: https://go-review.googlesource.com/64070
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The current generator is a simple LSFR, which showed strong
correlation in higher bits, as manifested by fastrandn().
Change it with xorshift64+, which is slightly more complex,
has a larger state, but has a period of 2^64-1 and is much better
at statistical tests. The version used here is capable of
passing Diehard and even SmallCrush.
Speed is slightly worse but is probably insignificant:
name old time/op new time/op delta
Fastrand-4 0.77ns ±12% 0.91ns ±21% +17.31% (p=0.048 n=5+5)
FastrandHashiter-4 13.6ns ±21% 15.2ns ±17% ~ (p=0.160 n=6+5)
Fastrandn/2-4 2.30ns ± 5% 2.45ns ±15% ~ (p=0.222 n=5+5)
Fastrandn/3-4 2.36ns ± 7% 2.45ns ± 6% ~ (p=0.222 n=5+5)
Fastrandn/4-4 2.33ns ± 8% 2.61ns ±30% ~ (p=0.126 n=6+5)
Fastrandn/5-4 2.33ns ± 5% 2.48ns ± 9% ~ (p=0.052 n=6+5)
Fixes#21806
Change-Id: I013bb37b463fdfc229a7f324df8fe2da8d286f33
Reviewed-on: https://go-review.googlesource.com/62530
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This change has no real effect in itself. This is to prepare for a
followup change that will call lockOSThread during a cgo callback when
there is no p assigned, and therefore when lockOSThread can not use a
write barrier.
Change-Id: Ia122d41acf54191864bcb68f393f2ed3b2f87abc
Reviewed-on: https://go-review.googlesource.com/63630
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Right now we only kind of sort of trace GC STW events. We emit events
around mark termination, but those start well after stopping the world
and end before starting it again, and we don't emit any events for
sweep termination.
Fix this by generalizing EvGCScanStart/EvGCScanDone. These were
already re-purposed to indicate mark termination (despite the names).
This commit renames them to EvGCSTWStart/EvGCSTWDone, adds an argument
to indicate the STW reason, and shuffles the runtime to generate them
right before stopping the world and right after starting the world,
respectively.
These events will make it possible to generate precise minimum mutator
utilization (MMU) graphs and could be useful in detecting
non-preemptible goroutines (e.g., #20792).
Change-Id: If95783f370781d8ef66addd94886028103a7c26f
Reviewed-on: https://go-review.googlesource.com/55411
Reviewed-by: Rick Hudson <rlh@golang.org>
Found with mvdan.cc/unindent. It skipped the cases where parentheses
would need to be added, where comments would have to be moved elsewhere,
or where actions and simple logic would mix.
One of them was of the form "err != nil && err == io.EOF", so the first
part was removed.
Change-Id: Ie504c2b03a2c87d10ecbca1b9270069be1171b91
Reviewed-on: https://go-review.googlesource.com/57690
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 36428 changed the way nanotime works so on Darwin and Windows it
now depends on runtime.startNano, which is computed at runtime.init
time. Unfortunately, the `runtimeInitTime = nanotime()` initialization
happened *before* runtime.init, so on these platforms runtimeInitTime
is set incorrectly. The one (and only) consequence of this is that the
start time printed in gctrace lines is bogus:
gc 1 18446653480.186s 0%: 0.092+0.47+0.038 ms clock, 0.37+0.15/0.81/1.8+0.15 ms cpu, 4->4->1 MB, 5 MB goal, 8 P
To fix this, this commit moves the runtimeInitTime initialization to
shortly after runtime.init, at which point nanotime is safe to use.
This also requires changing the condition in newproc1 that currently
uses runtimeInitTime != 0 simply to detect whether or not the main M
has started. Since runtimeInitTime could genuinely be 0 now, this
introduces a separate flag to newproc1.
Fixes#21554.
Change-Id: Id874a4b912d3fa3d22f58d01b31ffb3548266d3b
Reviewed-on: https://go-review.googlesource.com/58690
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Writing to selectdone on the stack of another goroutine meant a
pretty subtle dance between the select code and the stack copying
code. Instead move the selectdone variable into the g struct.
Change-Id: Id246aaf18077c625adef7ca2d62794afef1bdd1b
Reviewed-on: https://go-review.googlesource.com/53390
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, GC captures the start-the-world time stamp after
startTheWorldWithSema returns. This is problematic for two reasons:
1. It's possible to get preempted between startTheWorldWithSema
starting the world and calling nanotime.
2. startTheWorldWithSema does several clean-up tasks after the world
is up and running that on rare occasions can take upwards of 10ms.
Since the runtime uses the start-the-world time stamp to compute the
STW duration, both of these can significantly inflate the reported STW
duration.
Fix this by having startTheWorldWithSema itself call nanotime once the
world is started.
Change-Id: I114630234fb73c9dabae50a2ef1884661f2459db
Reviewed-on: https://go-review.googlesource.com/55410
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
64bit atomics on mips/mipsle are implemented using spinlocks. If SIGPROF
is received while the program is in the critical section, it will try to
write the sample using the same spinlock, creating a deadloop.
Prevent it by creating a counter of SIGPROFs during atomic64 and
postpone writing the sample(s) until called from elsewhere, with
pc set to _LostSIGPROFDuringAtomic64.
Added a test case, per Cherry's suggestion. Works around #20146.
Change-Id: Icff504180bae4ee83d78b19c0d9d6a80097087f9
Reviewed-on: https://go-review.googlesource.com/42652
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If we are using vfork, and if something (such as TSAN) is intercepting
the sigaction function, then we must call the system call, not the
libc function. Otherwise the intercepted sigaction call in the child
may trash the data structures in the parent.
Change-Id: Id9588bfeaa934f32c920bf829c5839be5cacf243
Reviewed-on: https://go-review.googlesource.com/50251
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Austin Clements <austin@google.com>
Currently, sysmon waits 60 ms during idle before relaxing. This is
primarily to avoid reducing the precision of short-duration timers. Of
course, if there are no short-duration timers, this wastes 60 ms
running the timer at high resolution.
Improve this by instead inspecting the time until the next timer fires
and relaxing the timer resolution immediately if the next timer won't
fire for a while.
Updates #20937.
Change-Id: If4ad0a565b65a9b3e8c4cdc2eff1486968c79f24
Reviewed-on: https://go-review.googlesource.com/47833
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, sysmon relaxes the Windows timer resolution as soon as the
Go process becomes idle. However, if it's going idle because of a
short sleep (< 15.6 ms), this can turn that short sleep into a long
sleep (15.6 ms).
To address this, wait for 60 ms of idleness before relaxing the timer
resolution. It would be better to check the time until the next wakeup
and relax immediately if it makes sense, but there's currently no
interaction between sysmon and the timer subsystem, so adding this
simple delay is a much simpler and safer change for late in the
release cycle.
Fixes#20937.
Change-Id: I817db24c3bdfa06dba04b7bc197cfd554363c379
Reviewed-on: https://go-review.googlesource.com/47832
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently the execLock is a mutex, which has the unfortunate
side-effect of serializing all thread creation. This replaces it with
an rwmutex so threads can be created in parallel, but exec still
blocks thread creation.
Fixes#20738.
Change-Id: Ia8f30a92053c3d28af460b0da71176abe5fd074b
Reviewed-on: https://go-review.googlesource.com/47072
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Linux's execve has (at the time of writing, and since v2.6.30) a bug when it ran
concurrently with clone, in that it would fail to set up some datastructures if
the thread count before and after some steps differed. This is described better
and in more detail by Colin King in Launchpad¹ and kernel² bugs. When a program
written in Go runtime.Exec's a setuid binary, this issue may cause the resulting
process to not have the expected uid. This patch works around the issue by using
a mutex to serialize exec and clone.
1. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1672819
2. https://bugzilla.kernel.org/show_bug.cgi?id=195453Fixes#19546
Change-Id: I126e87d1d9ce3be5ea4ec9c7ffe13f92e087903d
Reviewed-on: https://go-review.googlesource.com/43713
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Block all signals during a fork. In the parent process, after the
fork, restore the signal mask. In the child process, reset all
currently handled signals to the default handler, and then restore the
signal mask.
The effect of this is that the child will be operating using the same
signal regime as the program it is about to exec, as exec resets all
non-ignored signals to the default, and preserves the signal mask.
We do this so that in the case of a signal sent to the process group,
the child process will not try to run a signal handler while in the
precarious state after a fork.
Fixes#18600.
Change-Id: I9f39aaa3884035908d687ee323c975f349d5faaa
Reviewed-on: https://go-review.googlesource.com/45471
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
There are currently two arrays indexed by P ID: allp and pdesc.
Consolidate these by moving the pdesc fields into type p so they can
be indexed off allp along with all other per-P state.
For #15131.
Change-Id: Ib6c4e6e7612281a1171ba4a0d62e52fd59e960b4
Reviewed-on: https://go-review.googlesource.com/45572
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Back in the day, allp was just a pointer to an array. As a result, the
runtime has a few loops of the form:
for i := 0; ; i++ {
p := allp[i]
if p == nil {
break
}
...
}
This is silly now because it requires that allp be one longer than the
maximum possible number of Ps, but now that allp is in Go it has a
length.
Replace these with range loops.
Change-Id: I91ef4bc7bd3c9d4fda2264f4aa1b1d0271d7f578
Reviewed-on: https://go-review.googlesource.com/45571
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently the extra Ms created for cgo callbacks have a corresponding
G that's kept in syscall state with only a call to goexit on its
stack. This leads to confusing output from runtime.NumGoroutines and
in tracebacks:
goroutine 17 [syscall, locked to thread]:
runtime.goexit()
.../src/runtime/asm_amd64.s:2197 +0x1
Fix this by putting this goroutine into state _Gdead when it's not in
use instead of _Gsyscall. To keep the goroutine counts correct, we
also add one to sched.ngsys while the goroutine is in _Gdead. The
effect of this is as if the goroutine simply doesn't exist when it's
not in use.
Fixes#16631.
Fixes#16714.
Change-Id: Ieae08a2febd4b3d00bef5c23fd6ca88fb2bb0087
Reviewed-on: https://go-review.googlesource.com/45030
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Try to avoid a race between the main goroutine exiting and a panic
occurring. Don't try too hard, to avoid hanging.
Updates #3934Fixes#20018
Change-Id: I57a02b6d795d2a61f1cadd137ce097145280ece7
Reviewed-on: https://go-review.googlesource.com/41052
Reviewed-by: Austin Clements <austin@google.com>
Currently Go sets the system-wide timer resolution to 1ms the whole
time it's running. This has negative affects on system performance and
power consumption. Unfortunately, simply reducing the timer resolution
to the default 15ms interferes with several sleeps in the runtime
itself, including sysmon's ability to interrupt goroutines.
This commit takes a hybrid approach: it only reduces the timer
resolution when the Go process is entirely idle. When the process is
idle, nothing needs a high resolution timer. When the process is
non-idle, it's already consuming CPU so it doesn't really matter if
the OS also takes timer interrupts more frequently.
Updates #8687.
Change-Id: I0652564b4a36d61a80e045040094a39c19da3b06
Reviewed-on: https://go-review.googlesource.com/38403
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Currently the GC triggering condition is an awkward combination of the
gcMode (whether or not it's gcBackgroundMode) and a boolean
"forceTrigger" flag.
Replace this with a new gcTrigger type that represents the range of
transition predicates we need. This has several advantages:
1. We can remove the awkward logic that affects the trigger behavior
based on the gcMode. Now gcMode purely controls whether to run a
STW GC or not and the gcTrigger controls whether this is a forced
GC that cannot be consolidated with other GC cycles.
2. We can lift the time-based triggering logic in sysmon to just
another type of GC trigger and move the logic to the trigger test.
3. This sets us up to have a cycle count-based trigger, which we'll
use to make runtime.GC trigger concurrent GC with the desired
consolidation properties.
For #18216.
Change-Id: If9cd49349579a548800f5022ae47b8128004bbfc
Reviewed-on: https://go-review.googlesource.com/37516
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Currently sysmon triggers periodic GC if GC is not currently running
and it's been long enough since the last GC. This misses some
important conditions; for example, whether GC is enabled at all by
GOGC. As a result, if GOGC is off, once we pass the timeout for
periodic GC, sysmon will attempt to trigger a GC every 10ms. This GC
will be a no-op because gcStart will check all of the appropriate
conditions and do nothing, but it still goes through the motions of
waking the forcegc goroutine and printing a gctrace line.
Fix this by making sysmon call gcShouldStart to check *all* of the
appropriate transition conditions before attempting to trigger a
periodic GC.
Fixes#19247.
Change-Id: Icee5521ce175e8419f934723849853d53773af31
Reviewed-on: https://go-review.googlesource.com/37515
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The darwin linker for ARM does not allow PC-relative relocation
of external symbol in text section. Work around it by accessing
it indirectly: putting its address in a global variable (which is
not external), and accessing through that variable.
Fixes#19684.
Change-Id: I41361bbb281b5dbdda0d100ae49d32c69ed85a81
Reviewed-on: https://go-review.googlesource.com/38596
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
GOTRACEBACK=crash works by bouncing a SIGQUIT around the process
sched.mcount times. However, sched.mcount includes the extra Ms
allocated by oneNewExtraM for cgo callbacks. Hence, if there are any
extra Ms that don't have real OS threads, we'll try to send SIGQUIT
more times than there are threads to catch it. Since nothing will
catch these extra signals, we'll fall back to blocking for five
seconds before aborting the process.
Avoid this five second delay by subtracting out the number of extra Ms
when sending SIGQUITs.
Of course, in a cgo binary, it's still possible for the SIGQUIT to go
to a cgo thread and cause some other failure mode. This does not fix
that.
Change-Id: I4fbf3c52dd721812796c4c1dcb2ab4cb7026d965
Reviewed-on: https://go-review.googlesource.com/38182
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There are a few problems from change 35494, discovered during testing
of change 37852.
1. I was confused about the usage of n.key in the sema variant, so we
were looping on the wrong condition. The error was not caught by
the TryBots (presumably due to missing TSAN coverage in the BSD and
darwin builders?).
2. The sysmon goroutine sometimes skips notetsleep entirely, using
direct usleep syscalls instead. In that case, we were not calling
_cgo_yield, leading to missed signals under TSAN.
3. Some notetsleep calls have long finite timeouts. They should be
broken up into smaller chunks with a yield at the end of each
chunk.
updates #18717
Change-Id: I91175af5dea3857deebc686f51a8a40f9d690bcc
Reviewed-on: https://go-review.googlesource.com/37867
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
After benchmarking with a compiler modified to have better
spill location, it became clear that this method of checking
was actually faster on (at least) two different architectures
(ppc64 and amd64) and it also provides more timely interruption
of loops.
This change adds a modified FOR loop node "FORUNTIL" that
checks after executing the loop body instead of before (i.e.,
always at least once). This ensures that a pointer past the
end of a slice or array is not made visible to the garbage
collector.
Without the rescheduling checks inserted, the restructured
loop from this change apparently provides a 1% geomean
improvement on PPC64 running the go1 benchmarks; the
improvement on AMD64 is only 0.12%.
Inserting the rescheduling check exposed some peculiar bug
with the ssa test code for s390x; this was updated based on
initial code actually generated for GOARCH=s390x to use
appropriate OpArg, OpAddr, and OpVarDef.
NaCl is disabled in testing.
Change-Id: Ieafaa9a61d2a583ad00968110ef3e7a441abca50
Reviewed-on: https://go-review.googlesource.com/36206
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Currently almost every function that deals with a *_func has to first
look up the *moduledata for the module containing the function's entry
point. This means we almost always do at least two identical module
lookups whenever we deal with a *_func (one to get the *_func and
another to get something from its module data) and sometimes several
more.
Fix this by making findfunc return a new funcInfo type that embeds
*_func, but also includes the *moduledata, and making all of the
functions that currently take a *_func instead take a funcInfo and use
the already-found *moduledata.
This transformation is trivial for the most part, since the *_func
type is usually inferred. The annoying part is that we can no longer
use nil to indicate failure, so this introduces a funcInfo.valid()
method and replaces nil checks with calls to valid.
Change-Id: I9b8075ef1c31185c1943596d96dec45c7ab5100f
Reviewed-on: https://go-review.googlesource.com/37331
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
This doesn't change the functionality of the current code,
but it sets us up for exporting the profiling labels into the profile.
The old code had a hash table of profile samples maintained
during the signal handler, with evictions going into a log.
The new code just logs every sample directly, leaving the
hash-based deduplication to an ordinary goroutine.
The new code also avoids storing the entire profile in two
forms in memory, an unfortunate regression introduced
when binary profile support was added. After this CL the
entire profile is only stored once in memory. We'd still like
to get back down to storing it zero times (streaming it to
the underlying io.Writer).
Change-Id: I0893a1788267c564aa1af17970d47377b2a43457
Reviewed-on: https://go-review.googlesource.com/36712
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
It's common for some goroutines to loop calling time.Sleep.
Allocate once per goroutine, not every time.
This comes up in runtime/pprof's background reader.
Change-Id: I89d17dc7379dca266d2c9cd3aefc2382f5bdbade
Reviewed-on: https://go-review.googlesource.com/37162
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
This changes the os package to use the runtime poller for file I/O
where possible. When a system call blocks on a pollable descriptor,
the goroutine will be blocked on the poller but the thread will be
released to run other goroutines. When using a non-pollable
descriptor, the os package will continue to use thread-blocking system
calls as before.
For example, on GNU/Linux, the runtime poller uses epoll. epoll does
not support ordinary disk files, so they will continue to use blocking
I/O as before. The poller will be used for pipes.
Since this means that the poller is used for many more programs, this
modifies the runtime to only block waiting for the poller if there is
some goroutine that is waiting on the poller. Otherwise, there is no
point, as the poller will never make any goroutine ready. This
preserves the runtime's current simple deadlock detection.
This seems to crash FreeBSD systems, so it is disabled on FreeBSD.
This is issue 19093.
Using the poller on Windows requires opening the file with
FILE_FLAG_OVERLAPPED. We should only do that if we can remove that
flag if the program calls the Fd method. This is issue 19098.
Update #6817.
Update #7903.
Update #15021.
Update #18507.
Update #19093.
Update #19098.
Change-Id: Ia5197dcefa7c6fbcca97d19a6f8621b2abcbb1fe
Reviewed-on: https://go-review.googlesource.com/36800
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Since we're no longer stealing space for the stack barrier array from
the stack allocation, the stack allocation is simply
g.stack.hi-g.stack.lo.
Updates #17503.
Change-Id: Id9b450ae12c3df9ec59cfc4365481a0a16b7c601
Reviewed-on: https://go-review.googlesource.com/36621
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
Now that we don't rescan stacks, stack barriers are unnecessary. This
removes all of the code and structures supporting them as well as
tests that were specifically for stack barriers.
Updates #17503.
Change-Id: Ia29221730e0f2bbe7beab4fa757f31a032d9690c
Reviewed-on: https://go-review.googlesource.com/36620
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
With the hybrid barrier, rescanning stacks is no longer necessary so
the rescan list is no longer necessary. Remove it.
This leaves the gcrescanstacks GODEBUG variable, since it's useful for
debugging, but changes it to simply walk all of the Gs to rescan
stacks rather than using the rescan list.
We could also remove g.gcscanvalid, which is effectively a distributed
rescan list. However, it's still useful for gcrescanstacks mode and it
adds little complexity, so we'll leave it in.
Fixes#17099.
Updates #17503.
Change-Id: I776d43f0729567335ef1bfd145b75c74de2cc7a9
Reviewed-on: https://go-review.googlesource.com/36619
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
This ensures that SIGPROF is handled correctly when using
runtime/pprof in a c-archive or c-shared library.
Separate profiler handling into pre-process changes and per-thread
changes. Simplify the Windows code slightly accordingly.
Fixes#18220.
Change-Id: I5060f7084c91ef0bbe797848978bdc527c312777
Reviewed-on: https://go-review.googlesource.com/34018
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Fetch both monotonic and wall time together when possible.
Avoids skew and is cheaper.
Also shave a few ns off in conversion in package time.
Compared to current implementation (after monotonic changes):
name old time/op new time/op delta
Now 19.6ns ± 1% 9.7ns ± 1% -50.63% (p=0.000 n=41+49) darwin/amd64
Now 23.5ns ± 4% 10.6ns ± 5% -54.61% (p=0.000 n=30+28) windows/amd64
Now 54.5ns ± 5% 29.8ns ± 9% -45.40% (p=0.000 n=27+29) windows/386
More importantly, compared to Go 1.8:
name old time/op new time/op delta
Now 9.5ns ± 1% 9.7ns ± 1% +1.94% (p=0.000 n=41+49) darwin/amd64
Now 12.9ns ± 5% 10.6ns ± 5% -17.73% (p=0.000 n=30+28) windows/amd64
Now 15.3ns ± 5% 29.8ns ± 9% +94.36% (p=0.000 n=30+29) windows/386
This brings time.Now back in line with Go 1.8 on darwin/amd64 and windows/amd64.
It's not obvious why windows/386 is still noticeably worse than Go 1.8,
but it's better than before this CL. The windows/386 speed is not too
important; the changes just keep the two architectures similar.
Change-Id: If69b94970c8a1a57910a371ee91e0d4e82e46c5d
Reviewed-on: https://go-review.googlesource.com/36428
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change defines runtime/pprof.SetGoroutineLabels and runtime/pprof.Do, which
are used to set profiler labels on goroutines. The change defines functions
in the runtime for setting and getting profile labels, and sets and unsets
profile labels when goroutines are created and deleted. The change also adds
the package runtime/internal/proflabel, which defines the structure the runtime
uses to store profile labels.
Change-Id: I747a4400141f89b6e8160dab6aa94ca9f0d4c94d
Reviewed-on: https://go-review.googlesource.com/34198
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/35010
