| Age | Commit message (Collapse) | Author |
|---|
|
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 #27707
Fixes #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 #27707
Fixes #35274
Fixes #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 #27707
Fixes #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>
|
|
The addAdjustedTimers function was a late addition, and it got some of
the state machine wrong, leading to failures like
https://storage.googleapis.com/go-build-log/930576b6/windows-amd64-2016_53d0319e.log
Updates #6239
Updates #27707
Change-Id: I9e94e563b4698ff3035ce609055ca292b9cab3df
Reviewed-on: https://go-review.googlesource.com/c/go/+/204280
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
No big changes in the runtime package benchmarks.
Changes in the time package benchmarks:
name old time/op new time/op delta
AfterFunc-12 1.57ms ± 1% 0.07ms ± 1% -95.42% (p=0.000 n=10+8)
After-12 1.63ms ± 3% 0.11ms ± 1% -93.54% (p=0.000 n=9+10)
Stop-12 78.3µs ± 3% 73.6µs ± 3% -6.01% (p=0.000 n=9+10)
SimultaneousAfterFunc-12 138µs ± 1% 111µs ± 1% -19.57% (p=0.000 n=10+9)
StartStop-12 28.7µs ± 1% 31.5µs ± 5% +9.64% (p=0.000 n=10+7)
Reset-12 6.78µs ± 1% 4.24µs ± 7% -37.45% (p=0.000 n=9+10)
Sleep-12 183µs ± 1% 125µs ± 1% -31.67% (p=0.000 n=10+9)
Ticker-12 5.40ms ± 2% 0.03ms ± 1% -99.43% (p=0.000 n=10+10)
Sub-12 114ns ± 1% 113ns ± 3% ~ (p=0.069 n=9+10)
Now-12 37.2ns ± 1% 36.8ns ± 3% ~ (p=0.287 n=8+8)
NowUnixNano-12 38.1ns ± 2% 37.4ns ± 3% -1.87% (p=0.020 n=10+9)
Format-12 252ns ± 2% 195ns ± 3% -22.61% (p=0.000 n=9+10)
FormatNow-12 234ns ± 1% 177ns ± 2% -24.34% (p=0.000 n=10+10)
MarshalJSON-12 320ns ± 2% 250ns ± 0% -21.94% (p=0.000 n=8+8)
MarshalText-12 320ns ± 2% 245ns ± 2% -23.30% (p=0.000 n=9+10)
Parse-12 206ns ± 2% 208ns ± 4% ~ (p=0.084 n=10+10)
ParseDuration-12 89.1ns ± 1% 86.6ns ± 3% -2.78% (p=0.000 n=10+10)
Hour-12 4.43ns ± 2% 4.46ns ± 1% ~ (p=0.324 n=10+8)
Second-12 4.47ns ± 1% 4.40ns ± 3% ~ (p=0.145 n=9+10)
Year-12 14.6ns ± 1% 14.7ns ± 2% ~ (p=0.112 n=9+9)
Day-12 20.1ns ± 3% 20.2ns ± 1% ~ (p=0.404 n=10+9)
Updates #6239
Updates #27707
Change-Id: I51e25a90f941574f1a9cf83a22e84ac8c678537d
Reviewed-on: https://go-review.googlesource.com/c/go/+/171883
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.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>
|
|
Updates #27707
Change-Id: Id4b37594511895f404ee3c09a85263b2b35f835d
Reviewed-on: https://go-review.googlesource.com/c/go/+/171881
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>
|
|
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>
|
|
Updates #27707
Change-Id: I51da8a04ec12ba1efa435e86e3a15d4d13c96c45
Reviewed-on: https://go-review.googlesource.com/c/go/+/171879
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
|
|
Updates #27707
Change-Id: I1e65effb708911c727d126c51e0f50fe219f42ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/171878
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>
|
|
Updates #27707
Change-Id: Idda31d0065064a81c570e291ef588d020871997d
Reviewed-on: https://go-review.googlesource.com/c/go/+/171836
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>
|
|
Updates #27707
Change-Id: I02f97ec7869ec8a3fb2dfc94cff246badc7ea0fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/171833
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.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>
|
|
Updates #27707
Change-Id: I720e8af9e183c75abcb63ccc30466734c8dba74f
Reviewed-on: https://go-review.googlesource.com/c/go/+/171831
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>
|
|
Updates #6239
Updates #27707
Change-Id: I52cab8bf3dc8c552463725fc1d9e4e6b12230b03
Reviewed-on: https://go-review.googlesource.com/c/go/+/171828
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>
|
|
The nanotime wrappers in runtime introduce a bunch
of unnecessary code onto hot paths, e.g.:
0000000000449d70 <time.runtimeNano>:
449d70: 64 48 8b 0c 25 f8 ff mov %fs:0xfffffffffffffff8,%rcx
449d77: ff ff
449d79: 48 3b 61 10 cmp 0x10(%rcx),%rsp
449d7d: 76 26 jbe 449da5 <time.runtimeNano+0x35>
449d7f: 48 83 ec 10 sub $0x10,%rsp
449d83: 48 89 6c 24 08 mov %rbp,0x8(%rsp)
449d88: 48 8d 6c 24 08 lea 0x8(%rsp),%rbp
449d8d: e8 ae 18 01 00 callq 45b640 <runtime.nanotime>
449d92: 48 8b 04 24 mov (%rsp),%rax
449d96: 48 89 44 24 18 mov %rax,0x18(%rsp)
449d9b: 48 8b 6c 24 08 mov 0x8(%rsp),%rbp
449da0: 48 83 c4 10 add $0x10,%rsp
449da4: c3 retq
449da5: e8 56 e0 00 00 callq 457e00 <runtime.morestack_noctxt>
449daa: eb c4 jmp 449d70 <time.runtimeNano>
Move them to the corresponding packages which eliminates all of this.
name old time/op new time/op delta
TCP4OneShotTimeout-6 17.1µs ± 1% 17.0µs ± 0% -0.66% (p=0.032 n=5+5)
SetReadDeadline-6 234ns ± 1% 232ns ± 0% -0.77% (p=0.016 n=5+4)
Update #25729
Change-Id: Iee05027adcdc289ba895c5f5a37f154e451bc862
Reviewed-on: https://go-review.googlesource.com/c/146342
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@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>
|
|
This reverts commit 4eea887fd477368653f6fcf8ad766030167936e5.
Reason for revert: broke s390x build
Change-Id: Id6c2b6a7319273c4d21f613d4cdd38b00d49f847
Reviewed-on: https://go-review.googlesource.com/100375
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
|
|
Every time I poke at #14921, the g.waitreason string
pointer writes show up.
They're not particularly important performance-wise,
but it'd be nice to clear the noise away.
And it does open up a few extra bytes in the g struct
for some future use.
Change-Id: I7ffbd52fbc2a286931a2218038fda52ed6473cc9
Reviewed-on: https://go-review.googlesource.com/99078
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
The cgo checker was issuing an error with cgocheck=2 when a timer
bucket was stored in a pollDesc. The pollDesc values are allocated
using persistentalloc, so they are not in the Go heap. The code is OK
since timer bucket pointers point into a global array, and as such are
never garbage collected or moved.
Mark timersBucket notinheap to avoid the problem. timersBucket values
only occur in the global timers array.
Fixes #23435
Change-Id: I835f31caafd54cdacc692db5989de63bb49e7697
Reviewed-on: https://go-review.googlesource.com/87637
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
|
Otherwise low-res timers cause problems at call sites that expect to
be able to use 0 as meaning "no time set" and therefore expect that
nanotime never returns 0 itself. For example, sched.lastpoll == 0
means no last poll.
Fixes #22394.
Change-Id: Iea28acfddfff6f46bc90f041ec173e0fea591285
Reviewed-on: https://go-review.googlesource.com/73410
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
|
|
Return early from deltimer, with false as the result,
to indicate that we couldn't delete the timer since its
timersBucket was nil(not set) in the first place.
That happens in such a case where a user created
the timer from a Ticker with:
t := time.Ticker{C: c}
The above usage skips the entire setup of assigning
the appropriate underlying runtimeTimer and timersBucket,
steps that are done for us by time.NewTicker.
CL 34784 introduced this bug with an optimization, by changing
stopTimer to retrieve the timersBucket from the timer itself
(which is unset with the mentioned usage pattern above),
whereas the old behavior relied on indexing
by goroutine ID into the global slice of runtime
timers, to retrieve the appropriate timersBucket.
Fixes #21874
Change-Id: Ie9ccc6bdee685414b2430dc4aa74ef618cea2b33
Reviewed-on: https://go-review.googlesource.com/63970
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Write the moving timer only once, since it is overwritten
by swapped timers on all the iterations except the last one.
Additionally, explicitly pass timers heap into siftupTimer
and siftdownTimer in order to make the code more clear.
Relevant benchmark results on linux/amd64:
Stop 700µs ± 7% 608µs ± 1% -13.13% (p=0.000 n=10+10)
Stop-2 440µs ± 4% 376µs ± 4% -14.48% (p=0.000 n=10+10)
Stop-4 339µs ± 2% 330µs ± 3% -2.66% (p=0.015 n=10+10)
SimultaneousAfterFunc 702µs ± 9% 709µs ± 1% ~ (p=0.436 n=9+9)
SimultaneousAfterFunc-2 573µs ± 2% 546µs ± 2% -4.71% (p=0.000 n=10+10)
SimultaneousAfterFunc-4 387µs ± 1% 368µs ± 1% -4.89% (p=0.000 n=8+10)
StartStop 268µs ± 0% 270µs ± 0% +0.91% (p=0.000 n=9+9)
StartStop-2 155µs ± 6% 145µs ± 6% -6.70% (p=0.000 n=10+10)
StartStop-4 125µs ± 1% 124µs ± 1% ~ (p=0.065 n=10+9)
Change-Id: I3685835b5e3e82844e2e5e73ee03a1e22100bf0e
Reviewed-on: https://go-review.googlesource.com/63110
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Use per-P timers, so each P may work with its own timers.
This CL improves performance on multi-CPU systems
in the following cases:
- When serving high number of concurrent connections
with read/write deadlines set (for instance, highly loaded
net/http server).
- When using high number of concurrent timers. These timers
may be implicitly created via context.WithDeadline
or context.WithTimeout.
Production servers should usually set timeout on connections
and external requests in order to prevent from resource leakage.
See https://blog.cloudflare.com/the-complete-guide-to-golang-net-http-timeouts/
Below are relevant benchmark results for various GOMAXPROCS values
on linux/amd64:
context package:
name old time/op new time/op delta
WithTimeout/concurrency=40 4.92µs ± 0% 5.17µs ± 1% +5.07% (p=0.000 n=9+9)
WithTimeout/concurrency=4000 6.03µs ± 1% 6.49µs ± 0% +7.63% (p=0.000 n=8+10)
WithTimeout/concurrency=400000 8.58µs ± 7% 9.02µs ± 4% +5.02% (p=0.019 n=10+10)
name old time/op new time/op delta
WithTimeout/concurrency=40-2 3.70µs ± 1% 2.78µs ± 4% -24.90% (p=0.000 n=8+9)
WithTimeout/concurrency=4000-2 4.49µs ± 4% 3.67µs ± 5% -18.26% (p=0.000 n=10+10)
WithTimeout/concurrency=400000-2 6.16µs ±10% 5.15µs ±13% -16.30% (p=0.000 n=10+10)
name old time/op new time/op delta
WithTimeout/concurrency=40-4 3.58µs ± 1% 2.64µs ± 2% -26.13% (p=0.000 n=9+10)
WithTimeout/concurrency=4000-4 4.17µs ± 0% 3.32µs ± 1% -20.36% (p=0.000 n=10+10)
WithTimeout/concurrency=400000-4 5.57µs ± 9% 4.83µs ±10% -13.27% (p=0.001 n=10+10)
time package:
name old time/op new time/op delta
AfterFunc 6.15ms ± 3% 6.07ms ± 2% ~ (p=0.133 n=10+9)
AfterFunc-2 3.43ms ± 1% 3.56ms ± 1% +3.91% (p=0.000 n=10+9)
AfterFunc-4 5.04ms ± 2% 2.36ms ± 0% -53.20% (p=0.000 n=10+9)
After 6.54ms ± 2% 6.49ms ± 3% ~ (p=0.393 n=10+10)
After-2 3.68ms ± 1% 3.87ms ± 0% +5.14% (p=0.000 n=9+9)
After-4 6.66ms ± 1% 2.87ms ± 1% -56.89% (p=0.000 n=10+10)
Stop 698µs ± 2% 689µs ± 1% -1.26% (p=0.011 n=10+10)
Stop-2 729µs ± 2% 434µs ± 3% -40.49% (p=0.000 n=10+10)
Stop-4 837µs ± 3% 333µs ± 2% -60.20% (p=0.000 n=10+10)
SimultaneousAfterFunc 694µs ± 1% 692µs ± 7% ~ (p=0.481 n=10+10)
SimultaneousAfterFunc-2 714µs ± 3% 569µs ± 2% -20.33% (p=0.000 n=10+10)
SimultaneousAfterFunc-4 782µs ± 2% 386µs ± 2% -50.67% (p=0.000 n=10+10)
StartStop 267µs ± 3% 274µs ± 0% +2.64% (p=0.000 n=8+9)
StartStop-2 238µs ± 2% 140µs ± 3% -40.95% (p=0.000 n=10+8)
StartStop-4 320µs ± 1% 125µs ± 1% -61.02% (p=0.000 n=9+9)
Reset 75.0µs ± 1% 77.5µs ± 2% +3.38% (p=0.000 n=10+10)
Reset-2 150µs ± 2% 40µs ± 5% -73.09% (p=0.000 n=10+9)
Reset-4 226µs ± 1% 33µs ± 1% -85.42% (p=0.000 n=10+10)
Sleep 857µs ± 6% 878µs ± 9% ~ (p=0.079 n=10+9)
Sleep-2 617µs ± 4% 585µs ± 2% -5.21% (p=0.000 n=10+10)
Sleep-4 689µs ± 3% 465µs ± 4% -32.53% (p=0.000 n=10+10)
Ticker 55.9ms ± 2% 55.9ms ± 2% ~ (p=0.971 n=10+10)
Ticker-2 28.7ms ± 2% 28.1ms ± 1% -2.06% (p=0.000 n=10+10)
Ticker-4 14.6ms ± 0% 13.6ms ± 1% -6.80% (p=0.000 n=9+10)
Fixes #15133
Change-Id: I6f4b09d2db8c5bec93146db6501b44dbfe5c0ac4
Reviewed-on: https://go-review.googlesource.com/34784
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>
|
|
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 will make it possible to use the poller with the os package.
This is a lot of code movement but the behavior is intended to be
unchanged.
Update #6817.
Update #7903.
Update #15021.
Update #18507.
Change-Id: I1413685928017c32df5654ded73a2643820977ae
Reviewed-on: https://go-review.googlesource.com/36799
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
|
|
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>
|
|
See https://golang.org/design/12914-monotonic for details.
Fixes #12914.
Change-Id: I80edc2e6c012b4ace7161c84cf067d444381a009
Reviewed-on: https://go-review.googlesource.com/36255
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Caleb Spare <cespare@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Change-Id: I5c501f598f41241e6d7b21d98a126827a3c3ad9a
Reviewed-on: https://go-review.googlesource.com/32018
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
|
|
Fixes #14259
Change-Id: I23fedec0eb85ae28e56bc24539bc864674856130
Reviewed-on: https://go-review.googlesource.com/19318
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Stip uninteresting bottom and top frames from trace stacks.
This makes both binary and json trace files smaller,
and also makes stacks shorter and more readable in the viewer.
Change-Id: Ib9c80ccc280504f0e235f867f53f1d2652c41583
Reviewed-on: https://go-review.googlesource.com/5523
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
|
