1322328 - Animation demo regressed after bug 1300659

Status: RESOLVED WONTFIX

(Core :: DOM: Core & HTML, defect)

Description

[Emilio Cobos Álvarez (:emilio)]

Apparently this is intended, but I was profiling some style system stuff today, checked out this demo[1], and noticed how it had regressed on nightly.

I'm filling this because I suspect more people may be doing this stuff, so this may be a compat issue.

[1]: https://mozdevs.github.io/servo-experiments/experiments/tiles/

Comment 1

[Emilio Cobos Álvarez (:emilio)]

needinfoing bkelly since he wrote the patch in bug 1300659.

Comment 2

[Markus Stange [:mstange]]

That demo should really be doing the color changes during a requestAnimationFrame callback.

But you're right, it's possible that more people are doing expensive animations using timeouts which would be "slowed down" as we paint more frequently between the individual timeouts.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Markus Stange [:mstange] from comment #2)
> That demo should really be doing the color changes during a
> requestAnimationFrame callback.

Oh, sure, I just was testing with it since the last time I profiled it it treated pretty badly our restyling code.

I just filled it since I noticed this and I was slightly worried about the web compat perspective.

I'm personally fine breaking this since there's an obvious better alternative (RAF), this was a very artificial test case, and this has gone unnoticed for about a month (so I expect this not to be really common). But I thought I'd let other people decide about that :)

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I'll look at it.  I noticed we also have some interesting behavior when switching tabs.  We trigger a lot of tile changes when switching back.  I'm curious why that is happening.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Attachment: ff50-tile-demo.png

Here is a screen capture of the performance timeline in FF50 running this demo.  We are basically hitting 0 fps for long periods.  Its so bad devtools can't even complete its processing of the timeline.  I just get beachball cursor and eventually the long script modal dialog for the devtools script.  The main thread is just too janked.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Attachment: ff52-tile-demo.png

Here is a screen capture of FF52 which includes bug 1300659.  Here we can see that the browser maintains around 30fps.  In addition, devtools is able to actually complete processing the performance timeline without problem.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

At the end of the day this demo schedules more work than firefox can complete in the given timeframe.  The question is, should we allow this workload to jank the browser or should we limit the workload in order to keep the browser functioning?

IMO we should definitely slow down the workload in order to keep the browser functioning for the user.  Dropping to 0fps and breaking things like devtools seems pretty bad to me.

In addition, the reduction in speed here should be automatically addressed once our quantum project efforts speed up layout and painting.  If layout and painting can run at 60fps, then this animation will paint at 60fps.

In general, I think we should discourage sites from scheduling huge numbers of timers in a storm that attempts to jank the browser.  There are many ways to perform this kind of animation without that technique.  Besides requestAnimationFrame(), the site could also measure how long it was since the last timer fired and flip more than one tile at a time.

So I'd like to recommend we WONTFIX this.

Finally, as a side note.  I believe we are able to flip many tiles during tab switch because the timers do not trigger painting when the tab is in the background.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

To clarify, the site schedules all the tiles flips as separate timers up front.  This is thousands of timers being scheduled.  Chaining a single timer to the next time would be a more traditional way to do this with less chance of janking the main thread.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

I'm totally fine wontfixing this. Feel free to reopen if somebody disagrees.

Thanks Ben for checking it, just wanted to make sure you were fine with this :)

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Bug 1342854 shows a real site that kind of uses this approach.  It has a number of parallel setInterval() callbacks that each trigger a very slow paint.