From 4a3c3ec9966022fd6a02e1790f71536acd6bcf1e Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 14 Feb 2024 11:56:57 -0500 Subject: [PATCH] runtime: delete addtimer modtimer can always be used in place of addtimer. Do that and delete addtimer, avoiding duplicated logic. [This is one CL in a refactoring stack making very small changes in each step, so that any subtle bugs that we miss can be more easily pinpointed to a small change.] Change-Id: I70291796bdac3bef5e0850f039f6f4a1da4498ae Reviewed-on: https://go-review.googlesource.com/c/go/+/564118 Reviewed-by: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI --- src/runtime/time.go | 70 +++++++++------------------------------------ src/time/sleep.go | 2 +- src/time/tick.go | 2 +- 3 files changed, 16 insertions(+), 58 deletions(-) diff --git a/src/runtime/time.go b/src/runtime/time.go index c09fc1eac0..d75cab0ba8 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -44,18 +44,16 @@ type timer struct { // The pp, status, and nextwhen fields may only be used by code in this file. // // Code that creates a new timer value can set the when, period, f, -// arg, and seq fields. -// A new timer value may be passed to addtimer (called by time.startTimer). -// After doing that no fields may be touched. +// arg, and seq fields before the first call to modtimer. +// After that, period, f, arg, and seq are immutable. +// They may be read but not modified. // -// An active timer (one that has been passed to addtimer) may be +// An active timer (one that has been passed to modtimer) may be // passed to deltimer (time.stopTimer), after which it is no longer an // active timer. It is an inactive timer. // In an inactive timer the period, f, arg, and seq fields may be modified, // but not the when field. // It's OK to just drop an inactive timer and let the GC collect it. -// It's not OK to pass an inactive timer to addtimer. -// Only newly allocated timer values may be passed to addtimer. // // An active timer may be passed to modtimer. No fields may be touched. // It remains an active timer. @@ -64,18 +62,14 @@ type timer struct { // active timer with an updated when field. // It's OK to pass a newly allocated timer value to resettimer. // -// Timer operations are addtimer, deltimer, modtimer, resettimer, -// cleantimers, adjusttimers, and runtimer. +// Timer operations are deltimer, modtimer, adjusttimers, and runtimer. // -// We don't permit calling addtimer/deltimer/modtimer/resettimer simultaneously, +// We don't permit calling deltimer/modtimer simultaneously, // but adjusttimers and runtimer can be called at the same time as any of those. // // Active timers live in heaps attached to P, in the timers field. // Inactive timers live there too temporarily, until they are removed. // -// addtimer: -// timerNoStatus -> timerWaiting -// anything else -> panic: invalid value // deltimer: // timerWaiting -> timerModifying -> timerDeleted // timerModifiedEarlier -> timerModifying -> timerDeleted @@ -209,7 +203,10 @@ func startTimer(t *timer) { if raceenabled { racerelease(unsafe.Pointer(t)) } - addtimer(t) + if t.status.Load() != timerNoStatus { + throw("startTimer called with initialized timer") + } + resettimer(t, t.when) } // stopTimer stops a timer. @@ -235,8 +232,8 @@ func resetTimer(t *timer, when int64) bool { // modTimer modifies an existing timer. // //go:linkname modTimer time.modTimer -func modTimer(t *timer, when, period int64, f func(any, uintptr), arg any, seq uintptr) { - modtimer(t, when, period, f, arg, seq) +func modTimer(t *timer, when, period int64) { + modtimer(t, when, period, t.f, t.arg, t.seq) } // Go runtime. @@ -246,42 +243,6 @@ func goroutineReady(arg any, seq uintptr) { goready(arg.(*g), 0) } -// Note: this changes some unsynchronized operations to synchronized operations -// addtimer adds a timer to the current P. -// This should only be called with a newly created timer. -// That avoids the risk of changing the when field of a timer in some P's heap, -// which could cause the heap to become unsorted. -func addtimer(t *timer) { - // when must be positive. A negative value will cause runtimer to - // overflow during its delta calculation and never expire other runtime - // timers. Zero will cause checkTimers to fail to notice the timer. - if t.when <= 0 { - throw("timer when must be positive") - } - if t.period < 0 { - throw("timer period must be non-negative") - } - if t.status.Load() != timerNoStatus { - throw("addtimer called with initialized timer") - } - t.status.Store(timerWaiting) - - when := t.when - - // Disable preemption while using pp to avoid changing another P's heap. - mp := acquirem() - - pp := getg().m.p.ptr() - lock(&pp.timersLock) - cleantimers(pp) - doaddtimer(pp, t) - unlock(&pp.timersLock) - - wakeNetPoller(when) - - releasem(mp) -} - // doaddtimer adds t to the current P's heap. // The caller must have locked the timers for pp. func doaddtimer(pp *p, t *timer) { @@ -429,7 +390,6 @@ loop: mp = acquirem() // Timer was already run and t is no longer in a heap. - // Act like addtimer. if t.status.CompareAndSwap(status, timerModifying) { wasRemoved = true pending = false // timer already run or stopped @@ -511,16 +471,14 @@ loop: // resettimer resets the time when a timer should fire. // If used for an inactive timer, the timer will become active. -// This should be called instead of addtimer if the timer value has been, -// or may have been, used previously. -// Reports whether the timer was modified before it was run. +// Reports whether the timer was active and was stopped. func resettimer(t *timer, when int64) bool { return modtimer(t, when, t.period, t.f, t.arg, t.seq) } // cleantimers cleans up the head of the timer queue. This speeds up // programs that create and delete timers; leaving them in the heap -// slows down addtimer. Reports whether no timer problems were found. +// slows down heap operations. Reports whether no timer problems were found. // The caller must have locked the timers for pp. func cleantimers(pp *p) { gp := getg() diff --git a/src/time/sleep.go b/src/time/sleep.go index 0aec4cacc6..ffc69bcd2a 100644 --- a/src/time/sleep.go +++ b/src/time/sleep.go @@ -41,7 +41,7 @@ func when(d Duration) int64 { func startTimer(*runtimeTimer) func stopTimer(*runtimeTimer) bool func resetTimer(*runtimeTimer, int64) bool -func modTimer(t *runtimeTimer, when, period int64, f func(any, uintptr), arg any, seq uintptr) +func modTimer(t *runtimeTimer, when, period int64) // The Timer type represents a single event. // When the Timer expires, the current time will be sent on C, diff --git a/src/time/tick.go b/src/time/tick.go index 9da16b5d58..e06810db5d 100644 --- a/src/time/tick.go +++ b/src/time/tick.go @@ -55,7 +55,7 @@ func (t *Ticker) Reset(d Duration) { if t.r.f == nil { panic("time: Reset called on uninitialized Ticker") } - modTimer(&t.r, when(d), int64(d), t.r.f, t.r.arg, t.r.seq) + modTimer(&t.r, when(d), int64(d)) } // Tick is a convenience wrapper for NewTicker providing access to the ticking -- 2.48.1