Summary: | Every call to RenderObject::setAnimatableStyle() iterates through all m_compositeAnimations: potentially O(N^2) | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | Layout and Rendering | Assignee: | Igor Trindade Oliveira <igor.oliveira> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cmarrin, dino, igor.oliveira, simon.fraser, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | 80676 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2010-04-22 19:56:40 PDT
Created attachment 128366 [details]
Patch
Proposed Patch. For the worst case, this patch has the same amount of iterations of the code without the patch.
Comment on attachment 128366 [details]
Patch
Did you measure any performance gain?
Comment on attachment 128366 [details] Patch Clearing flags on attachment: 128366 Committed r108616: <http://trac.webkit.org/changeset/108616> All reviewed patches have been landed. Closing bug. Yeah, in some cases the patch reduced the number of iterations in 20%. (In reply to comment #2) > (From update of attachment 128366 [details]) > Did you measure any performance gain? This broke the animation on http://www.apple.com/safari/welcome/ and was rolled out via bug 80676. Investigating. (In reply to comment #6) > This broke the animation on http://www.apple.com/safari/welcome/ and was rolled out via bug 80676. Created attachment 132302 [details]
Test case
Safari Welcome page reduced test.
Created attachment 132744 [details]
Patch
Patch.
The bug happened because every time that we had an active timer, the patch stopped the timer and called timer.startOneShot, now we are checking if the new timer is bigger than the active timer, if not, we stop the active timer and started a new one shot timer. (In reply to comment #6) > This broke the animation on http://www.apple.com/safari/welcome/ and was rolled out via bug 80676. Comment on attachment 132744 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132744&action=review > LayoutTests/animations/animation-welcome-safari.html:124 > + document.body.className = "go" semicolon missing > Source/WebCore/ChangeLog:7 > + This patchs implements updateAnimationTimerForRenderer, it just checks the timeToNextService for > + the current RenderObject reducing the amount of iterations. Typo "patchs". I also suggest: Implement updateAnimationTimerForRender. This reduces unnecessary animation steps on the current RenderObject by checking the value of timeToNextService before starting a new timer. committed r111639: http://trac.webkit.org/changeset/111639 This caused bug 85929. |