Closed Bug 1435043 Opened 7 years ago Closed 7 years ago

Cut over DOM Timing to use DOMTimeMilliSec instead of TimeStamp

Categories

(Core :: DOM: Core & HTML, enhancement, P2)

enhancement

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox60 --- affected

People

(Reporter: tjr, Assigned: tjr)

References

Details

Attachments

(7 files)

IF we want to jitter all of the Performance APIs AND we want to prevent time from going backwards, I believe we need to perform the jittering at the point where time is recorded. That way we can say "This time is now, it is not in the past - ensure it is not less than a timestamp you have previously jittered." However, in nsDOMNavigationTiming we store all the members as TimeStamps, which are a unit-less type. To jitter them, we need them in seconds, ms, or us. While nsDOMNavigationTiming stores them as TimeStamps, it only exposes them as either DOMTimeMilliSec (whole milliseconds since the epoch) or DOMHighResTimeStamp (milliseconds (with decimals) since navigation start. This patchset cuts over the internal storage of the times to DOMHighResTimeStamp. It doesn't change how they get exposed to any callers, so it should, in theory, have zero actual effect on anything. I'm submitting the patches as a large number of smaller changes. Each change is much easier to review in isolation that way, because it only renames a member, or cuts over a few trivial members, or cuts over one more complicated member, or so on. I'm submitting the patches up for 'pre-review' I suppose, just to let you know where I'm going with jittering, but I'm not going to try and land these until I have a complete story in place for jittering and it seems to work. Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad4fec2c3033e24a2e3d424ec6cd07d2df5658bb
Forgot a const: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aeb5dd559dd9c342ea54dde79d374d2cd4b8cb7f I looked into this approach for Navigation Timing... In a lot of places they come from Necko. Not sure if this is going to be feasible there.
(In reply to Tom Ritter [:tjr] from comment #0) > This patchset cuts over the internal storage of the times to > DOMHighResTimeStamp. It doesn't change how they get exposed to any callers, > so it should, in theory, have zero actual effect on anything. Does this change the source? We've had power consumption issues in the past caused by the use of native high precision time sources.
(In reply to Jim Mathies [:jimm] from comment #19) > (In reply to Tom Ritter [:tjr] from comment #0) > > This patchset cuts over the internal storage of the times to > > DOMHighResTimeStamp. It doesn't change how they get exposed to any callers, > > so it should, in theory, have zero actual effect on anything. > > Does this change the source? We've had power consumption issues in the past > caused by the use of native high precision time sources. That's nasty. I assume you mean the source of time? In nsDocument I changed one mozilla::TimeStamp::Now() to (double)PR_Now() / PR_USEC_PER_MSEC - all the other locations still use TimeStamp::Now() but now immediately convert it into milliseconds and call the Reduction function. Also I worked out the last kink, and rolled the change up; successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8eebd554525747a89268a6c545d6c0e5a65f9dfd
Priority: -- → P2
We're not taking this approach.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: