Bug 140474 – Add a way to collect scrolling performance data (viewport tile coverage) with UI-side compositing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Add a way to collect scrolling performance data (viewport tile coverage) with UI-side compositing

[Simon Fraser (smfr)]

Created attachment 244670 [details]
Patch

[Csaba Osztrogonác]

Comment on attachment 244670 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244670&action=review

> Source/WebKit2/UIProcess/WebPageProxy.cpp:58
> +#include "RemoteLayerTreeScrollingPerformanceData.h"

Please move this include to the "#if PLATFORM(COCOA)" block 
_before_ landing not to break the EFL/GTK builds. Thanks.

[Simon Fraser (smfr)]

Created attachment 244774 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 244775 [details]
Patch

[Tim Horton]

Comment on attachment 244775 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=244775&action=review

> Source/WebCore/platform/graphics/ca/TileController.cpp:45
> +    return "TileGrid Container Layer";

While this is not perf-critical code, I know there's a better way to make a String from a literal (but I don't know what it is).

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:849
> +- (WebCore::FloatRect)visibleRectInScreenCoordinates

screen? I see no screen here.

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:930
> +        scrollPerfData->didCommitLayerTree([self visibleRectInScreenCoordinates]);

dot notation?

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:1391
> +        scrollPerfData->didScroll([self visibleRectInScreenCoordinates]);

dot notation?

> Source/WebKit2/UIProcess/API/Cocoa/WKWebView.mm:2128
> +    return nil;
> +#else
> +    return nil;
> +#endif

This can be simplified.

> Source/WebKit2/UIProcess/Cocoa/RemoteLayerTreeScrollingPerformanceData.h:45
> +    NSArray* data(); // Array of [ time, event type, unfilled pixel count ]

star's on the wrong side

> Source/WebKit2/UIProcess/Cocoa/RemoteLayerTreeScrollingPerformanceData.h:50
> +    RemoteLayerTreeDrawingAreaProxy& m_drawingArea;

This interspersion of type declarations and function declarations and members is weird. Can this be sorted?

> Source/WebKit2/UIProcess/Cocoa/RemoteLayerTreeScrollingPerformanceData.mm:78
> +    NSMutableArray* dataArray = [NSMutableArray arrayWithCapacity:m_blankPixelCounts.size()];

star's on the wrong side!

> Source/WebKit2/UIProcess/Cocoa/RemoteLayerTreeScrollingPerformanceData.mm:90
> +static CALayer *findTileGridContainerLayer(CALayer *layer)

Should we keep its id instead of using names and crawling the tree?

[Simon Fraser (smfr)]

https://trac.webkit.org/r178723