]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: adjust when timers.adjust is called
authorRuss Cox <rsc@golang.org>
Thu, 28 Mar 2024 01:46:12 +0000 (21:46 -0400)
committerGopher Robot <gobot@golang.org>
Thu, 28 Mar 2024 21:25:23 +0000 (21:25 +0000)
This new logic more closely mimics what we did before my CL stack.
I had reasoned that certainly

ts.adjust(now, force=true)
ts.run(now)

would be faster than

ts.adjust(now, force=false)
ts.run(now)
ts.adjust(now, force=true)

But certainty is just an emotion, and that turns out not to be the case.

I don't really understand why the second sequence is faster,
but it definitely is, so put it back.

goos: linux
goarch: amd64
pkg: time
cpu: AMD Ryzen 9 7950X 16-Core Processor
                                   │ s7base.txt  │               s7.txt               │
                                   │   sec/op    │   sec/op     vs base               │
AdjustTimers10000-32                 263.3µ ± 4%   239.9µ ± 5%  -8.87% (p=0.000 n=10)
AdjustTimers10000SingleThread-32     1.742m ± 3%   1.686m ± 8%       ~ (p=0.105 n=10)
AdjustTimers10000NoReset-32          192.2µ ± 2%   194.1µ ± 1%  +1.00% (p=0.009 n=10)
AdjustTimers10000NoSleep-32          237.0µ ± 2%   226.2µ ± 3%  -4.55% (p=0.001 n=10)
AdjustTimers10000NoResetNoSleep-32   185.2µ ± 1%   182.9µ ± 1%  -1.23% (p=0.003 n=10)

goos: darwin
goarch: arm64
pkg: time
cpu: Apple M3 Pro
                                   │ m3base.txt  │               m3.txt               │
                                   │   sec/op    │   sec/op     vs base               │
AdjustTimers10000-12                 272.6µ ± 3%   269.3µ ± 2%       ~ (p=0.063 n=10)
AdjustTimers10000SingleThread-12     1.126m ± 1%   1.176m ± 1%  +4.42% (p=0.000 n=10)
AdjustTimers10000NoReset-12          255.1µ ± 2%   262.6µ ± 2%  +2.96% (p=0.000 n=10)
AdjustTimers10000NoSleep-12          250.2µ ± 2%   247.8µ ± 1%       ~ (p=0.063 n=10)
AdjustTimers10000NoResetNoSleep-12   230.3µ ± 1%   231.0µ ± 1%       ~ (p=0.280 n=10)

Change-Id: I67b5765f97dfca0142ee38e15a9904b520f51e83
Reviewed-on: https://go-review.googlesource.com/c/go/+/574740
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/runtime/time.go

index 42f629d16843587b00f8d63bad5ef34f3e28b11e..b696c837abed5d3548c3188b7751a90cc8db6e47 100644 (file)
@@ -891,7 +891,7 @@ func (ts *timers) check(now int64) (rnow, pollUntil int64, ran bool) {
 
        ts.lock()
        if len(ts.heap) > 0 {
-               ts.adjust(now, force)
+               ts.adjust(now, false)
                for len(ts.heap) > 0 {
                        // Note that runtimer may temporarily unlock ts.
                        if tw := ts.run(now); tw != 0 {
@@ -902,6 +902,16 @@ func (ts *timers) check(now int64) (rnow, pollUntil int64, ran bool) {
                        }
                        ran = true
                }
+
+               // Note: Delaying the forced adjustment until after the ts.run
+               // (as opposed to calling ts.adjust(now, force) above)
+               // is significantly faster under contention, such as in
+               // package time's BenchmarkTimerAdjust10000,
+               // though we do not fully understand why.
+               force = ts == &getg().m.p.ptr().timers && int(ts.zombies.Load()) > int(ts.len.Load())/4
+               if force {
+                       ts.adjust(now, true)
+               }
        }
        ts.unlock()