Bug 61010

Summary: [chromium] Add histograms for paint times
Product: WebKit Reporter: Nat Duca <nduca>
Component: New BugsAssignee: Nat Duca <nduca>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, enne, jamesr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
typo fix.
none
Patch
none
Make patch applyable
none
i can haz commit nao? none

Description Nat Duca 2011-05-17 18:17:03 PDT
[chromium] Add histograms for paint times
Comment 1 Nat Duca 2011-05-17 18:17:29 PDT
Created attachment 93854 [details]
Patch
Comment 2 Nat Duca 2011-05-17 18:18:46 PDT
The goal here is to improve our visibility into tile paint durations in the field.
Comment 3 Dominic Cooney 2011-05-17 20:20:20 PDT
Comment on attachment 93854 [details]
Patch

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

> Source/WebKit/chromium/src/WebViewImpl.cpp:1024
> +    TRACE_EVENT("WebViewImpl::;layout", this, 0);

DBC: typo w/ extra ;?
Comment 4 Nat Duca 2011-05-17 20:45:41 PDT
Created attachment 93862 [details]
typo fix.
Comment 5 James Robinson 2011-05-18 14:18:26 PDT
Comment on attachment 93862 [details]
typo fix.

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

R=me, but you should use currentTime() or currentTimeMS() from <wtf/CurrentTime.h> instead of calling straight into PlatformBridge.

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:87
> +        double pixelsPerSec = static_cast<double>(contentRect.width() * contentRect.height()) / (paintEnd - paintStart);

nit: static_cast<double> here is redundant since the divisor is double already

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:88
> +        PlatformBridge::histogramCustomCounts("Renderer4.AccelNonRootPaintDurationMS", (paintEnd - paintStart) * 1000.0, 0, 120, 30);

nit: use 1000, or use currentTimeMS() to avoid having to do this math

> Source/WebCore/platform/graphics/chromium/ContentLayerChromium.cpp:89
> +        PlatformBridge::histogramCustomCounts("Renderer4.AccelNonRootPaintMegapixPerSecond", pixelsPerSec / 1000000.0, 10, 210, 30);

nit: use 1000000 or 1000 * 1000
Comment 6 Nat Duca 2011-05-18 15:16:25 PDT
Created attachment 93991 [details]
Patch
Comment 7 Nat Duca 2011-05-18 16:38:39 PDT
Created attachment 94000 [details]
Make patch applyable
Comment 8 James Robinson 2011-05-18 16:41:46 PDT
Comment on attachment 94000 [details]
Make patch applyable

laaaaaand
Comment 9 WebKit Commit Bot 2011-05-18 17:07:05 PDT
Comment on attachment 94000 [details]
Make patch applyable

Rejecting attachment 94000 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'land-a..." exit_code: 2

Last 500 characters of output:
s/chromium/ContentLayerChromium.cpp
	M	Source/WebCore/platform/graphics/chromium/LayerRendererChromium.h
	A	Source/WebCore/platform/graphics/chromium/LayerTextureSubImage.h
	A	Source/WebCore/platform/graphics/chromium/LayerTextureUpdaterCanvas.h
	M	Source/WebCore/platform/chromium/TraceEvent.h
	M	Source/WebCore/WebCore.gypi
r86805 = 2afa3b43989ae1a0a8f6b8998b7ed5ba92e855d5 (refs/remotes/trunk)
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/trunk.

Full output: http://queues.webkit.org/results/8707630
Comment 10 Nat Duca 2011-05-18 17:18:13 PDT
Created attachment 94007 [details]
i can haz commit nao?
Comment 11 WebKit Commit Bot 2011-05-18 20:19:45 PDT
The commit-queue encountered the following flaky tests while processing attachment 94007 [details]:

http/tests/websocket/tests/multiple-connections.html bug 53825 (author: abarth@webkit.org)
The commit-queue is continuing to process your patch.
Comment 12 WebKit Commit Bot 2011-05-18 20:21:15 PDT
Comment on attachment 94007 [details]
i can haz commit nao?

Clearing flags on attachment: 94007

Committed r86816: <http://trac.webkit.org/changeset/86816>
Comment 13 WebKit Commit Bot 2011-05-18 20:21:20 PDT
All reviewed patches have been landed.  Closing bug.