Closed Bug 493756 Opened 15 years ago Closed 13 years ago

PR_Now() and Date.now() get out of sync

Categories

(Core :: JavaScript Engine, defect)

ARM
Windows Mobile 6 Professional
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Tracking Status
fennec 1.0- ---

People

(Reporter: dougt, Unassigned)

References

()

Details

I was doing some simply profiling on windows mobile using printf's and dump's which used PR_Now() and Date.now().  What I saw was that dumps of Date.now() printed out values that were about 500ms ahead of the printf's, but the log file showed that the dump's were printed to the log before the printfs.

sample console log:

(1242755056070) WM_LBUTTONDOWN start  <-- from a printf call using PR_Now()
(1242755055580) js mousedown start    <-- from a dump using Date.now()
(1242755055594) js mousedown end      <-- from a dump using Date.now()

To work around this problem, I made PRMJ_Now just return the value of PR_Now().

This might be an acceptable solution on Windows Mobile now since we do a similar calibration as JS does.  Maybe we can use PR_Now on all platforms?
PR_Now doesn't have fine enough resolution for js on all platforms.  Using PR_Now for js would be good for Windows CE though, since we're paying the initialization penalty twice as it stands.
A similar bug has been investigated about Date's timing issues, bug 472233. I
don't think NSPR's functions are accurate enough for js, I mentioned using them
but was told they are within ~15 ms or some such...
(In reply to comment #2)
> A similar bug has been investigated about Date's timing issues, bug 472233. I
> don't think NSPR's functions are accurate enough for js, I mentioned using them
> but was told they are within ~15 ms or some such...

We're using essentially the same code in js and NSPR for windows ce since the landing of bug 488621.  So, for windows ce NSPR's resolution is fine.
Don't think so... cc'ing Rob Arnold.

/be
This hasn't been a problem in the past as far as I know even though PRMJ_Now and PR_Now used different timing methods. Some clock drift is expected but 500ms is very unusual.

I do plan to change the implementation of the desktop version of PRMJ_Now to use the multimedia timers when the system is on AC. This should give more stable results, especially if we can get NSPR to accept the changes. Though the multimedia timer API gives 15ms resolution by default, we can turn up the kernel's tick rate while on AC (see bug 454660) to give 1ms resolution. This should result in no change to NSPR's current PR_Now impl for other clients while allowing us to benefit from a better resolution. After that is completed and checked in, we can switch PRMJ_Now to call PR_Now which should make both desktop and mobile happy since their PR_Now impl will still differ (mm timers are not stable on WinMo from what I hear? Perhaps someone more familiar with WinMo can confirm - this is not as well documented and I don't have any WinMo devices). And of course as a result PR_Now will agree with PRMJ_Now.

I'm not sure at this point why there are separate implementations; is it because SpiderMonkey has different performance requirements than NSPR's clients or is it because SpiderMonkey does not want to depend on NSPR (or both)?
SpiderMonkey has different perf requirements, or at the least better accuracy/perf is more important to SpiderMonkey than to NSPR clients in general.

SpiderMonkey depends on NSPR only when built in the threadsafe configuration, which is simpler for ST embedders.  Simple enough to care about if sharing an implementation were deemed desirable?  Dunno, we'd have to ask them.
Since PR_Now and  PRMJ_Now have the same basic implementations, can we just make PRMJ_Now call PR_Now now?
I still think introducing this dependency on NSPR for non-threadsafe builds of Spidermonkey isn't a good idea.
tracking-fennec: --- → ?
It would really be nice for these to be the same.
tracking-fennec: ? → 1.0-
(In reply to comment #8)
> I still think introducing this dependency on NSPR for non-threadsafe builds of
> Spidermonkey isn't a good idea.

can we have the dependency conditional on it being a threadsafe build? otherwise fall back to the existing code.
WinCE/Windows Mobile support has been removed from the main build system, Spidermonkey, mobile installer, in-app updater and so on (see bug 614720, bug 554087 and all their dependants). Until such point where MS decide to release a Windows Phone 7 NDK and the decision is made to port to that platform, this is WONTFIX.

Filter bugmail on WinCEMassWONTFIX.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.