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

proposal: time: allow Ticker to change duration #36544

Closed
pjebs opened this issue Jan 14, 2020 · 5 comments
Closed

proposal: time: allow Ticker to change duration #36544

pjebs opened this issue Jan 14, 2020 · 5 comments

Comments

@pjebs
Copy link
Contributor

pjebs commented Jan 14, 2020

It would be good to be able to dynamically change the Ticker duration. Even if it is not super accurate in between the change in duration.

[untested] It should behave something like this: (but more accurate/intelligent):

type Ticker struct {
	C      chan time.Time
	stop   chan struct{}
	unleak chan struct{}
	stopped bool

	sync.Mutex
	tt *time.Ticker
}

func NewTicker(d time.Duration) *Ticker {

	c := make(chan time.Time, 1)
	t := &Ticker{
		C:      c,
		stop:   make(chan struct{}),
		unleak: make(chan struct{}),
		tt:     time.NewTicker(d),
		stopped: false,
	}

	go func() {
		for {
			select {
			case now := <-t.tt.C:
				t.C <- now
			case <-t.unleak:
				return
			case <-t.stop:
				return
			}
		}

	}()

	return t
}

func (t *Ticker) Stop() {
	t.Lock()
	defer t.Unlock()

	if t.stopped {
		return
	}

	t.stop <- struct{}{}
	t.tt.Stop()
	t.stopped = true
}

func (t *Ticker) SetDuration(d time.Duration) {
	t.Lock()
	defer t.Unlock()

	if t.stopped {
		return
	}

	t.tt.Stop()
	t.tt = time.NewTicker(d)
	t.unleak <- struct{}{}

	go func() {
		for {
			select {
			case now := <-t.tt.C:
				t.C <- now
			case <-t.unleak:
				return
			case <-t.stop:
				return
			}
		}
	}()
}

I can see that some people have asked for it: https://stackoverflow.com/questions/36689774/dynamically-change-ticker-interval

@gopherbot gopherbot added this to the Proposal milestone Jan 14, 2020
@ianlancetaylor ianlancetaylor changed the title Proposal: Allow time.Ticker to change duration proposal: time: allow Ticker to change duration Jan 14, 2020
@ianlancetaylor
Copy link
Contributor

If the implementation requires adding two new channels then I don't think this is a good idea. Very few people need to change the duration of a Ticker, so we shouldn't make them more expensive just to support that.

@pjebs
Copy link
Contributor Author

pjebs commented Jan 14, 2020

I'm sure it can be done better from inside the time package

@ianlancetaylor
Copy link
Contributor

I think this is a dup of #33184.

@pjebs
Copy link
Contributor Author

pjebs commented Jan 15, 2020

Not sure if Reseting from #33184 can allow change in duration

@ianlancetaylor
Copy link
Contributor

There is nothing to reset in a ticker other than the duration. The suggestion in #33184 is literally func (t *Ticker) Reset(d Duration) bool.

I'm going to close this as a dup. Please comment if you disagree.

@golang golang locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants