Bug 119761 - Replace currentTime() with monotonicallyIncreasingTime() in all possible places
Summary: Replace currentTime() with monotonicallyIncreasingTime() in all possible places
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P3 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 119762 119785 119958
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-13 11:12 PDT by Arunprasad Rajkumar
Modified: 2013-08-17 12:53 PDT (History)
11 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Arunprasad Rajkumar 2013-08-13 11:12:29 PDT
It is a meta-bug to track. Changes will be done in incremental phases,

- Initially WTF & JavaScriptCore
- WebCore
Comment 1 Ryosuke Niwa 2013-08-13 11:43:42 PDT
What is the goal of this change?  Why do we want to do this?
Comment 2 Arunprasad Rajkumar 2013-08-13 12:11:24 PDT
(In reply to comment #1)
> What is the goal of this change?  Why do we want to do this?

Actually some platforms(mostly embedded systems like TV/STB) doesn't have battery powered reliable clock source. Wall Clock time is obtained via time sources like NTP(it may have some latency). So I feel use currentTime() only on the places which really needs wall clock time(like jsDateTime()), remaining places monotonicallyIncreasingTime() is good.
Comment 3 Darin Adler 2013-08-13 12:13:27 PDT
We should think this through. I think we should use a shorter name for the time we use “almost everywhere”.
Comment 4 Ryosuke Niwa 2013-08-13 12:16:30 PDT
(In reply to comment #2)
> (In reply to comment #1)
> > What is the goal of this change?  Why do we want to do this?
> 
> Actually some platforms(mostly embedded systems like TV/STB) doesn't have battery powered reliable clock source. Wall Clock time is obtained via time sources like NTP(it may have some latency). So I feel use currentTime() only on the places which really needs wall clock time(like jsDateTime()), remaining places monotonicallyIncreasingTime() is good.

I'm still not sure replacing calls to currentTime() by monotonicallyIncreasingTime() everywhere is such a great idea. Can't those platforms emulate currentTime?  Presumably, this is just an optimization, right?
Comment 5 Anders Carlsson 2013-08-13 12:29:35 PDT
(In reply to comment #3)
> We should think this through. I think we should use a shorter name for the time we use “almost everywhere”.

C++11 calls this the “steady clock” - http://en.cppreference.com/w/cpp/chrono/steady_clock
Comment 6 Arunprasad Rajkumar 2013-08-13 12:32:41 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > What is the goal of this change?  Why do we want to do this?
> > 
> 
> I'm still not sure replacing calls to currentTime() by monotonicallyIncreasingTime() everywhere is such a great idea. Can't those platforms emulate currentTime?  

Not just blindly everywhere. I think wall clock time is needed only in some places like jsDateTime, HTTP Resource Caching,..

>Presumably, this is just an optimization, right?

Yes, it is a kind of optimization. The issue is some of the platform wall clock timer's accuracy is about 10ms(what we using), but monotonicallyIncreasingTime's accuracy is < 1ms.
Comment 7 Alexey Proskuryakov 2013-08-15 10:04:50 PDT
It almost feels like I'm missing something given the above discussion, but I think that the importance of using monotonicallyIncreasingTime for things like timeouts is that currentTime() can go back, e.g. for DST changes or NTP adjustments.

You don't want your 100 ms timer to take 1 hour + 100 ms at the moment when daylight savings changes happen.

Clock time is also more likely to jump forward under normal use conditions, for the same reasons. Of course, your process can be suspended and miss the exact time even when using monotonicallyIncreasingTime.

Also, I think that "monotonically increasing" may not be precise enough - some monotonically increasing time functions are unrelated to actual time, e.g. they could return raw number of CPU ticks since boot, which of course depends on constantly changing CPU clock speed. But I don't think that naming discussion should block fixing the behavior.
Comment 8 Arunprasad Rajkumar 2013-08-17 11:26:59 PDT
(In reply to comment #7)
> It almost feels like I'm missing something given the above discussion, but I think that the importance of using monotonicallyIncreasingTime for things like timeouts is that currentTime() can go back, e.g. for DST changes or NTP adjustments.

currentTime() returns UTC time(atleast in linux currentTime uses gettimeofday, which return UTC time), I think only localTime() is prone to DST adjustments. I agree that NTP correction is a problem here.

> 
> You don't want your 100 ms timer to take 1 hour + 100 ms at the moment when daylight savings changes happen.
> 
> Clock time is also more likely to jump forward under normal use conditions, for the same reasons. Of course, your process can be suspended and miss the exact time even when using monotonicallyIncreasingTime.
> 
> Also, I think that "monotonically increasing" may not be precise enough - some monotonically increasing time functions are unrelated to actual time, e.g. they could return raw number of CPU ticks since boot, which of course depends on constantly changing CPU clock speed. But I don't think that naming discussion should block fixing the behavior.
Comment 9 Darin Adler 2013-08-17 12:23:06 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > It almost feels like I'm missing something given the above discussion, but I think that the importance of using monotonicallyIncreasingTime for things like timeouts is that currentTime() can go back, e.g. for DST changes or NTP adjustments.
> 
> currentTime() returns UTC time(atleast in linux currentTime uses gettimeofday, which return UTC time), I think only localTime() is prone to DST adjustments. I agree that NTP correction is a problem here.

Yes, that’s correct. NTP correction and the end user changing the clock manually are both issues.