New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 803956 link

Starred by 2 users
Status: Fixed
Owner:
npm@chromium.org
Closed: May 2018
Cc:
█ bmcquade@chromium.org
panicker@chromium.org
igrigo...@chromium.org
csharrison@chromium.org
charliea@chromium.org
oysteine@chromium.org
tdres...@chromium.org
yoavweiss@chromium.org
npm@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

PerformanceTiming should use TimeTicks instead of doubles

Project Member Reported by npm@chromium.org, Jan 19 2018 
Currently, the PerformanceTiming class has a bunch of methods that do the following:
* Obtain a double from another class, which represents a timestamp in seconds.
* Transform using MonotonicTimeToPseudoWallTime.
* Clamp this timestamp using the TimeClamper, for security purposes. This performs one division. Also, subtract the clamped timestamp corresponding to the timeOrigin of the object.
* Multiply the double by 1000 and convert to unsigned long long, which is the format expected by the IDLs.

Ideally we would use TimeTicks instead of double in much of these computations. TimeTicks seems to store the value in int64_t. Is the change from double to TimeTicks feasible?

Comment 1 by tdres...@chromium.org, Jan 19 2018

Cc: igrigo...@chromium.org panicker@chromium.org bmcquade@chromium.org
+ some folks who might know why we use doubles. Is it just historical?

This might bleed into PageLoadMetrics. Bryan, any idea why we don't use TimeTicks / TimeDeltas there?

Comment 2 by bmcquade@chromium.org, Jan 19 2018

Cc: oysteine@chromium.org csharrison@chromium.org charliea@chromium.org
I believe it's historical. There was a time when we couldn't use base::TimeTicks from blink. If that's changed, it would be ideal to migrate away from doubles here.

Adding charliea, oysteine, and csharrison who may know of additional reasons why we should or should not make this change, or of subtleties to keep in mind while making the change.

Comment 3 by npm@chromium.org, Jan 19 2018 

We cannot use base directly but we do have a wrapper for base::TimeTicks that can be used in core:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/wtf/Time.h

Comment 4 by csharrison@chromium.org, Jan 19 2018 

I don't think base::TimeTicks can be used anywhere except platform/, but we could use WTF::TimeTicks and convert to base::TimeTicks in //content.

I don't see any reason not to do this, though there may be subtleties around the clamping.

Comment 5 by tdres...@chromium.org, Jan 22 2018 

Migrating to WTF::TimeTicks SGTM.
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91c1b85524d974f3b9ca143f4da5695b5448f7a8

commit 91c1b85524d974f3b9ca143f4da5695b5448f7a8
Author: Nicolas Pena <npm@chromium.org>
Date: Thu Jan 25 15:56:24 2018

Use TimeTicks in PerformanceTiming

This CL does the following to increase WTF::TimeTicks usage:
* Change the monotonic times in DocumentLoadTiming to TimeTicks. This change
propagates to other Timing classes, and conversions to double are used to stop
the propagation outside of timing related classes.

Bug:  803956 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ie718f6f1996008bd8cdb20b2a7254595e0b018d9
Reviewed-on: https://chromium-review.googlesource.com/881602
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531906}
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/animation/DocumentTimeline.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/css/CSSTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/DOMHighResTimeStamp.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/Document.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/DocumentParserTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/DocumentParserTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/DocumentTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/DocumentTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/ScriptedAnimationController.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/dom/events/Event.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/exported/WebDocumentLoaderImpl.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/exported/WebFrameTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/exported/WebViewImpl.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/frame/PerformanceMonitor.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/frame/PerformanceMonitor.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/html/ImageDocument.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/inspector/InspectorPerformanceAgent.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/inspector/InspectorPerformanceAgent.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/inspector/InspectorTraceEvents.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/DocumentLoadTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/DocumentLoadTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/DocumentLoadTimingTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/DocumentLoader.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/FrameFetchContext.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/IdlenessDetector.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/IdlenessDetector.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/IdlenessDetectorTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/InteractiveDetector.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/InteractiveDetector.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/InteractiveDetectorTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/loader/NavigationScheduler.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetector.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/paint/FirstMeaningfulPaintDetectorTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/paint/PaintTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/paint/PaintTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/testing/Internals.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/Performance.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceBase.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceBase.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceBaseTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceNavigationTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceNavigationTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceObserverTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceResourceTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceResourceTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/PerformanceTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/SubTaskAttribution.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/SubTaskAttribution.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/core/timing/WorkerPerformance.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/modules/sensor/Sensor.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/modules/serviceworkers/FetchEvent.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/modules/vr/VRDisplay.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/modules/webaudio/AudioContext.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/modules/webmidi/MIDIAccess.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/LongTaskDetector.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/LongTaskDetector.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/LongTaskDetectorTest.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/exported/WebURLLoadTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/loader/fetch/ResourceLoadTiming.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/loader/fetch/ResourceLoadTiming.h
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/probe/PlatformProbes.cpp
[modify] https://crrev.com/91c1b85524d974f3b9ca143f4da5695b5448f7a8/third_party/WebKit/Source/platform/probe/PlatformProbes.h

Comment 7 by npm@chromium.org, May 3 2018

Owner: npm@chromium.org
Status: Fixed (was: Available)
►

Sign in to add a comment