Bug 228833

Summary: REGRESSION (r268932): CPU usage higher than expected with sibling elements running WebAnimations
Product: WebKit Reporter: Liam DeBeasi <ldebeasi>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, graouts, graouts, koivisto, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=218080
https://bugs.webkit.org/show_bug.cgi?id=232703
Attachments:
Description Flags
Code reproduction
none
iOS 14.5 timeline
none
iOS 14.4 Timeline
none
Patch koivisto: review+, ews-feeder: commit-queue-

Description Liam DeBeasi 2021-08-05 09:33:27 PDT
Created attachment 434994 [details]
Code reproduction

When using Web Animations, WebKit continually invalidates + recalculates style and paints even when using only transform or opacity.

I can reproduce this issue in iOS 14.5+ and cannot reproduce this issue in iOS 14.4 and older which is why I think this is a performance regression.

Steps to reproduce:

1. Open code reproduction on an iPhone running iOS 14.5 or newer. You should see two blue circles translating up the page. This animation will run continuously.
2. Open Dev Tools and take a Timeline recording. You should notice that WebKit is continually painting and invalidating/recalculating styles. You should also notice that both the CPU usage and energy impact are moderate to high.
4. Repeat steps 1-2 on a device running iOS 14.4. You should notice that WebKit does not continually paint and invalidate/recalculate styles. You should also notice that both the CPU usage and energy impact are low.

Expected Behavior:

I would expect that WebKit does not paint and invalidate/recalculate styles on a running animation that can be hardware accelerated.

Actual Behavior:

WebKit paints and invalidates/recalculates styles on a running animation that can be hardware accelerated.

Other Information:

- This has significant performance implications for any website making use of animations in Safari/WebKit as the main thread is constantly performing work. Additionally, this increases the strain on the battery resulting in the battery getting drained faster.
- This does not reproduce in Chrome or Firefox.
Comment 1 Liam DeBeasi 2021-08-05 09:40:16 PDT
Created attachment 434995 [details]
iOS 14.5 timeline

Added Safari Timeline for code reproduction on iOS 14.5
Comment 2 Liam DeBeasi 2021-08-05 09:40:52 PDT
Created attachment 434996 [details]
iOS 14.4 Timeline

Added Safari Timeline for code reproduction on iOS 14.4
Comment 3 Simon Fraser (smfr) 2021-08-05 10:28:18 PDT
I can confirm. Also on macOS, inspector shows "composite" every frame on iOS and macOS
Comment 4 Radar WebKit Bug Importer 2021-08-05 10:29:32 PDT
<rdar://problem/81573075>
Comment 5 Simon Fraser (smfr) 2021-08-05 11:06:30 PDT
Regressed at https://trac.webkit.org/changeset/268932/webkit
Comment 6 Simon Fraser (smfr) 2021-08-05 11:29:41 PDT
I think this happens because we get to the end of a rendering update, but haven't cleared the Animations bit:

Page 0x3bc5f9680 renderingUpdateCompleted() - steps [] unfulfilled steps [Animations]
Comment 7 Simon Fraser (smfr) 2021-08-05 11:36:56 PDT
The problem is that the layoutIfNeeded() at the end of a rendering update dirties the Animations and LayerFlush bits, so we trigger rendering updates on every frame.

te(this=0x000000018c5f9680, requestedSteps={ size = 1 }) at Page.cpp:1469:9
    frame #1: 0x00000001672d8df5 WebCore`WebCore::RenderLayerCompositor::scheduleRenderingUpdate(this=0x000000018c3e9720) at RenderLayerCompositor.cpp:574:12
    frame #2: 0x00000001672d4249 WebCore`WebCore::RenderLayerCompositor::notifyFlushRequired(this=0x000000018c3e9720, (null)=0x000000018c3ef780) at RenderLayerCompositor.cpp:567:5
    frame #3: 0x00000001672d421f WebCore`WebCore::RenderLayerBacking::notifyFlushRequired(this=0x000000018c34d870, layer=0x000000018c3ef780) at RenderLayerBacking.cpp:3747:18
    frame #4: 0x0000000166ccfea9 WebCore`WebCore::GraphicsLayerCA::noteLayerPropertyChanged(this=0x000000018c3ef780, flags=16, scheduleFlush=ScheduleFlush) at GraphicsLayerCA.cpp:4709:22
    frame #5: 0x0000000166cd13e9 WebCore`WebCore::GraphicsLayerCA::setTransform(this=0x000000018c3ef780, t=0x00007ffee5eeab80) at GraphicsLayerCA.cpp:675:5
    frame #6: 0x00000001672c2937 WebCore`WebCore::RenderLayerBacking::updateTransform(this=0x000000018c34d870, style=0x000000018cecf7b8) at RenderLayerBacking.cpp:647:26
    frame #7: 0x00000001672c7e3c WebCore`WebCore::RenderLayerBacking::updateGeometry(this=0x000000018c34d870, compositedAncestor=0x000000018c3e9850) at RenderLayerBacking.cpp:1278:5
    frame #8: 0x00000001672db448 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x000000018c3e9720, layer=0x000000018c3e9d10, childLayersOfEnclosingLayer={ size = 0, capacity = 0 }, traversalState=0x00007ffee5eebb58, scrollingTreeState=0x00007ffee5eebb78, updateLevel={ size = 0 }) at RenderLayerCompositor.cpp:1341:27
    frame #9: 0x00000001672db9a0 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x000000018c3e9720, layer=0x000000018c3e9be0, childLayersOfEnclosingLayer={ size = 0, capacity = 0 }, traversalState=0x00007ffee5eebda8, scrollingTreeState=0x00007ffee5eebdc8, updateLevel={ size = 0 }) at RenderLayerCompositor.cpp:1406:13
    frame #10: 0x00000001672db9a0 WebCore`WebCore::RenderLayerCompositor::updateBackingAndHierarchy(this=0x000000018c3e9720, layer=0x000000018c3e9850, childLayersOfEnclosingLayer={ size = 0, capacity = 0 }, traversalState=0x00007ffee5eebf50, scrollingTreeState=0x00007ffee5eebf98, updateLevel={ size = 0 }) at RenderLayerCompositor.cpp:1406:13
    frame #11: 0x00000001672d88fa WebCore`WebCore::RenderLayerCompositor::updateCompositingLayers(this=0x000000018c3e9720, updateType=AfterStyleChange, updateRoot=0x000000018c3e9850) at RenderLayerCompositor.cpp:916:9
    frame #12: 0x00000001672d7a7b WebCore`WebCore::RenderLayerCompositor::didRecalcStyleWithNoPendingLayout(this=0x000000018c3e9720) at RenderLayerCompositor.cpp:549:12
    frame #13: 0x000000016684d937 WebCore`WebCore::FrameView::updateCompositingLayersAfterStyleChange(this=0x000000018cecc010) at FrameView.cpp:810:39
    frame #14: 0x0000000165a3a6d9 WebCore`WebCore::Document::resolveStyle(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/animations/animation-api-two-fixed.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }, type=Normal) at Document.cpp:2088:46
    frame #15: 0x0000000165a3b3c0 WebCore`WebCore::Document::updateStyleIfNeeded(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/animations/animation-api-two-fixed.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:2176:5
    frame #16: 0x000000016683d287 WebCore`WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive(this=0x000000018cecc010) at FrameView.cpp:4445:43
    frame #17: 0x00000001668b72ae WebCore`WebCore::Page::layoutIfNeeded(this=0x000000018c5f9680) at Page.cpp:1463:15
  * frame #18: 0x00000001668b7ba5 WebCore`WebCore::Page::updateRendering(this=0x000000018c5f9680) at Page.cpp:1577:5


  * frame #0: 0x00000001668b73dc WebCore`WebCore::Page::scheduleRenderingUpdate(this=0x000000018c5f9680, requestedSteps={ size = 1 }) at Page.cpp:1469:9
    frame #1: 0x0000000165364be7 WebCore`WebCore::DocumentTimeline::scheduleAnimationResolution(this=0x000000018c37fae0) at DocumentTimeline.cpp:170:25
    frame #2: 0x0000000165368c15 WebCore`WebCore::DocumentTimeline::animationAcceleratedRunningStateDidChange(this=0x000000018c37fae0, animation=0x000000018cecf660) at DocumentTimeline.cpp:436:9
    frame #3: 0x0000000165376249 WebCore`WebCore::WebAnimation::acceleratedStateDidChange(this=0x000000018cecf660) at WebAnimation.cpp:1254:49
    frame #4: 0x0000000165376183 WebCore`WebCore::KeyframeEffect::addPendingAcceleratedAction(this=0x000000018c363840, action=TransformChange) at KeyframeEffect.cpp:1658:18
    frame #5: 0x00000001653763ee WebCore`WebCore::KeyframeEffect::transformRelatedPropertyDidChange(this=0x000000018c363840) at KeyframeEffect.cpp:1686:5
    frame #6: 0x0000000165378ad2 WebCore`WebCore::KeyframeEffectStack::applyKeyframeEffects(this=0x0000000195ec49a0, targetStyle=0x0000000195ebd660, previousLastStyleChangeEventStyle=0x0000000195ebd5a0, parentElementStyle=0x000000018cecdc58) at KeyframeEffectStack.cpp:145:21
    frame #7: 0x000000016762fe9c WebCore`WebCore::Styleable::applyKeyframeEffects(this=0x00007ffee5ee9d98, targetStyle=0x0000000195ebd660, previousLastStyleChangeEventStyle=0x0000000195ebd5a0, parentElementStyle=0x000000018cecdc58) const at Styleable.h:93:60
    frame #8: 0x000000016762e9c0 WebCore`WebCore::Style::TreeResolver::createAnimatedElementUpdate(newStyle=WebCore::RenderStyle @ 0x0000000195ebd540, styleable=0x00007ffee5ee9d98, parentChange=None, parentStyle=0x000000018cecdc58, parentBoxStyle=0x000000018cecdc58) at StyleTreeResolver.cpp:355:37
    frame #9: 0x000000016762e0aa WebCore`WebCore::Style::TreeResolver::resolveElement(this=0x00007ffee5eec7c0, element=0x000000018cecf3d0) at StyleTreeResolver.cpp:226:19
    frame #10: 0x0000000167630835 WebCore`WebCore::Style::TreeResolver::resolveComposedTree(this=0x00007ffee5eec7c0) at StyleTreeResolver.cpp:556:35
    frame #11: 0x0000000167631784 WebCore`WebCore::Style::TreeResolver::resolve(this=0x00007ffee5eec7c0) at StyleTreeResolver.cpp:619:5
    frame #12: 0x0000000165a3a5d4 WebCore`WebCore::Document::resolveStyle(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/animations/animation-api-two-fixed.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }, type=Normal) at Document.cpp:2071:37
    frame #13: 0x0000000165a3b3c0 WebCore`WebCore::Document::updateStyleIfNeeded(this={ origin = file://, url = file:///Volumes/Data/Development/system/webkit/testcontent/animations/animation-api-two-fixed.html, inMainFrame = Detached, backForwardCacheState = NotInBackForwardCache }) at Document.cpp:2176:5
    frame #14: 0x000000016683d287 WebCore`WebCore::FrameView::updateLayoutAndStyleIfNeededRecursive(this=0x000000018cecc010) at FrameView.cpp:4445:43
    frame #15: 0x00000001668b72ae WebCore`WebCore::Page::layoutIfNeeded(this=0x000000018c5f9680) at Page.cpp:1463:15
    frame #16: 0x00000001668b7ba5 WebCore`WebCore::Page::updateRendering(this=0x000000018c5f9680) at Page.cpp:1577:5
Comment 8 Simon Fraser (smfr) 2021-08-05 12:26:08 PDT
Interestingly, the bug goes away if I make a trivial change to the class attribute ("bubble ") or disable style sharing.
Comment 9 Simon Fraser (smfr) 2021-08-05 15:24:49 PDT
Liam, does this affect any live web sites that you know of?
Comment 10 Liam DeBeasi 2021-08-06 06:54:51 PDT
I discovered this issue while demoing Ionic's animation library benchmark website: https://flamboyant-brattain-9c21f4.netlify.app/

(Select "Ionic" and a bubble count and click "Start Test")

Other than the benchmark website, I am not aware of any other sites impacted by this.
Comment 11 Antoine Quint 2021-11-03 11:15:13 PDT
Style sharing is disabled for elements with CSS Transitions or CSS Animations in SharingResolver::canShareStyleWithElement():

    if (style->transitions() || style->animations())
        return false;

This should likely check whether there are any keyframe effects for the candidate element.
Comment 12 Antoine Quint 2021-11-03 12:41:15 PDT
Created attachment 443227 [details]
Patch
Comment 13 Antti Koivisto 2021-11-03 13:41:17 PDT
Comment on attachment 443227 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443227&action=review

> Source/WebCore/style/StyleSharingResolver.cpp:263
> -    if (style->transitions() || style->animations())
> +    if (candidateElement.hasKeyframeEffects(PseudoId::None))

Transitions are "keyframe effects" too?
Comment 14 Antoine Quint 2021-11-03 14:43:57 PDT
(In reply to Antti Koivisto from comment #13)
> Comment on attachment 443227 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443227&action=review
> 
> > Source/WebCore/style/StyleSharingResolver.cpp:263
> > -    if (style->transitions() || style->animations())
> > +    if (candidateElement.hasKeyframeEffects(PseudoId::None))
> 
> Transitions are "keyframe effects" too?

Yes.
Comment 15 Antoine Quint 2021-11-04 01:58:02 PDT
Committed r285256 (243867@main): <https://commits.webkit.org/243867@main>