Bug 38025 – Every call to RenderObject::setAnimatableStyle() iterates through all m_compositeAnimations: potentially O(N^2)

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

RenderObject::setAnimatableStyle() calls animation()->updateAnimations(). This in turn calls m_data->updateAnimationTimer(), which loops through all m_compositeAnimations().

This means that for every RenderObject doing any kind of transition or keyframe animation, we iterate through all other CompositeAnimations in the page. This seems bad.

[Igor Trindade Oliveira]

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.

[Simon Fraser (smfr)]

Comment on attachment 128366 [details]
Patch

Did you measure any performance gain?

[WebKit Review Bot]

Comment on attachment 128366 [details]
Patch

Clearing flags on attachment: 128366

Committed r108616: <http://trac.webkit.org/changeset/108616>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Igor Trindade Oliveira]

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?

[Simon Fraser (smfr)]

This broke the animation on http://www.apple.com/safari/welcome/ and was rolled out via bug 80676.

[Igor Trindade Oliveira]

Investigating.

(In reply to comment #6)
> This broke the animation on http://www.apple.com/safari/welcome/ and was rolled out via bug 80676.

[Igor Trindade Oliveira]

Created attachment 132302 [details]
Test case

Safari Welcome page reduced test.

[Igor Trindade Oliveira]

Created attachment 132744 [details]
Patch

Patch.

[Igor Trindade Oliveira]

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.

[Dean Jackson]

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.

[Igor Trindade Oliveira]

committed r111639: http://trac.webkit.org/changeset/111639

[Simon Fraser (smfr)]

This caused bug 85929.