Skip to content

[WebAnimation] Optimize updateCSSTransitions#17897

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Constellation:eng/WebAnimation-Optimize-updateCSSTransitions
Sep 19, 2023
Merged

[WebAnimation] Optimize updateCSSTransitions#17897
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
Constellation:eng/WebAnimation-Optimize-updateCSSTransitions

Conversation

@Constellation
Copy link
Member

@Constellation Constellation commented Sep 19, 2023 •
edited by webkit-early-warning-system
Loading

7fd0774

[WebAnimation] Optimize updateCSSTransitions
https://bugs.webkit.org/show_bug.cgi?id=261715
rdar://115703255

Reviewed by Mark Lam.

WebAnimation's updateCSSTransitions is inefficient,

1. We are repeatedly copying AnimationProperty while it is held. This is appearing much in the profiler.
   Let's not copy them. We use `const AnimationProperty&`. This is OK since
   1.1. updateCSSTransitionsForStyleableAndProperty's owner is holding this ownership. Reference is always valid.
   1.2. Other change immediately copies or uses switchOn to dispatch. So this is fine.
2. Avoid repeated call of `styleable.hasRunningTransitionForProperty` etc. This is really costly function since
   it involves HashMap lookup. We reorganize updateCSSTransitionsForStyleableAndProperty not to do wasteful
   operations multiple times.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyBlendingContext::CSSPropertyBlendingContext):
(WebCore::CSSPropertyAnimation::blendProperty):
(WebCore::CSSPropertyAnimation::isPropertyAnimatable):
(WebCore::CSSPropertyAnimation::isPropertyAdditiveOrCumulative):
(WebCore::CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::canPropertyBeInterpolated):
* Source/WebCore/animation/CSSPropertyAnimation.h:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::hasCompletedTransitionForProperty const):
(WebCore::Element::hasRunningTransitionForProperty const):
* Source/WebCore/dom/Element.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::keyframeEffectForElementAndProperty):
(WebCore::propertyInStyleMatchesValueForTransitionInMap):
(WebCore::transitionMatchesProperty):
(WebCore::updateCSSTransitionsForStyleableAndProperty):
(WebCore::Styleable::updateCSSTransitions const):
* Source/WebCore/style/Styleable.h:
(WebCore::Styleable::hasCompletedTransitionForProperty const):
(WebCore::Styleable::hasRunningTransitionForProperty const):

Canonical link: https://commits.webkit.org/268113@main

bdc2485

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@Constellation Constellation self-assigned this Sep 19, 2023
@Constellation Constellation added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Sep 19, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 70373d6)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
⏳ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
⏳ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64   🛠 tv ⏳ 🧪 mac-AS-debug-wk2
  🛠 tv-sim
⏳ 🛠 watch
  🛠 watch-sim

@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from 70373d6 to d4a69c1 Compare September 19, 2023 03:45
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash d4a69c1)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe ⏳ 🛠 wincairo
⏳ 🧪 bindings ⏳ 🛠 ios-sim ⏳ 🛠 mac-AS-debug   🧪 wpe-wk2
  🧪 webkitperl ⏳ 🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
⏳ 🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc ⏳ 🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
⏳ 🛠 tv-sim
⏳ 🛠 watch
⏳ 🛠 watch-sim

@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from d4a69c1 to cbaf507 Compare September 19, 2023 03:53
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash cbaf507)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 wincairo
  🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64   🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
  🛠 watch-sim

@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch 2 times, most recently from 1437603 to b0eeb76 Compare September 19, 2023 04:16
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash b0eeb76)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac   🛠 wpe   🛠 wincairo
  🧪 bindings ❌ 🛠 ios-sim ⏳ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac ❌ 🛠 gtk
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ❌ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2 ❌ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
❌ 🛠 tv-sim ❌ 🧪 mac-wk2-stress
❌ 🛠 watch
⏳ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 1437603)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios   🛠 mac   🛠 wpe ⏳ 🛠 wincairo
⏳ 🧪 bindings   🛠 ios-sim ❌ 🛠 mac-AS-debug   🧪 wpe-wk2
  🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv ❌ 🧪 mac-AS-debug-wk2
⏳ 🧪 services ❌ 🛠 tv-sim   🧪 mac-wk2-stress
  🛠 watch
⏳ 🛠 watch-sim

MenloDorian
MenloDorian approved these changes Sep 19, 2023
View reviewed changes
Copy link

@MenloDorian MenloDorian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me if EWS is happy.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 19, 2023
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Sep 19, 2023
@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from b0eeb76 to 7e82d79 Compare September 19, 2023 04:22
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 7e82d79)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ❌ 🛠 ios ❌ 🛠 mac ❌ 🛠 wpe   🛠 wincairo
  🧪 bindings ❌ 🛠 ios-sim ⏳ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ❌ 🧪 api-mac   🛠 gtk
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1   🧪 gtk-wk2
⏳ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk2   🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
⏳ 🛠 tv-sim ❌ 🧪 mac-wk2-stress
⏳ 🛠 watch
⏳ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 19, 2023
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Sep 19, 2023
@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from 7e82d79 to eeece84 Compare September 19, 2023 04:30
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on previous version of this PR (hash eeece84)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ✅ 🛠 gtk
  🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2 ✅ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ⏳ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Constellation Constellation force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from eeece84 to bdc2485 Compare September 19, 2023 04:47
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Sep 19, 2023 •
edited
Loading

EWS run on current version of this PR (hash bdc2485)

Details

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@Constellation Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 19, 2023
@Constellation
[WebAnimation] Optimize updateCSSTransitions
7fd0774 
https://bugs.webkit.org/show_bug.cgi?id=261715
rdar://115703255

Reviewed by Mark Lam.

WebAnimation's updateCSSTransitions is inefficient,

1. We are repeatedly copying AnimationProperty while it is held. This is appearing much in the profiler.
   Let's not copy them. We use `const AnimationProperty&`. This is OK since
   1.1. updateCSSTransitionsForStyleableAndProperty's owner is holding this ownership. Reference is always valid.
   1.2. Other change immediately copies or uses switchOn to dispatch. So this is fine.
2. Avoid repeated call of `styleable.hasRunningTransitionForProperty` etc. This is really costly function since
   it involves HashMap lookup. We reorganize updateCSSTransitionsForStyleableAndProperty not to do wasteful
   operations multiple times.

* Source/WebCore/animation/CSSPropertyAnimation.cpp:
(WebCore::CSSPropertyBlendingContext::CSSPropertyBlendingContext):
(WebCore::CSSPropertyAnimation::blendProperty):
(WebCore::CSSPropertyAnimation::isPropertyAnimatable):
(WebCore::CSSPropertyAnimation::isPropertyAdditiveOrCumulative):
(WebCore::CSSPropertyAnimation::propertyRequiresBlendingForAccumulativeIteration):
(WebCore::CSSPropertyAnimation::animationOfPropertyIsAccelerated):
(WebCore::CSSPropertyAnimation::propertiesEqual):
(WebCore::CSSPropertyAnimation::canPropertyBeInterpolated):
* Source/WebCore/animation/CSSPropertyAnimation.h:
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::hasCompletedTransitionForProperty const):
(WebCore::Element::hasRunningTransitionForProperty const):
* Source/WebCore/dom/Element.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::keyframeEffectForElementAndProperty):
(WebCore::propertyInStyleMatchesValueForTransitionInMap):
(WebCore::transitionMatchesProperty):
(WebCore::updateCSSTransitionsForStyleableAndProperty):
(WebCore::Styleable::updateCSSTransitions const):
* Source/WebCore/style/Styleable.h:
(WebCore::Styleable::hasCompletedTransitionForProperty const):
(WebCore::Styleable::hasRunningTransitionForProperty const):

Canonical link: https://commits.webkit.org/268113@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebAnimation-Optimize-updateCSSTransitions branch from bdc2485 to 7fd0774 Compare September 19, 2023 08:30
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Sep 19, 2023

Committed 268113@main (7fd0774): https://commits.webkit.org/268113@main

Reviewed commits have been landed. Closing PR #17897 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 7fd0774 into WebKit:main Sep 19, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 19, 2023
@Constellation Constellation deleted the eng/WebAnimation-Optimize-updateCSSTransitions branch September 19, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MenloDorian MenloDorian MenloDorian approved these changes

@graouts graouts Awaiting requested review from graouts graouts is a code owner

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@Constellation Constellation

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Constellation @webkit-early-warning-system @webkit-commit-queue @MenloDorian @webkit-ews-buildbot