Bug 62162

Created attachment 96641 [details] Patch

Comment on attachment 96641 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96641&action=review > Source/WebKit/chromium/public/WebKitClient.h:270 > + virtual double monotonicallyIncreasingTime() { return 0; } i'm curious about using a double here. it seems like this clock is useful for computing time deltas (which will often be on the order of milliseconds). as the magnitude of this clock increases, won't the precision of the difference between two nearby values decrease? wouldn't it be better to use an int64_t in other words?

i think what i'm really trying to say is that it is important for the origin (0-point) of the clock to be well within the 53-bit precision of a double. making the origin be the epoch accomplishes this, but the definition of the clock doesn't make that clear. if you used int64_t, then you would be force to make that clear. maybe a double is used to avoid wrap-around issues, but instead of wrap-around issues, you have loss of precision issues, which are also problematic. using int64_t, and saying that the clock has an origin of process startup time, would probably be much clearer and less problematic.

(In reply to comment #3) > i think what i'm really trying to say is that it is important for the origin (0-point) of the clock to be well within the 53-bit precision of a double. making the origin be the epoch accomplishes this, but the definition of the clock doesn't make that clear. if you used int64_t, then you would be force to make that clear. > > maybe a double is used to avoid wrap-around issues, but instead of wrap-around issues, you have loss of precision issues, which are also problematic. using int64_t, and saying that the clock has an origin of process startup time, would probably be much clearer and less problematic. (This discussion might be better on the parent bug.) I expect all platforms will do what you've described, although maybe 0 ends up being boot time instead of process start. The main reason I avoided specifying 0 is that I didn't want everyone to have to do a bunch of math to normalize behavior across platforms when we really don't care about that. I want people to use whatever millisecond resolution monotonic clock is convenient. In practice, I expect 0 won't be any farther away than the epoch, so we should have plenty of precision to handle milliseconds. I also have a slight desire that the epoch not be used so that it's harder to confuse monotonic times with currentTime() when debugging. I did initially implement this with uint64_t for this reason, although that's far from foolproof. Can you think of some other way to spec these requirements? I felt (other) Darin's comment about using doubles was more for style and consistency reasons. I didn't think wrapping was a concern.

I think you should at least spell out the requirements for the embedder, who has to implement this new clock. I understand the stylistic reasons for preferring double. I do wonder if it wouldn't lead to mistakes, in which we confuse this clock for the wall clock because the types are the same :-(

Created attachment 97046 [details] Patch

Comment on attachment 97046 [details] Patch Clearing flags on attachment: 97046 Committed r88745: <http://trac.webkit.org/changeset/88745>

All reviewed patches have been landed. Closing bug.