[webkit-reviews] review granted: [Bug 61010] [chromium] Add histograms for paint times : [Attachment 93862] typo fix.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 18 14:18:26 PDT 2011


James Robinson <jamesr at chromium.org> has granted Nat Duca
<nduca at chromium.org>'s request for review:
Bug 61010: [chromium] Add histograms for paint times
https://bugs.webkit.org/show_bug.cgi?id=61010

Attachment 93862: typo fix.
https://bugs.webkit.org/attachment.cgi?id=93862&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
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.AccelNonRootPaintMegapixPerSec
ond", pixelsPerSec / 1000000.0, 10, 210, 30);

nit: use 1000000 or 1000 * 1000


More information about the webkit-reviews mailing list