core.time: Conversions and documentation

[CyberShadow]

I got absolutely fed up with having to check core.time's documentation and scrolling through pages of fluff to remember its inconsistent/obtuse conversion syntax, and I decided to fix it.

• You now can convert between Duration and TickDuration by using constructors (Duration(tickDuration) and vice-versa). Isn't this the proper way that all value types in D ought to convert between one another?
• The module top documentation now has two tables - one listing the structures and functions in the module, and another showing how to convert between various types.

Here's what the new module documentation looks like:

---

Needs dlang/phobos#1864.

[CyberShadow]

its inconsistent/obtuse conversion syntax

Here is an example:

TickDuration.seconds will return the total number of seconds in the TickDuration.

However, Duration.seconds will return the number of seconds in the Duration minus the larger units. WTF!

[MartinNowak]

However, Duration.seconds will return the number of seconds in the Duration minus the larger units. WTF!

Yeah, the naming could be better. One meaning is the total number of seconds and the other is seconds as fraction of a minute. How can we improve the method names?

[CyberShadow]

While preserving backwards compatibility? I'm not sure.

If I were to write the module from scratch, what I'd do is:

• Make Duration and TickDuration be nearly-identical types, using a common template mixin for most functionality, the only difference being the resolution (hnsecs, and the system high-resolution timer respectively).
• Getting a unit minus the larger unit is a rarely needed operation, usually used for printing the result. It should be hidden behind a template, like how we already have get!"seconds"() for Duration.
• Alternatively, we could use singular when getting units minus the larger units, and plural when getting the total number of units (so, both types have .second and .seconds). This is in line with the SysTime properties .year, .month etc. Disadvantage is that this puts bugs 1 letter away.
• I'm still not sure what FracSec is for and when is it useful, as I've never needed it, so I'd leave it out (but maybe someone can enlighten me).

I suppose that everything is possible with a deprecation path, but the changes would not be minor.

[jmdavis]

TickDuration was created by Shoo prior to std.datetime and really doesn't follow the rest of core.time or std.datetime. If I could go back, I wouldn't even have included it. I'd have made it so that Duration was used everywhere and then had a type which represented the monotonic time but would probably only convert to a real time by subtracting one monotonic time from another, resulting in a Duration. Having TickDuration just confuses things and doesn't really add much value IMHO. However, I don't know how much we can really do about it at this point, since removing it would break code. At minimum, I've considered adding an alias this to TickDuration which converts to Duration and then trying to phase out TickDuration from the few places in Phobos that it's used (e.g. benchmark).

[jmdavis]

I'm still not sure what FracSec is for and when is it useful, as I've never needed it, so I'd leave it out (but maybe someone can enlighten me).

It's primary use is for SysTime. It didn't make sense to use separate out milliseconds, microseconds, and hecto-nanoseconds the way that the day, hour, minute, etc. were separated out, because you wouldn't normally ask for the milliseconds and then want the microseconds without the milliseconds. Usually, when dealing with the fractional seconds, you just want them at a particular precision. So, you get the fractional seconds from a SysTime as a type and then get their value out of that at whatever precision you want. When separating out a Duration into separate units, it's in the same boat as SysTime, so FracSec made sense for its fractional seconds as well, so it ended up in core.time along with Duration. In most cases though, I would expect FracSec to be used with SysTime and not much else.

[jmdavis]

What's the point of adding these constructors over simply casting like you'd do now? I don't see how this adds anything.

[CyberShadow]

Casting should not be used as a mechanism for "safe" conversions between types. cast is a keyword in D specifically to aid code review to find places where something unsafe might be going on. I do not believe that overriding opCast for safe semantic conversions between types is in the spirit of D. It is also too much syntactic baggage for an operation as mundane as converting between these two similar types, and unlike e.g. .fracSec, you do not need to remember any property/method names to perform the conversion - just the type you want to convert to.

Using a constructor is already an established convention for conversion between types. We are still missing the int(x) syntax in the language for implicit casts to basic types, and I believe Andrei mentioned that he would be in favor of adding it.

[MartinNowak]

Yeah, casting easily looses the correct qualifier, which is not a problem here, because those are all value types.
Still it's a weird idiom for normal conversion because in general cast allow to do unsafe conversions.

Using a constructor is already an established convention for conversion between types. We are still missing the int(x)

Yep, it's scheduled for the next release and definitely necessary for generic programming.
http://wiki.dlang.org/Agenda#introduce_.22uniform_construction.22_syntax
Interesting side note, in C++ int(x) is actually a blunt C-style cast.
http://stackoverflow.com/questions/4474933/what-exactly-is-or-was-the-purpose-of-c-function-style-casts

[jmdavis]

If you don't like casting, then use std.conv.to. opCast is the standard way to define conversions between types.

Regardless, if we're going to introduce different conversions here, I'd be more inclined to create an alias this on TickDuration to implicitly convert to Duration and then not do anything about converting from Duration to TickDuration (and just leave the cast), since I don't think that converting from Duration to TickDuration makes sense under very many circumstances. You use TickDuration, because you need the high precision clock, and then you generally try and get the time out of a TickDuration - either by converting it to a Duration or calling one of its properties to convert it to seconds or milliseconds or whatever. At most, you keep it around to compare to other TickDurations. I don't think that much code is going to be interesting in convert a Duration into TickDuration.

[CyberShadow]

If you don't like casting, then use std.conv.to.

I don't think converting between these types should require a dependency on std.conv. It's great that std.conv can convert to it, as it aids generic code, but IMO it shouldn't be the only way (save for casting directly).

and then not do anything about converting from Duration to TickDuration (and just leave the cast), since I don't think that converting from Duration to TickDuration makes sense under very many circumstances.

I use Duration -> TickDuration conversions a LOT in my code. Why:

• The timer subsystem refers to the monotonic clock, and thus uses TickDuration internally.
• When you create a timer, you need to specify the duration as a TickDuration, because the interval you want the timer to have may come from the timer subsystem (and thus, using a Duration would involve a double conversion).
• When you create a timer, most of the time you want to specify the interval as a duration (e.g. 5.seconds).

There definitely needs to be an easy way to convert from Duration to TickDuration.

[CyberShadow]

@jmdavis Regarding FracSec, could you add an example to its documentation that showcases when it is useful? Because "I don't get it". If it's just about getting fractions of a second at a certain precision, there seems to be a lot of code/documentation/tests for what could have simply been a int fracSec(int digits) method.

[CyberShadow]

And regarding TickDuration:

However, I don't know how much we can really do about it at this point, since removing it would break code.

Programs still need a way to query high-resolution / monotonic clocks, whereas SysTime tracks real time. (I suppose you could stuff everything in SysTime, perhaps with a special fake "timezone" to indicate that the object indicates a monotonic clock, and not a real time clock).

[jmdavis]

Programs still need a way to query high-resolution / monotonic clocks, whereas SysTime tracks real time. (I suppose you could stuff everything in SysTime, perhaps with a special fake "timezone" to indicate that the object indicates a monotonic clock, and not a real time clock).

I certainly wouldn't suggest using SysTime instead of TickDuration. IMHO, what we should really have is a type which holds the monotonic time (which would be in ticks internally like TickDuration is), but it wouldn't have anything on it relating to real time in any way. In order to make a monotonic time meaningful in terms of seconds or the time of day or any of that, you really need to subtract one from another - i.e. get the duration of time which passed between the two monotonic times. And for that, I see no reason not to just use Duration. It's already in hecto-nanoseconds, which is more precise than most system clocks. And in the rare case that really needed the number of ticks for some reason, we could simply have the monotonic time type expose the number of ticks that it has.

We're currently trying to use TickDuration both to represent the monotonic time (by getting a TickDuration for the number of ticks since the app started) and for general durations in ticks. I posit that it's almost always useless as a duration in ticks, because Duration is already plenty precise, and if we have a monotonic time type, then we can get the monotonic system time for high performance timing, which is really the main reason why you'd currently use TickDuration rather than Duration.

So, we'd end up with something like

MonoTime before = MonoTime.currTime;
//do stuff
MonoTime after = MonoTime.currTime;
Duration timeElapsed = after - before;

The high precision, monotonic clock would be there with MonoTime, and Duration would give you a high precision duration between two MonoTimes. It wouldn't be in ticks internally, but hecto-nanoseconds is plenty precise for almost all cases (and more precise than most system clocks). So, you don't lose precision, and we get rid of all of this confusion with TickDuration vs Duration as well as the issues with TickDuration not using the same conventions as the rest of core.time or std.datetime.

[MartinNowak]

Sounds like you have some solid ideas for core.time improvements, would you want to add this as 2.067 goal (http://wiki.dlang.org/Agenda)?

However, I don't know how much we can really do about it at this point, since removing it would break code.

Until now we've always found acceptable solutions for deprecation after thinking hard enough about it, so I'm quite optimistic here too :).

[CyberShadow]

Good ideas. I agree that TickDuration playing the roles of both "point in time" and "time span" is not great. Is this purely hypothetical, or you're considering working on it?

BTW, why wasn't SysTime named simply Time? Is it about the origin (the system clock)? One may use SysTime without consulting the system clock, e.g. to do calendar operations.

[MartinNowak]

Regarding this pull request, the documentation overview is a huge improvement.
Adding a second conversion method might be questionable, but I'm in favor of using constructors or explicit functions too.

[MartinNowak]

Sure that std.conv.to can't use constructors too?

[jmdavis]

It might. It's also possible that it couldn't when this was written. Regardless, it can use opCast, and std.conv.to is pretty much the way to convert one type to another (especially because it tends to work much better in generic code), so that's what I would expect most code to be using anyway.

[CyberShadow]

Yes, it can. In fact, it had a bug where if the source type had opCast and the target had a constructor, there would be an ambiguity (this is dlang/phobos#1864 that this pull depends on). I guess opCast now only needs to stay to avoid breaking existing code that casts directly.

[MartinNowak]

Then replace the comment with // explicitly undocumented ... will be deprecated ..., that is if you two agree on this step.

[jmdavis]

Good ideas. I agree that TickDuration playing the roles of both "point in time" and "time span" is not great. Is this purely hypothetical, or you're considering working on it?

I've been debating it for a while but haven't done anything yet due to a combination of a lack of free time and a concern about code breakage. But I'm increasingly inclined to just do it and deal with the code breakage. Having both Duration and TickDuration is just too confusing. At that point, the only blocker is free time, which I'm expecting to start getting more of sometime next month. So, this is going on my todo list (probably after spliting std.datetime).

BTW, why wasn't SysTime named simply Time? Is it about the origin (the system clock)? One may use SysTime without consulting the system clock, e.g. to do calendar operations.

It was to distinguish it from DateTime, and because it's used for the system's wall clock time. Pure calendar operations (which don't care about time zone) should be done with DateTime. SysTime is for when you need a date/time which corresponds to an absolute point in time, whereas a DateTime could represent a fixed point in time in any number of time zones, as it doesn't have a time zone to tie it down.

I suppose that I could have called it Time, but that probably would have increased the confusion with TimeOfDay, and because you use it when you want the system's time, SysTime made sense. All in all, I think that all 4 of the time point types in std.datetime make good sense and that it wouldn't be desirable to try and remove any of them, but with 4 of them (SysTime, DateTime, Date, and TimeOfDay), it gets hard to name them in a way that makes it clear what they are and avoids confusion.

[CyberShadow]

I see. I'd forgotten about DateTime. I've never needed to use DateTime, Date and TimeOfDay either, though.

[jmdavis]

Regarding this pull request, the documentation overview is a huge improvement.
Adding a second conversion method might be questionable, but I'm in favor of using constructors or explicit functions too.

I agree that the documentation improvement is great. I just object to the addition of the constructors, because opCast already provides the necessary functionality, and you have std.conv.to if you prefer not to use casts. Personally, I wouldn't use a constructor for a type conversion - only for constructing a type from other values, which generally means multiple values or values which mean something completely different, not a single value of a similar type.

[jmdavis]

I see. I'd forgotten about DateTime. I've never needed to use DateTime, Date and TimeOfDay either, though.

They're ideal when you need to manipulate all of the individual properties (year, month, hour, etc.), because SysTime has to do a conversion from its internal representation every time you query for any of those properties, whereas they keep the individual properties as separate member variables. So, if you're doing a bunch of calendar stuff that doesn't need to be tied to a specific UTC time underneath the hood, it's better to use them. And SysTme uses them all internally for some stuff. I actually tried to remove them when porting SysTime to C++ for work, because we didn't need anything like that for our specific use case, and I ended up having to use them anyway in order to implement some of SysTime. Trying not to have them just made things worse.

[CyberShadow]

Well, what bugs me about the std.conv.to dependency is that this is a Druntime module, and it depends on Phobos, a standard library, which is a sort of reverse dependency. The reason why we did this split was to allow users to use other "standard libraries" like Tango with the same runtime, so is it really OK to require Phobos for everyday use of a Druntime module?

What would I put in the conversion table? std.conv.to!TickDuration(duration)? Seems overly verbose... duration.to!TickDuration() with an asterisk that mentions that you need to import std.conv for this to work?

From a practical point, I would also be happy with replacing the constructors with alias this, to allow implicit conversion. But this only works with one type, so e.g. if MonoTime is introduced before multiple alias this support is implemented in the compiler, that might become a problem.

[jmdavis]

Well, what bugs me about the std.conv.to dependency is that this is a Druntime module, and it depends on Phobos, a standard library. The reason why we did this split was to allow users to use other "standard libraries" like Tango with the same runtime, so is it really OK to require Phobos for everyday use of a Druntime module?

I honestly question forgoing Phobos so much that you don't even use std.conv.to (particularly since it's pretty much the way to do conversions in D), but in that case, I'd argue for just casting. And with we introduce MonoTime (which I'm increasingly convinced that we should do), the issue of whether it's ugly or not goes away anyway.

What would I put in the conversion table? std.conv.to!TickDuration(duration)? Seems overly verbose... duration.to!TickDuration() with an asterisk that mentions that you need to import std.conv for this to work?

That's fine with me. Either that or list them as casts with a note that you can use std.conv.to instead of casts if you don't want to use casts. For TickDuration -> Duration, I'd favor just adding the alias this, in which case you can use that in the table. Even if MonoTime isn't introduced, the implicit conversion from TickDuration to Duration will make dealing with TickDuration more pleasant.

From a practical point, I would also be happy with replacing the constructors with alias this, to allow implicit conversion. But this only works with one type, so e.g. if MonoTime is introduced before multiple alias this support is implemented in the compiler, that might become a problem.

I don't think that we want to introduce an alias this from Duration -> TickDuration, since you don't normally want to do that conversion, and I'm leery of having implicit conversions in both directions, but I don't think that an implicit conversion from TickDuration to Duration should pose any problems. And since MonoTime wouldn't convert to TickDuration (or vice versa), since it's a fixed point in time whereas TickDuration is as duration, I don't think that whether or not we have multiple alias thises is an issue.

[jmdavis]

Okay. I opened an enhancement request on the MonoTime issue and added it to the wiki.

https://d.puremagic.com/issues/show_bug.cgi?id=11989

And wow are those issue numbers getting high.

[CyberShadow]

I don't think that we want to introduce an alias this from Duration -> TickDuration, since you don't normally want to do that conversion,

Well, my experience shows otherwise on this point, as I mentioned above. I don't think my use case is unthinkably rare. Why do you think this conversion is so unlikely as to be an exception in practice?

Anyway, as there is no consensus I will probably remove the constructors from this pull request, and perhaps create a new one once the uniform construction syntax is implemented. However, I must insist on either undocumenting opCast or changing its documentation to point out that it is not the primary way of converting between the two types - the documentation certainly tricked me into using that method with the frustration that accompanied it.

[jmdavis]

Well, my experience shows otherwise on this point, as I mentioned above. I don't think my use case is unthinkably rare. Why do you think this conversion is so unlikely as to be an exception in practice?

I would expect the two primary cases for TickDuration would be because you need the monotonic time or because you're using benchmark (since its result is a TickDuration).

If you're using it to get the monotonic time, I'd expect that the only needs for conversion would be to convert to Duration when you've subtracted the two TickDurations that you were using for the monotonic time and want that result converted to a Duration and when you need to get a monotonic time in the future (e.g. because you want to have a timer go off at that time), in which case, you'd be adding a Duration to a TickDuration, where the Duration was the amount of time you wanted to wait and the TickDuration represented the current monotonic time.

If you're using benchmark, then I expect that you'd either convert the resultinrg TickDuration to Duration to make it more manageable, or you'd use its properties to convert it to seconds or milliseconds or whatever as an integral value.

In none of those cases is there a conversion from Duration to TickDuration (unless adding Duration to a TickDuration always results in a Duration - I can't remember, so maybe it does). So, if you have a frequent use case where you would want to convert a Duration to a TickDuration, I don't know what it is, and I'd certainly like to know about it. In either case, I wouldn't suggest adding an implicit conversion in both directions, as that would be asking for trouble IMHO.

Anyway, as there is no consensus I will probably remove the constructors from this pull request, and perhaps create a new one once the uniform construction syntax is implemented. However, I must insist on either undocumenting opCast or changing its documentation to point out that it is not the primary way of converting between the two types - the documentation certainly tricked me into using that method with the frustration that accompanied it.

Feel free to improve upon opCast's documentation. I try and write good documentation, but much of what's here is now several years old, and I'm not necessarily in tune with what other people are or aren't going to understand without further explanation. What you've added to the module documentation is certainly quite nice.

[CyberShadow]

So, if you have a frequent use case where you would want to convert a Duration to a TickDuration, I don't know what it is, and I'd certainly like to know about it.

I described it here: #711 (comment)

Edit: also, Duration + TickDuration = Duration.

[jmdavis]

I described it here: #711 (comment)

Oh sorry about that. I'd forgotten about that comment. It sounds like the problem is really the timer subsystem in that it should take a Duration for its parameters rather than a TickDuration (though it's understandable that it takes a TickDuration given the current situation). Using a TickDuration internally makes sense but it would be better if something like waitUntil took a Duration and added that to its TickDuration internally rather than requiring that you give a TickDuration.

Edit: also, Duration + TickDuration = Duration.

Yeah, that probably not so good given that you normally add a Duration to a TickDuration because you want a monotonic time that far in the future, in which case you want a TickDuration. So, the result of that operation was poorly chosen on my part, and it's obviously causing problems. From the sounds of it, the need to convert from Duration to TickDuration stems from a combination of bad API in core.time and in the timer subsystem (and the timer subsystem's API is likely the way it is at least in part because of core.time's bad API). Certainly, I think that the situation with TickDuration was my biggest mistake with core.time and std.datetime.

In any case, I'd favor adding an implicit conversion from TickDuration to Duration (which should mean that that particular opCast can be removed) and then leaving the rest of the conversions as-is. The situation will be improved with MonoTime, and think that your issues with needing to convert a Duration to a TickDuration are just further proof that we need to do something like MonoTime and phase out TickDuration.

[CyberShadow]

It sounds like the problem is really the timer subsystem in that it should take a Duration for its parameters

OK, I'm going to try changing all my timing code to use Duration and report back with results. I was thinking that at some point I'm going to run into a necessity to convert a TickDuration to Duration and then to TickDuration again, but then again, maybe not.

[CyberShadow]

(which should mean that that particular opCast can be removed)

I don't think we can do that at this point, as the opCast was explicitly documented and thus is likely to occur in user code. It would need to be put through deprecation.

[jmdavis]

I don't think we can do that at this point, as the opCast was explicitly documented and thus is likely to occur in user code. It would need to be put through deprecation.

My thinking was that if there's an alias this from TickDuration to Duration, then casting should still work (it would just use the alias this rather than the cast). If that's not true, then I think that alias this has a bug. The only code that should risk breaking AFAIK would be code that explicitly instantiated opCast - e.g. td.opCast!Duration() - rather than casting normally - e.g. cast(Duration)td - and I don't know of a good reason why anyone would ever do that.

[jmdavis]

OK, I'm going to try changing all my timing code to use Duration and report back with results. I was thinking that at some point I'm going to run into a necessity to convert a TickDuration to Duration and then to TickDuration again, but then again, maybe not.

Adding a Duration to a TickDuration when you want the result to be a TickDuration will probably force that given that fact that adding them unfortunately results in a Duration. But that would then be internal to the timer stuff and not in the caller, so it would only be in one place rather than every time the timer stuff is used. And once MonoTime is introduced, then any timer stuff which wasn't exposing the monotonic time (e.g waitUntil) could change over without requiring that the caller change at all.

[CyberShadow]

OK, I'm going to try changing all my timing code to use Duration and report back with results

OK, I've returned with some RealWorld™ results. I've actually went ahead and implemented a mock MonoTime similar to the design we discussed here, and replaced all usage of TickDuration with MonoTime & Duration. Here are my observations:

1. The good:
   • Everything seems to come together quite nicely. Having to set apart durations and time points (which previously were both represented using TickDuration) took some manual work but in the end improved the codebase, as the type now better indicates function.
   • Duration.zero and Duration.max came in handy, so I'm glad those exist. My MonoTime has max as well.
2. The bad:
   • duration.total!"msecs"() has too darn many letters and punctuation. Such a common operation really should take less effort to type and remember. But it's probably too late to fix it now, because of:
3. The ugly:
   • Duration.seconds and co. It's really easy to accidentally use these when you actually meant to use .total!"seconds". However, this is much much worse when porting your code away from TickDuration and to Duration, because (as I mentioned above) both Duration and TickDuration have a property called seconds, but it does different things! Compounded with the fact that getting a unit minus the larger unit is a rarely needed operation, I'm even more convinced that these properties need to be disappeared, ASAP (we already have .get!"unit" which does the same thing, and in fact I think it should be renamed to getOnly so it's extra-clear what any code that's calling it is doing).
4. Other things I noticed:

   • I believe MonoTime should store its internal counter in hnsecs, not system ticks. This means that MonoTime.currTime() will convert to hnsecs eagerly. The reason for that is that otherwise, the following assert may fail:

     auto start = MonoTime.currTime();
     // ...
     auto end = MonoTime.currTime();
     auto elapsed = end - start;
     auto end2 = start + elapsed;
     assert(end == end2); // fails due to precision loss from double conversion
     

     Such rounding issues could introduce subtle bugs in timing systems, so I think should be avoided.

   • Duration has a constructor which accepts a number of hnsecs. However:

     1. That constructor is private
     2. That constructor is used in Duration's documentation.

     I'm not sure how to fix this, as the example is based on having two different ways to construct a Duration object, in order to assert that they're equal.

As for this pull request: I've removed the new constructors, and simply undocumented opCast. The conversion table now has duration.to!TickDuration() and vice-versa, with to being a link to std.conv.to's doc.

[jmdavis]

I believe MonoTime should store its internal counter in hnsecs, not system ticks. This means that MonoTime.currTime() will convert to hnsecs eagerly.

The solution to that IMHO is just to not do any conversions with floating point values at all. Floating point types really have no business in time-related operations as they introduce rounding errors. If only integral operations are used, then the conversion should be consistent, and MonoTime can leave its time in system ticks, which has the side benefit of making it so that anyone who really wants ticks for some reason can get them easily.

That constructor is used in Duration's documentation.

It shouldn't be. That's a bug in the documentation. I'd have to look at it to see best how to adjust it though. I'll try and get to that later.

As for this pull request: I've removed the new constructors, and simply undocumented opCast. The conversion table now has duration.to!TickDuration() and vice-versa, with to being a link to std.conv.to's doc.

The docs at the top are good, but I think that opCast should still be documented. In general, I think that providing undocumented functionality is a bad idea, and I see no problem with opCast being documented. That documentation can suggest that you use std.conv.to rather than casting directly, but it should still be there IMHO.

[CyberShadow]

The solution to that IMHO is just to not do any conversions with floating point values at all.

Sorry if I used any terminology improperly, but I did not mean to imply floating points.

Consider:

On my system, the monotonic clock resolution is 3_515_654.
There are 10_000_000 hnsecs in a second, and that comes out as 1757827/5000000. Any way you turn it, you can't convert between the two precisely.

[CyberShadow]

The docs at the top are good, but I think that opCast should still be documented. In general, I think that providing undocumented functionality is a bad idea, and I see no problem with opCast being documented.

Well, I don't think it should be documented because we do not provide it as functionality to the user. It is a hook for std.conv and nothing else, isn't it? So I think the reason for not documenting it is the same as the reason for why nothing in the rt package is documented - everything in there is internals and compiler hooks.

[jmdavis]

There are 10_000_000 hnsecs in a second, and that comes out as 1757827/5000000. Any way you turn it, you can't convert between the two precisely.

Hmmm, I was thinking that you could get away with it, but you're right. It's bound to be off just slightly at least some of the time. I had a little bit of time this evening, and I figured that writing MonoTime would be quick since it needs almost no member functions, and when I try your example with my code (which uses no floating point values), it gets results like

before MonoTime(19389457836713) -> after            MonoTime(19389457837342)
before MonoTime(19389457836713) -> before + elapsed MonoTime(19389457837313)

I'm going to have to think about this. It seems off to me for the monotonic clock to have its internal time in hnsecs rather than the actual monotonic time, which leads me to consider having it contain both, but that opens the question of when to use hnsecs and when to use ticks. Maybe two MonoTimes would be equal if either their hnsecs or ticks were equal? I don't know. I'll have to think about it and experiment. You raise a very good point.

[jmdavis]

Well, I don't think it should be documented because we do not provide it as functionality to the user. It is a hook for std.conv and nothing else, isn't it? So I think the reason for not documenting it is the same as the reason for why nothing in the rt package is documented - everything in there is internals and compiler hooks.

The user is free to use opCast if they want to. I frequently cast rather than using std.conv.to with types that define opCast. If the user wants to use std.conv.to instead of an explicit cast, that's fine, but I don't see any reason to hide the fact that they can cast instead. Also, documenting opCast documents that the conversion exists, and while good module-level documentation is a definite boon, I think that a type's functionality should be documented on the type itself and not just in the module-level docs.

Documenting that opCast is used by std.conv.to is helpful to those who don't immediately realize that the fact that opCast was declared means that you use std.conv.to instead of casting, but if opCast isn't documented, then you're hiding the fact that the cast is possible (which I see no point in doing), and you don't have a place to document the conversion on the type except maybe in the documentation for the type itself rather than any of its functions.

[CyberShadow]

Ah, good points. I'll add them back.

[CyberShadow]

Done.

[jmdavis]

Auto-merge toggled on

[CyberShadow]

I've just realized a potential problem with my idea of converting the monotonic clock eagerly. What happens if it overflows? A pair of pre- and post-overflow values, converted to something else (like hnsecs), and subtracted one from another, are going to give wildly inaccurate results. However, I don't think this is a problem in practice on any of the monotonic clocks we use, as they have enough bits to not overflow for a long time. Monotonic clocks which might overflow exist, e.g. GetTickCount on Windows, but we aren't using them.

[jmdavis]

@CyberShadow I'm considering going for having MonoClock have a resolution that's a multiple of 10 which is at least as great as the system's monotonic clock and at least hnsecs. So, if the system's monotonic clock has a resolution which is a multiple of 10, then it's resolution will be used, whereas if it's not a multiple of 10, then the multiple of ten with one more digit will be used so that MonoClock isn't less precise than the system's monotonic clock, and by making sure that it's at least hnsecs, you don't run into a loss of precision when adding or subtracting Duration from it. In practice, I believe that that means that Linux and Mac OS X's native resolutions will be used, whereas on Windows and FreeBSD, a multiple of 10 one digit greater will be used (since IIRC the first two seem to normally go with multiples of ten, whereas the other 2 pick numbers which aren't even close to a multiple of 10).

The greatest resolution that I would expect at this point would be nanoseconds, which gives over 200 years worth of ticks. So, just so long as the monotonic clock doesn't start well into those 200 years, we should be fine. On Linux, it appears to start at 0 when the computer boots up. I don't know what other OSes do though. However, I don't see an easy way to deal with overflow if it happens. It can be done (you have to do pretty much just that with stuff like RTP sequence numbers), but you start having to use heuristics to figure out whether you've overflowed or not, and it's not something that I'd want to do if we can avoid it. But even if we were using the native resolution directly, we'd have the same overflow problem if overflow can occur, and any conversion problems that we'd have converting to hnsecs would occur whenever we subtracted two MonoTimes just like they'd occur if we converted eagerly.

[CyberShadow]

What's MonoClock? MonoTime?

Anyway, my gut says that there might still a catch somewhere with your suggestion, but I can't lay it down on paper without some code to work with (and there is still enough ambiguity with e.g. rounding and conversions that I can't implement it myself knowing that's what you meant too). So I guess let me know when you have a PoC implementation.

[jmdavis]

What's MonoClock? MonoTime?

MonoTime is the new type that we've been discussing for holding the monotonic time. I just accidentally called it MonoClock. Sorry for the confusion.

Anyway, my gut says that there might still a catch somewhere with your suggestion, but I can't lay it down on paper without some code to work with (and there is still enough ambiguity with e.g. rounding and conversions that I can't implement it myself knowing that's what you meant too). So I guess let me know when you have a PoC implementation.

Well, you get some amount of rounding error when you do the conversion from system ticks to hnsecs or nsecs or whatever the resolution for MonoTime would be. And even if MonoTime kept its time in system ticks, as soon as you deal with Duration, you still get the conversion and whatever rounding error comes with it. And even if we still had TickDuration so that the diff between two MonoTimes could be in system ticks, as soon as you convert it to seconds or nanoseconds or whatever you need to actually make it human-readable or to compare it with any duration of time that isn't generated by subtracting two MonoTimes, you still end up with the conversion problem. So, I think that we're just stuck with at least small rounding errors no matter what we do due to the fact that the system ticks aren't necessarily a multiple of 10 and therefore cleanly convertible to normal time units (such as seconds or milliseconds).

However, by doing the conversion only once (by having MonoTime keep its time in a resolution which is a multiple of 10 and only having a lossy conversion when that time is initially generated from the current system tick), I think that that will keep the rounding errors as small as they can be. And by having MonoTime's resolution by a multiple of 10, it'll interact with Duration without any rounding errors. So, there would be only one point where rounding error could creep in, and by having MonoTime's resolution be at least as precise as the system clock's resolution, we should minimize the rounding error in the one lossy conversion that we have to do.

But yeah, I'll have to finish it and see what issues other folks find with the design. I could very well be missing something.

[ghost]

Thanks for making this pull, I'm in the same boat getting confused with timing in D.

[jmdavis]

Hmm, it seems that the example

auto start = MonoTime.currTime();
// ...
auto end = MonoTime.currTime();
auto elapsed = end - start;
auto end2 = start + elapsed;
assert(end == end2); // fails due to precision loss from double conversion

is going to fail unless MonoTime has exactly hnsecs for its precision, since even if it's a multiple of 10 (e.g. nanoseconds), it's possible for endTime to not be exactly elapsed apart from start due to loss of precision when converting to hnsecs for Duration. So, if we get rid of TickDuration, either we're going to be stuck having MonoTime only have hnsec precision (which seems a bit restrictive to me given that some machines have nanosecond precision), or we have to live with end not necessarily being equal to end2. And if we do that, then we might as well just leave MonoTime in the native clock resolution.

sigh More food for thought, I guess. The basic solution here is so simple, but some of the ramifications definitely aren't. I guess that I'll have to chew on this some more. I'd kind of like to bring it up in the newsgroup for discussion, but I think that I need a definitive proposal to be critiqued for that to work well. Otherwise, I fear that there would be too much bikeshedding and too little of value in the discussion. I feel like I'm so close and yet so far. Oh, well. Back to chewing on it, I guess.

[MartinNowak]

You could make subtraction return a MonoTimeDiff which stores the difference in MonoTime units but implicitly converts to Duration via alias this.

[jmdavis]

@MartinNowak Yeah. That could be one way to go about it. However, that does move us towards having two duration types again. It wouldn't be quite the same - especially if MonoTimeDiff has almost no functions on it and essentially is just there to hold the diff value rather than trying to be fully functional like Duration or TickDuration - but we have to be careful with the balance we strike so that it has the precision and functionality we need without being too complicated or confusing like it is now. Maybe we could make MonoTimeDiff be a Voldemort type? Though that might be taking it too far, especially since you're probably going to want to at least get the number of ticks out it. Voldemort really only works if you don't need to call anything on it (just pass it around) or if it follows a generic API like ranges do.

Definitely a good suggestion, but it would have to be done in a way that gave just enough functionality to do what we need without getting overly complicated or causing confusion. The implicit conversion to Duration would help reduce the problem, but I suppose that it essentially becomes a question of how little we could get away with putting on MonoTimeDiff and still have it be useful so as to make it so that Duration is almost always what gets used by name, but it's possible to get the actual time for the diff out at greater precision that hnsecs in a friendly manner when it's actually necessary.

[MartinNowak]

Voldemort would work if you had multiple alias this, i.e. multiple implicit conversion targets, but that is far from implemented.