Summary: | Add system tracing points for requestAnimationFrame() workflow | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||
Component: | Animations | Assignee: | Said Abou-Hallawa <sabouhallawa> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, commit-queue, dino, esprehn+autocc, kangil.han, simon.fraser, thorton | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Said Abou-Hallawa
2016-06-13 20:03:21 PDT
Created attachment 281302 [details]
Patch
Comment on attachment 281302 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281302&action=review Too much logging left in this patch! > Source/WebCore/dom/ScriptedAnimationController.cpp:147 > + WTFLogAlways("ScriptedAnimationController::serviceScriptedAnimations at %f", monotonicTimeNow); You don't want to commit this logging. > Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp:95 > + WTFLogAlways("DisplayRefreshMonitor::displayDidRefresh()"); No logging here. Created attachment 281309 [details]
Patch
Comment on attachment 281309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281309&action=review > Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:41 > No blank line here. > Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:396 > + TraceScope(RAFUIDidRefreshDisplayStart, RAFUIDidRefreshDisplayEnd); Odd that you don't name the variable here. > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:429 > + TraceScope tracingScope(RAFWK2LayerFlushStart, RAFWK2LayerFlushEnd); This one... > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:494 > + TraceScope tracingScope(RAFWK2BackingStoreFlushStart, RAFWK2BackingStoreFlushEnd); ... and this basically track the same scope. You should remove one of them. Created attachment 281431 [details]
Patch
Comment on attachment 281309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281309&action=review >> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:41 >> > > No blank line here. The blank line is removed. >> Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.mm:396 >> + TraceScope(RAFUIDidRefreshDisplayStart, RAFUIDidRefreshDisplayEnd); > > Odd that you don't name the variable here. A variable is added to keep the scope till the end of the function. >> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:429 >> + TraceScope tracingScope(RAFWK2LayerFlushStart, RAFWK2LayerFlushEnd); > > This one... This trace point was removed. Created attachment 281772 [details]
Patch
Comment on attachment 281772 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281772&action=review > Source/WTF/wtf/SystemTracing.h:82 > +inline void WTFTraceCodeRecord(TracePointCode code) I don't think this needs the WTF prefix, since it's in the WTF namespace. Let's call it TracePoint(). Created attachment 281775 [details]
Patch
Comment on attachment 281775 [details] Patch Clearing flags on attachment: 281775 Committed r202297: <http://trac.webkit.org/changeset/202297> All reviewed patches have been landed. Closing bug. |