From 66c60f076c167f252b0d8f0ac37f4c5c0c2adb51 Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 13 Jul 2022 11:48:04 -0400 Subject: [release-branch.go1.17] runtime: clear timerModifiedEarliest when last timer is deleted timerModifiedEarliest contains the lowest possible expiration for a modified earlier timer, which may be earlier than timer0When because we haven't yet updated the heap. Note "may", as the modified earlier timer that set timerModifiedEarliest may have since been modified later or deleted. We can clear timerModifiedEarliest when the last timer is deleted because by definition there must not be any modified earlier timers. Why does this matter? checkTimersNoP claims that there is work to do if timerModifiedEarliest has passed, causing findRunnable to loop back around to checkTimers. But the code to clean up timerModifiedEarliest in checkTimers (i.e., the call to adjusttimers) is conditional behind a check that len(pp.timers) > 0. Without clearing timerModifiedEarliest, a spinning M that would otherwise go to sleep will busy loop in findRunnable until some other work is available. Note that changing the condition on the call to adjusttimers would also be a valid fix. I took this approach because it feels a bit cleaner to clean up timerModifiedEarliest as soon as it is known to be irrelevant. For #51654. Fixes #53846. Change-Id: I3f3787c67781cac7ce87939c5706cef8db927dd5 Reviewed-on: https://go-review.googlesource.com/c/go/+/417434 Auto-Submit: Michael Pratt TryBot-Result: Gopher Robot Run-TryBot: Michael Pratt Reviewed-by: Michael Knyszek Reviewed-by: Ian Lance Taylor (cherry picked from commit c006b7ac2765252f397dec40fef610a3c17d956d) Reviewed-on: https://go-review.googlesource.com/c/go/+/417476 --- src/runtime/time.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/runtime/time.go b/src/runtime/time.go index 517a493a70..70e21d4482 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -390,7 +390,11 @@ func dodeltimer(pp *p, i int) int { if i == 0 { updateTimer0When(pp) } - atomic.Xadd(&pp.numTimers, -1) + n := atomic.Xadd(&pp.numTimers, -1) + if n == 0 { + // If there are no timers, then clearly none are modified. + atomic.Store64(&pp.timerModifiedEarliest, 0) + } return smallestChanged } @@ -414,7 +418,11 @@ func dodeltimer0(pp *p) { siftdownTimer(pp.timers, 0) } updateTimer0When(pp) - atomic.Xadd(&pp.numTimers, -1) + n := atomic.Xadd(&pp.numTimers, -1) + if n == 0 { + // If there are no timers, then clearly none are modified. + atomic.Store64(&pp.timerModifiedEarliest, 0) + } } // modtimer modifies an existing timer. -- cgit v1.2.3-54-g00ecf