[Webkit-unassigned] [Bug 111725] Adding a histogram to track when m_needsCompositedScrolling is turned on and off

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 7 14:03:53 PST 2013


https://bugs.webkit.org/show_bug.cgi?id=111725


Julien Chaffraix <jchaffraix at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #191986|review?                     |review+, commit-queue-
               Flag|                            |




--- Comment #2 from Julien Chaffraix <jchaffraix at webkit.org>  2013-03-07 14:06:18 PST ---
(From update of attachment 191986)
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).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list