Performance: Cache ProxyAnimation instances #84
Conversation
bramus
force-pushed
the
quick-perf-win
branch
from
2955b8e to
b59ed75
Compare
|
FYI: Rebased this branch onto |
src/scroll-timeline-css.js
Outdated
| const proxyAnimation = new ProxyAnimation(anim, result.timeline, result.animOptions); | ||
| // Create a cache on the element, to store all Proxy Animations in | ||
| if (!evt.target.proxiedAnimations) { | ||
| evt.target.proxiedAnimations = new Map(); |
There was a problem hiding this comment.
We should not hang new stuff off element as it breaks encapsulation. We have animation and timeline details to avoid exposing polyfill internals to the outside world.
Instead we can add a global element map to the polyfill, i.e.:
let elementAnimationsMap = new WeakMap();
and then retrieve the map of proxied animations from there. Please take a look at how proxyAnimations.get(...) is used to retrieve animation details without exposing to the "outside world". We should follow the same pattern here.
There was a problem hiding this comment.
Addressed in 3fb2b7f.
Note I could not use a WeakMap for the nested cache, as WeakMaps require Objects as their key. As the used anim always is a new Object, this didn’t work – it only created more Objects.
| Using WeakMap | Using Map |
|---|---|
 |
 |
| const elementProxyAnimations = proxyAnimations.get(evt.target); | ||
|
|
||
| // Store Proxy Animation in the cache | ||
| if (!elementProxyAnimations.has(anim.animationName)) { |
There was a problem hiding this comment.
animation-name allows listing the same animation more than once, e.g.
#target {
animation-name: spin, spin;
}This is expected to produce 2 animations with the latter having a higher composite order, but I believe this change will result in a single animation.
There was a problem hiding this comment.
We'll need to track the index as well as the name.
There was a problem hiding this comment.
I created an alternative version in #89 which also implements getAnimations support in this polyfill.
…mations This makes document.getAnimations and Element.getAnimations correctly return proxied instances of the animation so as to behave as expected. Also by getting the proxied animations we can intelligently reuse the existing proxied CSS animation when only timing details have changed which implicitly fixes the performance issue of #84.
…tions (#89) This makes document.getAnimations and Element.getAnimations correctly return proxied instances of the animation so as to behave as expected. Also by getting the proxied animations we can intelligently reuse the existing proxied CSS animation when only timing details have changed which implicitly fixes the performance issue of #84.
Currently, the CSS polyfill creates new
ProxyAnimationinstances every time an animation starts. As you’re scrolling back and forth, you can see performance stutter.This PR caches created instances onto the targeted elements, which improves performance.
Tested using the demo from https://codepen.io/bramus/pen/GRdGoKy, the performance timelines look like this: