web-animations / web-animations-next

Development repository for web-animations-js:
https://github.com/web-animations/web-animations-js
Apache License 2.0
121 stars 30 forks source link

Seeking very slow with many nodes and keyframes #472

Open jide opened 8 years ago

jide commented 8 years ago

I made a little demo of an animation that targets 100 nodes with a SequenceEffect of 5 KeyframeEffect each.

http://jsbin.com/gicoju/edit?js

On chrome, although the animation creation takes time, seeking is ultra fast. On polyfilled browsers (i.e. safari desktop) the main thread seems unresponsive.

Is this due to the way the polyfill is done, or can this be due to a hidden bottleneck somewhere ?

alancutter commented 7 years ago

I suspect this is due to https://github.com/web-animations/web-animations-next/commit/409df4d4a4608a6a45c437066dc4185ffcd43f8a which forces effects to be reapplied as soon as they happen as a work around other browsers not executing RAF in the correct order with regular script.

We should rethink the approach. Perhaps performing the invalidation in a setTimeout(..., 0) instead of immediately. @dstockwell WDYT?

alancutter commented 7 years ago

Spoke with dstockwell, sounds like setTimeout(..., 0) won't work as the bug fix was for single frame flashes rather than persistant incorrect states. Synchronous updating is the only way to guarantee effects are being applied before frames get rendered.

An alternative solution to improve performance would be to reduce the number of updated animations by adding dirty flags.

jide commented 7 years ago

Thanks for the explanation. I'll keep an eye on the issue, I'm not familiar enough with the internals of the project to suggest something. I guess setImmediate won't help either.

alancutter commented 7 years ago

http://jsbin.com/jinexesado/edit?output

Performance is better but still not as good as it could be. The synchronous update to animation effects to work around browser bugs is what's holding us back. I also noticed that GC was the dominant use of CPU during loading, being more conservative with our object allocation would help performance there.

jide commented 7 years ago

Yay, it's a great improvement !

I was thinking: In this example, in the end, there is only one running animation at a time, all others have either not started or are already ended (before delay, or after delay + duration)

Couldn't something smart be done to ignore updating animations that are in these inactive states ?

alancutter commented 7 years ago

I suspect we may be redundantly clearing their effects, if so that's something that could be optimised out.

jide commented 7 years ago

For the record, in a react app I did, I kind of did this manually:

I have a lot of animated divs animated one after each other, like in the example. I mount divs around the current time when it changes and pass them the currentTime of the animation. It is much much faster than animating all the divs at once.

What's interesting is that I tried an approach with all the divs empty along with their animations, and then putting things inside on seeking, or mounting the divs and then create the animation on seeking and set the currentTime, and the latter is much more performant.

jide commented 7 years ago

To illustrate what I mean :

Approach 1, performance is bad :

<div>Some content #1</div> // starts at 0
<div/> // starts at 1
<div/> // starts at 2
<div/> // starts at 3
<div/> // starts at 4

// set currentTime on all these divs to 1.5. We fill the divs with their content as currentTime changes :

<div/> // set currentTime 1.5
<div>Some content #2</div> // set currentTime 1.5
<div/> // set currentTime 1.5
<div/> // set currentTime 1.5
<div/> // set currentTime 1.5

Approach 2, performance is better :

<div>Some content #1</div>
<!-- empty -->
<!-- empty -->
<!-- empty -->
<!-- empty -->

// set currentTime to 1.5, mount divs that are in range as currentTime changes and set their currentTime :

<!-- empty -->
<div>Some content #2</div> // set currentTime 1.5
<!-- empty -->
<!-- empty -->
<!-- empty -->

So my conclusion is that setting currentTime on divs that are out of bounds regarding delay and duration takes time, although I'd thought it should not.

On Chrome approach 1 is faster, in polyfilled browsers approach 2 is faster.

jide commented 7 years ago

Said differently: the DOM operation of creating a dom node is faster than setting currentTime on a node that is out of its active duration.