-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
Use HPET-enabled Stopwatch class #575
Use HPET-enabled Stopwatch class #575
Conversation
It replaces DateTime.Now in ms-critical sections, as per .net documentation, DateTime.Now's resolution can have an impact on sub 100ms time intervals. https://docs.microsoft.com/en-us/dotnet/api/system.datetime?view=net-5.0#datetime-resolution
Co-Authored-By: X9VoiD <oscar.silvestrexx@gmail.com>
DeltaStopwatch wraps regular Stopwatch, providing incremental restarts with one kernel call without losing ticks. It also provides static methods to get app runtime timespan.
The stopwatch based timer is only more accurate than |
return delta; | ||
} | ||
} | ||
public double RestartMs() => Restart().TotalMilliseconds; |
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.
Remove these unnecessary helper methods, since they're only referenced like twice ever
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.
If it looks out of place, then sure, will remove those. Made them because most of the time milliseconds are what's being used in the end.
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.
Just looks odd for it to be exposed like that the main method going unused
namespace OpenTabletDriver.Plugin.Utils | ||
{ | ||
public class DeltaStopwatch |
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.
Change namespace to OpenTabletDriver.Plugin.Timing
and class name to HighPrecisionEventStopwatch
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.
Do we really need a wrapper for an already existing class? This seems kinda pointless.
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.
DeltaStopwach name was used because it allows for measuring laps without redundant queries, in contrast to regular Stopwatch. HighPrecisionEventStopwatch doesn't really reflect the difference between those classes, and is very long imo
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.
Class name length isn't an issue. Can we compromise on HPETDeltaStopwatch
as the class name?
HPET was introduced by intel in ~2005 and is included in basically every platform since. It starts running at a system startup and only ever costs you anything when you query it. The wrapper is there so that we can "lap" the stopwatch with just 1 kernel call/HPET query instead of two when querying elapsed and then restarting. This also eliminates the possibility of missing some ticks between checking the time and restarting. |
It replaces DateTime.Now in ms-critical sections,
as per .net documentation, DateTime.Now's resolution
can have an impact on sub 100ms time intervals.
https://docs.microsoft.com/en-us/dotnet/api/system.datetime?view=net-5.0#datetime-resolution