Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: special case timer channels #8898

Closed
dvyukov opened this issue Oct 7, 2014 · 9 comments
Closed

runtime: special case timer channels #8898

dvyukov opened this issue Oct 7, 2014 · 9 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Oct 7, 2014

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.
@bradfitz
Copy link
Contributor

bradfitz commented Apr 2, 2015

Comments about this bug are in https://go-review.googlesource.com/#/c/8356/

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc
Copy link
Contributor

rsc commented Mar 18, 2016

I intend to look into this for Go 1.7. I've started work on a CL.

@dvyukov
Copy link
Member Author

dvyukov commented Mar 18, 2016

Note there is a caveat: if we have code like:

t := time.NewTimer(time.Munute)
for {
  select {
  case <-t.C:
  case ...
  }
}

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.
Or do you have some smart plan for this?

@adg adg modified the milestones: Go1.8Early, Go1.7Early May 4, 2016
@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 30, 2016
@quentinmit
Copy link
Contributor

@rsc is this going to make 1.8?

@gyf19
Copy link

gyf19 commented Oct 18, 2016

@dvyukov @rsc ping?

@rsc
Copy link
Contributor

rsc commented Oct 20, 2016

Did not get to this.

@bcmills
Copy link
Member

bcmills commented Nov 7, 2016

  1. No need to stop timers. If nobody waits on a timers, it's just a normal heap object
    that can be garbage collected.

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 gc compiler but leak memory when compiled with other implementations).

Unless you're proposing to make a corresponding language change, in which case this seems like it needs to be a full-fledged proposal.

  1. Can combine time.Ticker/After into a single allocation.

There is a more general version of this optimization which we might want to consider anyway. If we:

  • have a struct with a pointer or reference-type field, and
  • can detect at compile-time that the referent of that field never changes (because it is not exported and never reassigned in methods), and
  • heuristically believe that the struct and referent always or almost always have the same lifetime (e.g. because the struct contains unexported fields, has only pointer methods, and is always returned by pointer),

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 select.

@bradfitz bradfitz added early-in-cycle A change that should be done early in the 3 month dev cycle. and removed early-in-cycle A change that should be done early in the 3 month dev cycle. labels Jun 14, 2017
@bradfitz bradfitz modified the milestones: Go1.10Early, Go1.10 Jun 14, 2017
@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 19, 2018
@bradfitz bradfitz modified the milestones: Go1.12, Go1.11.3, Go1.13 Nov 3, 2018
@bradfitz bradfitz modified the milestones: Go1.13, Go1.14 Apr 29, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@rsc rsc removed their assignment Jun 23, 2022
@rsc
Copy link
Contributor

rsc commented Jul 23, 2023

This is now a duplicate of #61542.

@rsc rsc closed this as not planned Won't fix, can't repro, duplicate, stale Jul 23, 2023
@gopherbot
Copy link

Change https://go.dev/cl/512355 mentions this issue: time: garbage collect unstopped Tickers and Timers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

8 participants