Previously we stopped the timer and then reset it. With the current
timer implementation that is no longer required.
Change-Id: Ie7aba61ad53ce835f6fcd0b6bce7fe0a15b10e24
Reviewed-on: https://go-review.googlesource.com/c/go/+/227180
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently if a goroutine is preempted while owning a timer in the
timerModifying state, it could self-deadlock. When the goroutine is
preempted and calls into the scheduler, it could call checkTimers. If
checkTimers encounters the timerModifying timer and calls runtimer on
it, then runtimer will spin, waiting for that timer to leave the
timerModifying state, which it never will.
So far we got lucky that for the most part that there were no preemption
points while timerModifying is happening, however CL 221077 seems to
have introduced one, leading to sporadic self-deadlocks.
This change disables preemption explicitly while a goroutines holds a
timer in timerModifying. Since only checkTimers (and thus runtimer) is
called from the scheduler, this is sufficient to prevent
preemption-based self-deadlocks.
Fixes#38070.
Updates #37894.
Change-Id: Idbfac310889c92773023733ff7e2ff87e9896f0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/225497
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reduce the length of time that other timer functions can see timerModifying.
In particular avoid system calls.
Fixes#38023
Change-Id: I1b61229c668e6085d9ee6dca9488a90055386c36
Reviewed-on: https://go-review.googlesource.com/c/go/+/224902
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The cleantimers can run for a while in some unlikely cases.
If the GC is trying to preempt the G, it is forced to wait as the
G is holding timersLock. To avoid introducing a GC delay,
return from cleantimers if the G has a preemption request.
Fixes#37779
Change-Id: Id9a567f991e26668e2292eefc39e2edc56efa4e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/223122
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
If we see a racy use of timers, as in concurrent calls to Timer.Reset,
do the operations in an unpredictable order, rather than crashing.
Fixes#37400
Change-Id: Idbac295df2dfd551b6d762909d5040fc532c1b34
Reviewed-on: https://go-review.googlesource.com/c/go/+/221077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This CL implements Ticker.Reset method in time package.
Benchmark:
name time/op
TickerReset-12 6.41µs ±10%
TickerResetNaive-12 95.7µs ±12%
Fixes#33184
Change-Id: I4cbd31796efa012b2a297bb342158f11a4a31fef
Reviewed-on: https://go-review.googlesource.com/c/go/+/220424
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts CL 217362 (6e5652bebede2d53484a872f6d1dfeb498b0b50c.)
Reason for revert: Causing failures on arm64 bots. See #33184 for more info
Change-Id: I72ba40047e4138767d95aaa68842893c3508c52f
Reviewed-on: https://go-review.googlesource.com/c/go/+/220638
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL implements Ticker.Reset method in time package.
Benchmark:
name time/op
TickerReset-12 6.41µs ±10%
TickerResetNaive-12 95.7µs ±12%
Fixes#33184
Change-Id: I12c651f81e452541bcbbc748b45f038aae1f8dae
Reviewed-on: https://go-review.googlesource.com/c/go/+/217362
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The timers code used to have a problem: if code started and stopped a
lot of timers, as would happen with, for example, lots of calls to
context.WithTimeout, then it would steadily use memory holding timers
that had stopped but not been removed from the timer heap.
That problem was fixed by CL 214299, which would remove all deleted
timers whenever they got to be more than 1/4 of the total number of
timers on the heap.
The timers code had a different problem: if there were some idle P's,
the running P's would have lock contention trying to steal their timers.
That problem was fixed by CL 214185, which only acquired the timer lock
if the next timer was ready to run or there were some timers to adjust.
Unfortunately, CL 214185 partially undid 214299, in that we could now
accumulate an increasing number of deleted timers while there were no
timers ready to run. This CL restores the 214299 behavior, by checking
whether there are lots of deleted timers without acquiring the lock.
This is a performance issue to consider for the 1.14 release.
Change-Id: I13c980efdcc2a46eb84882750c39e3f7c5b2e7c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/215722
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reduces lock contention when only a few P's are running and
checking for whether they need to run timers on the sleeping P's.
Without this change the running P's would get lock contention
while looking at the sleeping P's timers. With this change a single
atomic load suffices to determine whether there are any ready timers.
Change-Id: Ie843782bd56df49867a01ecf19c47498ec827452
Reviewed-on: https://go-review.googlesource.com/c/go/+/214185
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: David Chase <drchase@google.com>
Whenever more than 1/4 of the timers on a P's heap are deleted,
remove them from the heap.
Change-Id: Iff63ed3d04e6f33ffc5c834f77f645c52c007e52
Reviewed-on: https://go-review.googlesource.com/c/go/+/214299
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When adjustTimers sees a timerModifiedEarlier or timerModifiedLater,
it removes it from the heap, leaving a new timer at that position
in the heap. We were accidentally skipping that new timer in our loop.
In some unlikely cases this could cause adjustTimers to look at more
timers than necessary.
Change-Id: Ic71e54c175ab7d86a7fa46f1497aca71ed1c43cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/214338
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We were using the race context of the P that held the timer,
but since we unlock the P's timers while executing a timer
that could lead to a race on the race context itself.
Updates #6239
Updates #27707Fixes#35906
Change-Id: I5f9d5f52d8e28dffb88c3327301071b16ed1a913
Reviewed-on: https://go-review.googlesource.com/c/go/+/209580
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Dan Scales pointed out a theoretical deadlock in the runtime.
The timer code runs timer functions while holding the timers lock for a P.
The scavenger queues up a timer function that calls wakeScavenger,
which acquires the scavenger lock.
The scavengeSleep function acquires the scavenger lock,
then calls resetTimer which can call addInitializedTimer
which acquires the timers lock for the current P.
So there is a potential deadlock, in that the scavenger lock and
the timers lock for some P may both be acquired in different order.
It's not clear to me whether this deadlock can ever actually occur.
Issue 35532 describes another possible deadlock.
The pollSetDeadline function acquires pd.lock for some poll descriptor,
and in some cases calls resettimer which can in some cases acquire
the timers lock for the current P.
The timer code runs timer functions while holding the timers lock for a P.
The timer function for poll descriptors winds up in netpolldeadlineimpl
which acquires pd.lock.
So again there is a potential deadlock, in that the pd lock for some
poll descriptor and the timers lock for some P may both be acquired in
different order. I think this can happen if we change the deadline
for a network connection exactly as the former deadline expires.
Looking at the code, I don't see any reason why we have to hold
the timers lock while running a timer function.
This CL implements that change.
Updates #6239
Updates #27707Fixes#35532
Change-Id: I17792f5a0120e01ea07cf1b2de8434d5c10704dd
Reviewed-on: https://go-review.googlesource.com/c/go/+/207348
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
In the discussion of CL 171828 we decided that it was not necessary to
acquire timersLock around the call to moveTimers, because the world is
stopped. However, that is not correct, as sysmon runs even when the world
is stopped, and it calls timeSleepUntil which looks through the timers.
timeSleepUntil acquires timersLock, but that doesn't help if moveTimers
is running at the same time.
Updates #6239
Updates #27707
Updates #35462
Change-Id: I346c5bde594c4aff9955ae430b37c2b6fc71567f
Reviewed-on: https://go-review.googlesource.com/c/go/+/206938
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Otherwise, we can get into a deadlock: sysmon takes the scheduler lock
and calls timeSleepUntil which takes each P's timer lock. Simultaneously,
some P calls runtimer (holding the P's own timer lock) which wakes up
the scavenger, calling goready, calling wakep, calling startm, getting
the scheduler lock. Now the sysmon thread is holding the scheduler lock
and trying to get a P's timer lock, while some other thread running on
that P is holding the P's timer lock and trying to get the scheduler lock.
So change sysmon to call timeSleepUntil without holding the scheduler
lock, and change timeSleepUntil to use allpLock, which is only held for
limited periods of time and should never compete with timer locks.
This hopefully
Fixes#35375
At least it should fix the linux-arm64-packet builder problems,
which occurred more reliably as that system has GOMAXPROCS == 96,
giving a lot more scope for this deadlock.
Change-Id: I7a7917daf7a4882e0b27ca416e4f6300cfaaa774
Reviewed-on: https://go-review.googlesource.com/c/go/+/205558
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Before this CL adjustTimers left timers being moved in an inconsistent
state: status timerWaiting but not on a P. Simplify the code by
leaving the timers in timerMoving status until they are actually moved.
Other functions (deltimer, modtimer) will wait until the move is complete
before changing anything on the timer. This does leave timers in timerMoving
state for longer, but still not all that long.
Fixes#35367
Change-Id: I31851002fb4053bd6914139125b4c82a68bf6fb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/205418
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When dropping a P, if it has any timers, and if some thread is
sleeping in the netpoller, wake the netpoller to run the P's timers.
This mitigates races between the netpoller deciding how long to sleep
and a new timer being added.
In sysmon, if all P's are idle, check the timers to decide how long to sleep.
This avoids oversleeping if no thread is using the netpoller.
This can happen in particular if some threads use runtime.LockOSThread,
as those threads do not block in the netpoller.
Also, print the number of timers per P for GODEBUG=scheddetail=1.
Before this CL, TestLockedDeadlock2 would fail about 1% of the time.
With this CL, I ran it 150,000 times with no failures.
Updates #6239
Updates #27707Fixes#35274Fixes#35288
Change-Id: I7e5193e6c885e567f0b1ee023664aa3e2902fcd1
Reviewed-on: https://go-review.googlesource.com/c/go/+/204800
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
If multiple goroutines call time.(*Timer).Reset then the timer will go
from timerWaiting to timerDeleted to timerModifying to timerModifiedLater.
The timer can be on a different P, meaning that simultaneously cleantimers
could change it from timerDeleted to timerRemoving to timerRemoved.
If Reset sees timerRemoved, it was doing an atomic.Store of timerWaiting,
meaning that it did not necessarily see the other values set in the timer,
so the timer could appear to be in an inconsistent state. Use atomic.Cas
to avoid that possibility.
Updates #6239
Updates #27707Fixes#35272
Change-Id: I1d59a13dc4f2ff4af110fc6e032c8c9d59cfc270
Reviewed-on: https://go-review.googlesource.com/c/go/+/204717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Since the new timers run on g0, which does not have a race context,
we add a race context field to the P, and use that for timer functions.
This works since all timer functions are in the standard library.
Updates #27707
Change-Id: I8a5b727b4ddc8ca6fc60eb6d6f5e9819245e395b
Reviewed-on: https://go-review.googlesource.com/c/go/+/171882
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Since timers are now on a P, rather than having a G running timerproc,
timejump changes to return a P rather than a G.
Updates #27707
Change-Id: I3d05af2d664409a0fd906e709fdecbbcbe00b9a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/171880
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The adjusttimers function is where we check the adjustTimers field in
the P struct to see if we need to resort the heap. We walk forward in
the heap and find and resort timers that have been modified, until we
find all the timers that were modified to run earlier. Along the way
we remove deleted timers.
Updates #27707
Change-Id: I1cba7fe77b8112b7e9a9dba80b5dfb08fcc7c568
Reviewed-on: https://go-review.googlesource.com/c/go/+/171877
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Also add a skeleton of the runOneTimer function.
Updates #27707
Change-Id: Ic6a0279354a57295f823093704b7e152ce5d769d
Reviewed-on: https://go-review.googlesource.com/c/go/+/171835
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The dodeltimer function removes a timer from a heap. The dodeltimer0
function removes the first timer from a heap; in the old timer code
this common special case was inlined in the timerproc function.
Updates #27707
Change-Id: I1b7c0af46866abb4bffa8aa4d8e7143f9ae8f402
Reviewed-on: https://go-review.googlesource.com/c/go/+/171834
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
This adds a new field to P, adjustTimers, that tells the P that one of
its existing timers was modified to be earlier, and that it therefore
needs to resort them.
Updates #27707
Change-Id: I4c5f5b51ed116f1d898d3f87cdddfa1b552337f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/171832
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
When we add a timer, make sure that the network poller is initialized,
since we will use it if we have to wait for the timer to be ready.
Updates #27707
Change-Id: I0637fe646bade2cc5ce50b745712292aa9c445b1
Reviewed-on: https://go-review.googlesource.com/c/go/+/171830
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Add new fields to runtime.timer, and adjust the various timer
functions in preparation for adding timers to P's. This continues to
use the old timer code.
Updates #6239
Updates #27707
Change-Id: I9adb3814f657e083ec5e22736c4b5b52b77b6a3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/171829
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Add support to the main scheduler loop for handling timers on P's.
This is not used yet, as timers are not yet put on P's.
Updates #6239
Updates #27707
Change-Id: I6a359df408629f333a9232142ce19e8be8496dae
Reviewed-on: https://go-review.googlesource.com/c/go/+/171826
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
As a small step toward speeding up timers, restrict modification
of the timer.when field to the timer code itself. Other code that
wants to change the when field of an existing timer must now call
resettimer rather than changing the when field and calling addtimer.
The new resettimer function also works for a new timer.
This is just a refactoring in preparation for later code.
Updates #27707
Change-Id: Iccd5dcad415ffbeac4c2a3cf015e91f82692acf8
Reviewed-on: https://go-review.googlesource.com/c/go/+/171825
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
You were a useful port and you've served your purpose.
Thanks for all the play.
A subsequent CL will remove amd64p32 (including assembly files and
toolchain bits) and remaining bits. The amd64p32 removal will be
separated into its own CL in case we want to support the Linux x32 ABI
in the future and want our old amd64p32 support as a starting point.
Updates #30439
Change-Id: Ia3a0c7d49804adc87bf52a4dea7e3d3007f2b1cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/199499
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This adds a platform-independent implementation of nacl's faketime
support. It can be enabled by setting the faketime build tag.
Updates #30439.
Change-Id: Iee097004d56d796e6d2bfdd303a092c067ade87e
Reviewed-on: https://go-review.googlesource.com/c/go/+/192740
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Move startNano from runtime to time package.
In preparation for a subsequent change that speeds up Since and Until.
This also makes code simpler as we have less assembly as the result,
monotonic time handling is better localized in time package.
This changes values returned from nanotime on windows
(it does not account for startNano anymore), current comments state
that it's important, but it's unclear how it can be important
since no other OS does this.
Update #25729
Change-Id: I2275d57b7b5ed8fd0d53eb0f19d55a86136cc555
Reviewed-on: https://go-review.googlesource.com/c/146340
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently when netpoll deadline is incrementally prolonged,
we delete and re-add timer each time.
Add modtimer function that does both and use it when we need
to modify an existing netpoll timer to avoid unnecessary lock/unlock.
TCP4OneShotTimeout-6 17.2µs ± 0% 17.0µs ± 0% -0.82% (p=0.008 n=5+5)
SetReadDeadline-6 274ns ± 2% 261ns ± 0% -4.89% (p=0.008 n=5+5)
Update #25729
Change-Id: I08b89dbbc1785dd180e967a37b0aa23b0c4613a8
Reviewed-on: https://go-review.googlesource.com/c/146339
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It's not always necessary to wake timerproc even if we add
a new timer to the top of the heap. Since we don't wake and
reset timerproc when we remove timers, it still can be sleeping
with shorter timeout. It such case it's more profitable to let it
sleep and then update timeout when it wakes on its own rather than
proactively wake it, let it update timeout and go to sleep again.
name old time/op new time/op delta
TCP4OneShotTimeout-6 18.6µs ± 1% 17.2µs ± 0% -7.66% (p=0.008 n=5+5)
SetReadDeadline-6 562ns ± 5% 319ns ± 1% -43.27% (p=0.008 n=5+5)
Update #25729
Change-Id: Iec8eacb8563dbc574a82358b3bac7ac479c16826
Reviewed-on: https://go-review.googlesource.com/c/146337
Reviewed-by: Ian Lance Taylor <iant@golang.org>
sys here is runtime/internal/sys.
Replace uses of sys.CacheLineSize for padding by
cpu.CacheLinePad or cpu.CacheLinePadSize.
Replace other uses of sys.CacheLineSize by cpu.CacheLineSize.
Remove now unused sys.CacheLineSize.
Updates #25203
Change-Id: I1daf410fe8f6c0493471c2ceccb9ca0a5a75ed8f
Reviewed-on: https://go-review.googlesource.com/126601
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If we run into data corruption due to the program accessing timers in
a racy way, do a normal panic rather than a hard crash with "panic
holding locks". The hope is to make the problem less confusing for users.
Fixes#25686
Change-Id: I863417adf21f7f8c088675b67a3acf49a0cdef41
Reviewed-on: https://go-review.googlesource.com/115815
Reviewed-by: Austin Clements <austin@google.com>
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
This is a re-roll of CL 99078, which was rolled
back because of failures on s390x.
Those failures were apparently due to an old version of gdb.
Change-Id: Icc2c12f449b2934063fd61e272e06237625ed589
Reviewed-on: https://go-review.googlesource.com/111256
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Ian Lance Taylor
Michael Anthony Knyszek
Changkun Ou
Andrew Bonventre
Draven
Brad Fitzpatrick
Austin Clements
Dmitry Vyukov
Martin Möhrmann
Josh Bleecher Snyder