-
Notifications
You must be signed in to change notification settings - Fork 208
use Stopwatch for ElapsedMilliseconds #347
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
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.
While this is less cryptic for people scanning (and I also agree that the math is v error prone wrt overflow and/or getting int64 vs double conversions wrong), this line does allocate so I went the other way in my helpers in https://github.com/jet/propulsion/blob/master/src/Propulsion/Internal.fs#L9-L18
Having said that, I also do have lots of places where I actually want/need it to be in ticks so I can add lots together and then render variously as seconds, or ms.
I'd also note that the jitter and the compiler also have plenty special casing to make the math and/or inlining be more efficient than you'd expect for the code as it was
Also .net 7 adds a System.Diagnostics.Stopwatch.GetElapsedTime() so maybe that can be used?
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.
👍 the shift to GetTimestamp() was to avoid a wasted allocation here; using GetElapsedTime() would be the best-of-both-worlds solution
No description provided.