Implement issue 13433 - add option for using coarser realtime clock. #2584
Conversation
|
dlang/druntime#981 should fix the build error on FreeBSD. |
| } | ||
|
|
||
| /++ Ditto +/ | ||
| static SysTime currTime(bool coarse) @safe |
There was a problem hiding this comment.
Booleans like this are pretty bad interface design. It's impossible to see at a glance what the boolean is actually for, i.e. it can't be inferred from the function name, so it has to be memorized from the documentation.
Can we please use an enum instead?
There was a problem hiding this comment.
Another option would be to split currTime and currStdTime into two functions, one that gives coarse time and one that doesn't. This reduces branching as the decision to use coarse time or not is almost always going to be known at compile-time.
There was a problem hiding this comment.
Can we please use an enum instead?
Isn't it a job for std.typecons.Flag ?
There was a problem hiding this comment.
+1. However, I remember some time ago that somebody objected to std.typecons.Flag for some reason I can't remember... nevertheless, I agree we should use it, it does make the code a lot clearer.
There was a problem hiding this comment.
@quickfur That may have been here: #1965 (comment). The only problem with it that I can see is that the user has to import std.typecons to use the Yes and No structs.
|
To deal with the concerns raised about using a Once that has been accepted, I'll update this pull to use it. |
|
Okay. This has been updated to use |
| assert(norm1 <= norm2, format("%s %s", norm1, norm2)); | ||
| assert(abs(norm1 - norm2) <= limit); | ||
|
|
||
| auto coarse1 = Clock.currStdTime!(ClockType.coarse); |
There was a problem hiding this comment.
These 3 quartets of lines are exactly the same - foreach over a TypeTuple!(ClockType.coarse, ClockType.precise, ClockType.second) ?
|
Okay. The tests were updated to use |
|
FWIW, C |
| throw new TimeException("Failed in clock_gettime()."); | ||
|
|
||
| if(clock_gettime(clockArg, &ts) != 0) | ||
| throw new TimeException("Call to gettimeofday() failed"); |
There was a problem hiding this comment.
Shouldn't this say "Call to clock_gettime"?
There was a problem hiding this comment.
Yeah. It's gettimeofday for Mac OS X, and clock_gettime for Linux and FreeBSD, and apparently, I screwed up the message when I was doing some copy-pasting. Fixed now.
This adds an option for getting the time using an alternative clock chosen via core.time.ClockType, including access to a coarser clock as requested by issue# 13433. For ClockType.normal, the normal clock which has always been used is used. For ClockType.coarse, a faster clock is used if available even if it's coarser so long as it still has sub-second precision, and the normal clock is used if no such clock is available. For ClockType.precise, a more precise clock is used if available (currently only on FreeBSD), otherwise the normal clock is used. For ClockType.second, a faster clock is used which only has second precision. If no such clock is available, then the normal clock is used, but the result is truncated to second precision.
|
Any more comments on this or things that need to be changed before this can be merged? |
|
LGTM |
|
Auto-merge toggled on |
Implement issue 13433 - add option for using coarser realtime clock.
This adds an option for getting the time using the coarser, faster
realtime clock on systems that support it for those who need to get the
time frequently (and thus quickly) but don't need it to be particularly
precise.
https://issues.dlang.org/show_bug.cgi?id=13433
The OSX change is because it's the only one using
gettimeofday. I figured that it was better to change that to go off of the OS version rather than whetherclock_gettimeexisted or not.