Bug 158723 – Add system tracing points for requestAnimationFrame() workflow

• Component: Animations
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

We need to measure how long each step takes and identify the perf issues especially for iOS.

[Said Abou-Hallawa]

Created attachment 281302 [details]
Patch

[Simon Fraser (smfr)]

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.

[Said Abou-Hallawa]

Created attachment 281309 [details]
Patch

[Simon Fraser (smfr)]

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.

[Said Abou-Hallawa]

Created attachment 281431 [details]
Patch

[Said Abou-Hallawa]

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.

[Said Abou-Hallawa]

Created attachment 281772 [details]
Patch

[Simon Fraser (smfr)]

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().

[Said Abou-Hallawa]

Created attachment 281775 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 281775 [details]
Patch

Clearing flags on attachment: 281775

Committed r202297: <http://trac.webkit.org/changeset/202297>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.