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
Labels
Milestone

Comments

@pjebs
Copy link

@pjebs 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
@gopherbot gopherbot added the Proposal label 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

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 14, 2020

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

This comment has been minimized.

Copy link
Author

@pjebs pjebs commented Jan 14, 2020

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 15, 2020

I think this is a dup of #33184.

@pjebs

This comment has been minimized.

Copy link
Author

@pjebs pjebs commented Jan 15, 2020

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

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 15, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.