Bug 111725 – Adding a hook to collect data for a Google UMA histogram to track when m_needsCompositedScrolling is turned on and off

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

[Glenn Hartmann]

Adding a histogram to track when m_needsCompositedScrolling is turned on and off

[Glenn Hartmann]

Created attachment 191986 [details]
Patch

[Julien Chaffraix]

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)]

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]

Created attachment 192219 [details]
Patch

[WebKit Review Bot]

Comment on attachment 192219 [details]
Patch

Clearing flags on attachment: 192219

Committed r145229: <http://trac.webkit.org/changeset/145229>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.