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: new Init() and Start() methods for Timer #54870
Comments
please include the proposed function signatures and a description of what they do here. |
Proposed function signatures: func (t *Timer) Init()
func (t *Timer) Start(d Duration)
func (t *Ticker) Init()
func (t *Ticker) Tick(d Duration) Keep in mind that these new methods is not introduce new functionality, They just get some functionalities from NewTimer & NewTicker and made them be simple. Init() method as usuall initialize the new allocated object. Start() or Tick() method will start timer(runtime package) that will send signal (on the channel) once or on periods base on its type. |
Please write doc comments for these new methods. Thanks. In particular, what does the |
Although I wrote comments in #54861, you are right I must copy them here for better referencing. // Init initialize the Timer.
// It must call before call Start() or Reset() methods
func (t *Timer) Init()
// Start start the timer that will send
// the current time on its channel after at least duration d.
func (t *Timer) Start(d Duration)
// Init initialize the Ticker.
// It must call before call Tick() or Reset() methods
func (t *Ticker) Init()
// Tick start the timer that will send
// the current time on its channel after each tick.
func (t *Ticker) Tick(d Duration) As said in the first comment, Due to type HeartBeatService struct {
// other fields
interval time.Duration
ticker time.Ticker
}
func (h *HeartBeatService) Init(interval time.Duration) {
h.interval = interval
h.ticker.Init()
}
func (h *HeartBeatService) Reinit() {
h.ticker.Stop()
// can flush h.ticker.C here too.
}
func (h *HeartBeatService) Deinit() {
h.ticker.Stop()
}
func (h *HeartBeatService) Process() {
h.ticker.Start(h.interval)
for now := range h.ticker.C {
// send signal
}
} In this way, each new methods have a single responsibility and complete the life cycle of |
What does Init do that Start cannot do instead? |
@rsc But anyway you are the ruler, if you don't like |
It sounds like the goal is purely to avoid allocations. But each Timer is kept in a heap stored in the runtime, and we don't want the runtime's copy to be writable by user code, or else the heap invariants can be violated. So I don't think we want the runtime to allow the user to provide the memory for a Timer. Is there a program or real-world benchmark you can point to that is suffering from these allocations? |
This proposal has been added to the active column of the proposals project |
@rsc How runtime's copy can be writable by user code?
When any developer decides to embed a structure into another one, It means that have enough knowledge about bad things that may happen like memory leaks and it is like many other features in Go such as string that can be a huge problem if we don't be careful.
I know the usage of proposed APIs is rare (in comparison to other features) and usually used in low-level libraries such as /runtime/netpoll.go/pollDesc but it can be a helpful feature in some use-cases like implementing network protocols in user space (instead of kernel space) as QUIC. P.S.: I try to rewrite the timer package and get all functionality away from the Go runtime and time packages, It is not ready to use yet but it is a possible way. |
The fact that the fields are private doesn't prevent someone from writing var v HeartBeatService
v.Init()
v = HeartBeatService{}
That is interesting. The current timer implementation is closely tied to the scheduler, as I expect you know. This has been guided by a desire for efficiency and timer accuracy, especially for programs with millions of timers. That is, performance and accuracy are extremely important here. If we can get comparable results by moving this out of the runtime, that would be great. But it sounds hard. |
/runtime/time.go/timerNoStatus is the answer that if any developer tries to do in your way, in many logics in timing like this will drop its timer. I know it is a bug but the bug came from the developer's mistake not the timer package that try to reallocation without
Until now, I know we can't have the In overall I know it is not so hard to split the Go runtime package, It is just about giving responsibility to some ones that have enough rights. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
As suggested here #54861 by add these two new methods to time package, we achieve this goals:
If devs want to add a Timer to other structs (embed or as a field) to allocate both structs in one request,
existing NewTimer() don't let us do this.
The text was updated successfully, but these errors were encountered: