-
Notifications
You must be signed in to change notification settings - Fork 953
runtime: avoid an allocation in (*time.Timer).Reset #4890
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. My only question is if we can just reuse the timer struct as-is and we don't need to reset or update any other fields.
834ec50
to
df1f8b7
Compare
Good point. PTAL. |
Looks good now, thanks for the improvement @eliasnaur and to @dgryski for review/feedback. Now merging. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit late, LGTM but there's a small nit in that the documentation now doesn't match the function. Might be a good idea to fix in a separate PR if you want.
@@ -117,11 +117,11 @@ func addTimer(tim *timerNode) { | |||
|
|||
// removeTimer is the implementation of time.stopTimer. It removes a timer from | |||
// the timer queue, returning true if the timer is present in the timer queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is now incorrect: it doesn't return a bool anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to change the comment in PR tinygo-org#4890.
I forgot to change the comment in PR #4890.
No description provided.