Bug 140474 - Add a way to collect scrolling performance data (viewport tile coverage) with UI-side compositing
Summary: Add a way to collect scrolling performance data (viewport tile coverage) with...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-14 15:52 PST by Simon Fraser (smfr)
Modified: 2015-01-20 10:41 PST (History)
3 users (show)

See Also:


Attachments
Patch (31.52 KB, patch)
2015-01-14 17:51 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (32.16 KB, patch)
2015-01-16 11:13 PST, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Patch (31.33 KB, patch)
2015-01-16 11:14 PST, Simon Fraser (smfr)
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2015-01-14 15:52:46 PST
Add a way to collect scrolling performance data (viewport tile coverage) with UI-side compositing
Comment 1 Simon Fraser (smfr) 2015-01-14 17:51:35 PST
Created attachment 244670 [details]
Patch
Comment 2 Csaba Osztrogonác 2015-01-15 04:03:55 PST
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.
Comment 3 Simon Fraser (smfr) 2015-01-16 11:13:59 PST
Created attachment 244774 [details]
Patch
Comment 4 Simon Fraser (smfr) 2015-01-16 11:14:48 PST
Created attachment 244775 [details]
Patch
Comment 5 Tim Horton 2015-01-16 11:36:06 PST
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?
Comment 6 Simon Fraser (smfr) 2015-01-20 10:41:02 PST
https://trac.webkit.org/r178723