[webkit-dev] RFC: stop using std::chrono, go back to using doubles for time
Filip Pizło
fpizlo at apple.com
Mon May 23 07:24:34 PDT 2016
-Filip
> On May 23, 2016, at 12:12 AM, Michal Debski <m.debski at samsung.com> wrote:
>
> Please this example: http://ideone.com/c640Xd
>
I'm not going to write "WTF::now<std::chrono::system_clock>()" every time I want the time. That's awful!
One of the problems with chrono is the use of awkward templates throughout the API. Your solution makes this worse.
> From cppreference:
>
> "(time_point) is implemented as if it stores a value of typeDuration indicating the time interval from the start of the Clock's epoch."
>
> "If Rep is floating point, then the duration can represent fractions of ticks."
>
> http://en.cppreference.com/w/cpp/chrono/time_point
>
> http://en.cppreference.com/w/cpp/chrono/duration
>
>
>
> time_point still remembers which clock it belongs to, see the specialization now() returns.
>
> The only problem I see is that conversion from fractional duration with Infinite value to integer duration is undefined. But I assume it would be forbiden to use integer based chrono altogether.
>
That seems like a tough thing to guarantee.
> As for the last point I don't understand what do you mean by non-canonical.
>
According to Merriam-Webster, canonical means: conforming to a general rule or acceptable procedure. I don't think that your approach is the normal, generally accepted way to use std::chrono.
> It uses the API as intended, why would c++ expose the templates if not for specializing the behaviour? Also move operator is not canonical in WTF strictly speaking.
>
>
>
> I just remember about ugly bugs in MediaTime arithmetics.
>
>
>
> Best regards,
>
> Michal Debski
>
>
>
> ------- Original Message -------
>
> Sender : Filip Pizlo<fpizlo at apple.com>
>
> Date : May 23, 2016 07:32 (GMT+01:00)
>
> Title : Re: [webkit-dev] RFC: stop using std::chrono, go back to using doubles for time
>
>
>
>
>
>> On May 22, 2016, at 10:46 PM, Michal Debski <m.debski at samsung.com> wrote:
>>
>> Hi,
>>
>>
>>
>> sorry but this really bugs me. Isn't this enough?
>>
>>
>>
>> namespace WTF {
>>
>> using nanoseconds = std::chrono::duration<double, std::nano>;
>> using microseconds = std::chrono::duration<double, std::micro>;
>> using milliseconds = std::chrono::duration<double, std::milli>;
>> using seconds = std::chrono::duration<double>;
>> using minutes = std::chrono::duration<double, std::ratio<60>>;
>> using hours = std::chrono::duration<double, std::ratio<3600>>;
>>
>>
>> template <class Clock>
>> std::chrono::time_point<Clock, seconds> now()
>> {
>> return Clock::now();
>> }
>>
> Can you clarify how this returns fractional seconds?
>
> Note that your code snippets are not enough to cover WebKit's use of clocks. For example we would need wall clock and monotonic clock classes with time_point types. If we have to significantly customize std::chrono and forbid some but not all of its API, then probably the total amount of code to do this would be comparable to writing our own Seconds/WallTime/MonotonicTime classes.
>>
>> }
>>
>>
>>
>> and forbid using std::chrono::clock::now()/durations with check-style. It seems like the best of both worlds.
>>
> It's an interesting perspective. But I would find it confusing if we used a non-canonical kind of std::chrono. And I'm not convinced that the changes required to make this work are as simple as what you say.
>> Oh and the infinity:
>>
>>
>>
>> namespace std {
>> namespace chrono {
>>
>> template<>
>> struct duration_values<double> {
>> static constexpr double min() { return -std::numeric_limits<double>::infinity(); }
>> static constexpr double zero() { return .0; }
>> static constexpr double max() { return std::numeric_limits<double>::infinity(); }
>> };
>>
>> }
>> }
>>
>> Best regards,
>> Michal Debski
>>
>>
>>
>> ------- Original Message -------
>>
>> Sender : Filip Pizlo<fpizlo at apple.com>
>>
>> Date : May 23, 2016 02:41 (GMT+01:00)
>>
>> Title : [webkit-dev] RFC: stop using std::chrono, go back to using doubles for time
>>
>>
>>
>> Hi everyone!
>>
>> I’d like us to stop using std::chrono and go back to using doubles for time. First I list the things that I think we wanted to get from std::chrono - the reasons why we started switching to it in the first place. Then I list some disadvantages of std::chrono that we've seen from fixing std::chrono-based code. Finally I propose some options for how to use doubles for time.
>>
>> Why we switched to std::chrono
>>
>> A year ago we started using std::chrono for measuring time. std::chrono has a rich typesystem for expressing many different kinds of time. For example, you can distinguish between an absolute point in time and a relative time. And you can distinguish between different units, like nanoseconds, milliseconds, etc.
>>
>> Before this, we used doubles for time. std::chrono’s advantages over doubles are:
>>
>> Easy to remember what unit is used: We sometimes used doubles for milliseconds and sometimes for seconds. std::chrono prevents you from getting the two confused.
>>
>> Easy to remember what kind of clock is used: We sometimes use the monotonic clock and sometimes the wall clock (aka the real time clock). Bad things would happen if we passed a time measured using the monotonic clock to functions that expected time measured using the wall clock, and vice-versa. I know that I’ve made this mistake in the past, and it can be painful to debug.
>>
>> In short, std::chrono uses compile-time type checking to catch some bugs.
>>
>> Disadvantages of using std::chrono
>>
>> We’ve seen some problems with std::chrono, and I think that the problems outweigh the advantages. std::chrono suffers from a heavily templatized API that results in template creep in our own internal APIs. std::chrono’s default of integers without overflow protection means that math involving std::chrono is inherently more dangerous than math involving double. This is particularly bad when we use time to speak about timeouts.
>>
>> Too many templates: std::chrono uses templates heavily. It’s overkill for measuring time. This leads to verbosity and template creep throughout common algorithms that take time as an argument. For example if we use doubles, a method for sleeping for a second might look like sleepForSeconds(double). This works even if someone wants to sleep for a nanoseconds, since 0.000001 is easy to represent using a double. Also, multiplying or dividing a double by a small constant factor (1,000,000,000 is small by double standards) is virtually guaranteed to avoid any loss of precision. But as soon as such a utility gets std::chronified, it becomes a template. This is because you cannot have sleepFor(std::chrono::seconds), since that wouldn’t allow you to represent fractions of seconds. This brings me to my next point.
>>
>> Overflow danger: std::chrono is based on integers and its math methods do not support overflow protection. This has led to serious bugs like https://bugs.webkit.org/show_bug.cgi?id=157924. This cancels out the “remember what unit is used” benefit cited above. It’s true that I know what type of time I have, but as soon as I duration_cast it to another unit, I may overflow. The type system does not help! This is insane: std::chrono requires you to do more work when writing multi-unit code, so that you satisfy the type checker, but you still have to be just as paranoid around multi-unit scenarios. Forgetting that you have milliseconds and using it as seconds is trivially fixable. But if std::chrono flags such an error and you fix it with a duration_cast (as any std::chrono tutorial will tell you to do), you’ve just introduced an unchecked overflow and such unchecked overflows are known to cause bugs that manifest as pages not working correctly.
>>
>> I think that doubles are better than std::chrono in multi-unit scenarios. It may be possible to have std::chrono work with doubles, but this probably implies us writing our own clocks. std::chrono’s default clocks use integers, not doubles. It also may be possible to teach std::chrono to do overflow protection, but that would make me so sad since using double means not having to worry about overflow at all.
>>
>> The overflow issue is interesting because of its implications for how we do timeouts. The way to have a method with an optional timeout is to do one of these things:
>>
>> - Use 0 to mean no timeout.
>> - Have one function for timeout and one for no timeout.
>> - Have some form of +Inf or INT_MAX to mean no timeout. This makes so much mathematical sense.
>>
>> WebKit takes the +Inf/INT_MAX approach. I like this approach the best because it makes the most mathematical sense: not giving a timeout is exactly like asking for a timeout at time-like infinity. When used with doubles, this Just Works. +Inf is greater than any value and it gets preserved properly in math (+Inf * real = +Inf, so it survives gracefully in unit conversions; +Inf + real = +Inf, so it also survives absolute-to-relative conversions).
>>
>> But this doesn’t work with std::chrono. The closest thing to +Inf is duration::max(), i.e. some kind of UINT_MAX, but this is guaranteed to overflow anytime it’s converted to a more precise unit of time (nanoseconds::max() converted to milliseconds is something bogus). It appears that std::chrono doesn’t have a good story for infinite timeout, which means that anyone writing a function that can optionally have a timeout is going to have a bad time. We have plenty of such functions in WebKit. For example, I’m not sure how to come up with a feel-good solution to https://bugs.webkit.org/show_bug.cgi?id=157937 so long as we use std::chrono.
>>
>> Going back to doubles
>>
>> Considering these facts, I propose that we switch back to using doubles for time. We can either simply revert to the way we used doubles before, or we can come up with some more sophisticated approach that blends the best of both worlds. I prefer plain doubles because I love simplicity.
>>
>> Simply revert to our old ways: I like this approach the best because it involves only very simple changes. Prior to std::chrono, we used a double to measure time in seconds. It was understood that seconds was the default unit. We would use both monotonic and wall clocks, and we used double for both of them.
>>
>> Come up with a new type system: Having learned from std::chrono and doubles, it seems that the best typesystem for time would comprise three classes: Seconds, WallTime, and MonotonicTime. Seconds would be a class that holds a double and supports +/+=/-/-=/</<=/>/>=/==/!= operations, as well as conversions to a raw double for when you really need it. WallTime and MonotonicTime would be wrappers for Seconds with a more limited set of available operations. You can convert WallTime or MonotonicTime to Seconds and vice-versa, but some operators are overloaded to make casts unnecessary in most cases (WallTime + Seconds = WallTime, WallTime - WallTime = Seconds, etc). This would save us from forgetting the unit or the clock. The name of the Seconds class is a dead give-away, and WallTime and MonotonicTime will not yield you a value that is unit-sensitive unless you say something like WallTime::toSeconds(). There will be no easy way to convert WallTime to MonotonicTime and vice-versa, since we want to discourage such conversions.
>>
>> Personally I feel very comfortable with doubles for time. I like to put the word “Seconds” into variable names and function names (waitForSeconds(double) is a dead give-away). On the other hand, I sort of like the idea of a type system to protect clock mix-ups. I think that’s the biggest benefit we got from std::chrono.
>>
>> If it was entirely up to me, I’d go for doubles. I think that there needs to be a high burden of proof for using types to catch semantic bugs. A type system *will* slow you down when writing code, so the EV (expected value) of the time savings from bugs caught early needs to be greater than the EV of the time lost due to spoonfeeding the compiler or having to remember how to use those classes. Although I know that using doubles sometimes meant we had bugs, I don’t think they were frequent or severe enough for the odds to be good for the Seconds/WallTime/MonotonicTime solution.
>>
>> Thoughts?
>>
>> -Filip
>>
>>
>>
>>
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev at lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-dev/attachments/20160523/f652706f/attachment.html>
More information about the webkit-dev
mailing list