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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
WONTFIX
Tracking | Status | |
---|---|---|
firefox60 | --- | affected |
People
(Reporter: tjr, Assigned: tjr)
References
Details
Attachments
(7 files)
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details | |
(deleted),
text/x-review-board-request
|
Details |
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
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
Straightforward mistake, new try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b8b8c0c4df0ba150570f7fa8dacf05c8f551ec7f
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 15•7 years ago
|
||
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.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 19•7 years ago
|
||
(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.
Assignee | ||
Comment 20•7 years ago
|
||
(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
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Comment 21•7 years ago
|
||
We're not taking this approach.
Assignee | ||
Updated•7 years ago
|
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•