RESOLVED FIXED 111725
Adding a hook to collect data for a Google UMA histogram to track when m_needsCompositedScrolling is turned on and off
https://bugs.webkit.org/show_bug.cgi?id=111725
Summary Adding a hook to collect data for a Google UMA histogram to track when m_need...
Glenn Hartmann
Reported 2013-03-07 06:34:39 PST
Adding a histogram to track when m_needsCompositedScrolling is turned on and off
Attachments
Patch (1.67 KB, patch)
2013-03-07 06:35 PST, Glenn Hartmann
no flags
Patch (3.25 KB, patch)
2013-03-08 06:52 PST, Glenn Hartmann
no flags
Glenn Hartmann
Comment 1 2013-03-07 06:35:14 PST
Julien Chaffraix
Comment 2 2013-03-07 14:06:18 PST
Comment on attachment 191986 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191986&action=review > Source/WebCore/rendering/RenderLayer.cpp:2018 > + if (acceleratedCompositingForOverflowScrollEnabled()) > + HistogramSupport::histogramEnumeration("Renderer.NeedsCompositedScrolling", m_needsCompositedScrolling, 2); It would have been nice to explain *what* you are trying to measure as well as *why*. Without the context, it's difficult to know if this is correct or why we can't other some other mechanism like FeatureObserver. After talking with Glenn over IM, they want to measure the frequency of the feature at the layer level as a rough estimate of what could be done. They understand that this number will not be actionable but that giving the reasons for not having composited scrolling are expensive to compute and we want the overhead minimal. All in all, the code change make sense but please add the relevant bits to the ChangeLog or as a comment in the code (remember that WebKit prefers 'why' comments over 'what' comments though).
Simon Fraser (smfr)
Comment 3 2013-03-07 14:16:20 PST
Out of context, I've no idea what it means to "add a Histogram". How do I get to see the histogram? I think what you really mean is "add some instrumentation so that Google can collect data, which might be displayed in a histogram at some point".
Glenn Hartmann
Comment 4 2013-03-08 06:52:50 PST
WebKit Review Bot
Comment 5 2013-03-08 08:30:41 PST
Comment on attachment 192219 [details] Patch Clearing flags on attachment: 192219 Committed r145229: <http://trac.webkit.org/changeset/145229>
WebKit Review Bot
Comment 6 2013-03-08 08:30:44 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.