runtime: special case timer channels #8898
Comments
|
Comments about this bug are in https://go-review.googlesource.com/#/c/8356/ |
|
I intend to look into this for Go 1.7. I've started work on a CL. |
|
Note there is a caveat: if we have code like: Currently we arm and install the timer once. If we special case timer channels, we will need to install and uninstall the timer for each select. |
quentinmit
added
the
NeedsFix
|
@rsc is this going to make 1.8? |
|
Did not get to this. |
That seems like a disadvantage, not an advantage: it would encourage writing non-portable Go code (that is, programs which appear to work when using the Unless you're proposing to make a corresponding language change, in which case this seems like it needs to be a full-fledged proposal.
There is a more general version of this optimization which we might want to consider anyway. If we:
then we can (and arguably should) combine the struct and the referent into a single allocation. That optimization is more or less orthogonal to any special-casing for timer channels in |
bradfitz
added
early-in-cycle
|
This is now a duplicate of #61542. |
|
Change https://go.dev/cl/512355 mentions this issue: |
From the beginning of Go, the time package has had a gotcha: if you use a select on <-time.After(1*time.Minute), even if the select finishes immediately because some other case is ready, the underlying timer from time.After keeps running until the minute is over. This pins the timer in the timer heap, which keeps it from being garbage collected and in extreme cases also slows down timer operations. The lack of garbage collection is the more important problem. The docs for After warn against this scenario and suggest using NewTimer with a call to Stop after the select instead, purely to work around this garbage collection problem. Oddly, the docs for NewTimer and NewTicker do not mention this problem, but they have the same issue: they cannot be collected until either they are Stopped or, in the case of Timer, the timer expires. (Tickers repeat, so they never expire.) People have built up a shared knowledge that timers and tickers need to defer t.Stop even though the docs do not mention this (it is somewhat implied by the After docs). This CL fixes the garbage collection problem, so that a timer that is unreferenced can be GC'ed immediately, even if it is still running. The approach is to only insert the timer into the heap when some channel operation is blocked on it; the last channel operation to stop using the timer takes it back out of the heap. When a timer's channel is no longer referenced, there are no channel operations blocked on it, so it's not in the heap, so it can be GC'ed immediately. This CL adds an undocumented GODEBUG asynctimerchan=1 that will disable the change. The documentation happens in the CL 568341. Fixes #8898. Fixes #61542. Change-Id: Ieb303b6de1fb3527d3256135151a9e983f3c27e6 Reviewed-on: https://go-review.googlesource.com/c/go/+/512355 Reviewed-by: Austin Clements <austin@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Russ Cox <rsc@golang.org>
|
Submitted the CL! |
Consider special-casing timer channels created with time.Ticker and time.After. Namely, such chans need to contain next fire time and period. Then receive from such looks like: func chanrecv(c *Hchan) { ... if c.isTimeChan { for { next := c.nextTime if next == -1 { // already fired one-time timer blockForever() } newTime := -1 if c.period != 0 { newTime = next + c.period } if CAS(&c.nextTime, next, newTime) { wait := next - now if wait > 0 { sleepFor(wait) } return } } } ... } This has several advantages: 1. No need to stop timers. If nobody waits on a timers, it's just a normal heap object that can be garbage collected. 2. Faster operation for both blocking and non-blocking case. 3. Faster selects involving time chans. 4. Can combine time.Ticker/After into a single allocation.The text was updated successfully, but these errors were encountered: