Implement time.NewTimer and time.NewTicker #1402
Conversation
This solves a few issues:
1. It avoids the nondeterministic order that was previously used to
declare these types, as they were declared while iterating through a
map.
2. The next commit adds a runtime type that also includes standard Go
types such as an interface and func value. If this type
(runtime.timer) was found before runtime._interface for example, the
runtime._interface type wasn't found triggering a panic.
3. It might improve the performance of getLLVMRuntimeType as it does
not involve a CGo call anymore.
| timerQueue = tn.next | ||
| tn.next = nil | ||
| // Run the callback stored in this timer node. | ||
| tn.callback(tn) |
There was a problem hiding this comment.
I don't think this is really safe with coroutine lowering right now?
Any user code can define a function value of the type func(interface{}, uintptr) which I think is sufficient to break this?
There was a problem hiding this comment.
I'm afraid you're right. I'll have to think a bit about how to fix this. It's not trivial, as the signature is set by the time package.
Thank you for taking a look. I hadn't thought of this.
There was a problem hiding this comment.
So on coroutines we should do go tn.callback(tn). I dunno, there is probably a way to improve this.
There was a problem hiding this comment.
No I didn't, I haven't really thought about how to do this in a way that works with the coroutines scheduler.
This commit adds support for time.NewTimer and time.NewTicker. It also adds support for the Stop() method on time.Timer, but doesn't (yet) add support for the Reset() method. The implementation has been carefully written so that programs that don't use these timers will normally not see an increase in RAM or binary size. None of the examples in the drivers repo change as a result of this commit. This comes at the cost of slightly more complex code and possibly slower execution of the timers when they are used.
|
When asyncify lands might be a good time to revisit this...? |
@dkegel-fastly that is a very good question. @aykevl what do you think? |
|
@aykevl Implementing
It would also be nice to be able to use time.After, time.Tick, etc. |
|
Closing in favor of #2954 |
This has been requested before in #1037 and it is necessary for using the stdlib testing package. I have compared the code size before and after and except for very nontrivial cases the code size is the same (even with the scheduler additions). All examples in the drivers repo remain the same.