1550511 - Slow interaction with interactive lichess analysis graph (inline SVG in a document with slow-to-reflow grid and flex ancestors)

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Gian-Carlo Pascutto [:gcp]]

https://github.com/ornicar/lila/issues/5080

STR:

1. Navigate to: https://lichess.org/SWmnJhnm
2. Hover over the graph on the bottom and move left and right
3. Notice how the red dot trails your cursor

Test the same in Chrome: no cursor lag in 3).

From a quick look, this is using Highcharts 4.2.5.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

I'll try to get a profile here soon.

Comment 2

[Markus Stange [:mstange]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #0)

https://github.com/ornicar/lila/issues/5080

STR:

1. Navigate to: https://lichess.org/SWmnJhnm
2. Hover over the graph on the bottom and move left and right

I had to scroll down first and switch to the "Computer analysis" tab before it was showing me a graph.

Profile: https://perfht.ml/309jTc3

Looks like a layout problem.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Thanks Markus!

Comment 4

[Gian-Carlo Pascutto [:gcp]]

As pointed out here: https://github.com/ornicar/lila/issues/5080#issuecomment-490968080

Our performance goes up when the window is made smaller, so all the visual elements sit vertically stacked. If you hover the graph then, our performance is much better. So something about the side by side layout is killing our performance.

Comment 5

[Daniel Holbert [:dholbert]]

The page uses nested CSS grid + flex containers, which are both a bit overeager in reflowing things that may not have changed, when one of their other descendants changes.

As noted in https://github.com/ornicar/lila/issues/5080#issuecomment-490968080 , it sounds like Firefox 58 and earlier were "fast" -- and Firefox 58 was the last release that lacked preffed-on-by-default CSS Grid support per https://developer.mozilla.org/en-US/docs/Web/CSS/grid#Browser_compatibility .

The page gets much more responsive in Firefox Nightly if you add contain: layout size to the outer <svg> element for the graph, which makes that element into a reflow root and allows us to exempt the rest of the page from needing a possible-relayout when the SVG subtree changes, per bug 1497414. If we're in contact with the page developers, we could suggest that as an improvement (though it'll only help in versions with CSS Containment, which for now is just Firefox 68 and might only make it to release in 69 -- that's tracked in bug 1487493).

I also filed bug 1550535 on making inline SVG content into reflow roots a bit more opportunistically (i.e. not requiring contain:layout size).

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #4)

Our performance goes up when the window is made smaller, so all the visual elements sit vertically stacked. If you hover the graph then, our performance is much better. So something about the side by side layout is killing our performance.

This is also an interesting observation. At first glance, it looks like the same ancestors are flex/grid containers in both layouts, so it's interesting that we do get away with avoiding slow flex/grid relayouts in one scenario but not the other. It's possible that the side-by-side layout has more dependence on content sizing, whereas the single-column layout has its grid tracks set up to not care about content size as much.

It's likely that this would also benefit from caching grid-item "measuring reflows", as suggested in bug 1524056 comment 8.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

If we're in contact with the page developers, we could suggest that as an improvement (though it'll only help in versions with CSS Containment, which for now is just Firefox 68 and might only make it to release in 69 -- that's tracked in bug 1487493).

We can get to the page developers, but the SVG appears to be generated by Highcharts (https://www.highcharts.com/) which appears to be a reasonably popular graphing library. If this is internal to that library we could try reaching out to them to do that optimization.

Comment 8

[Gian-Carlo Pascutto [:gcp]]

s noted in https://github.com/ornicar/lila/issues/5080#issuecomment-490968080 , it sounds like Firefox 58 and earlier were "fast" -

He noted that he did not test earlier releases, not that they were fast. The site requires Firefox 61 and later, I assume because they just switched to a new grid+flex based design.

Comment 9

[Gian-Carlo Pascutto [:gcp]]

The user did another test with the SVG tab switched away and still reports bad scrolling performance in the move list. This doesn't reproduce for me, but he provided both a release and a Nightly profile: https://github.com/ornicar/lila/issues/5080#issuecomment-491208623

dholbert, is still this the SVG issue underneath or something else?

Comment 10

[Daniel Holbert [:dholbert]]

The underlying issue is that grid + flex can be slow to reflow in some circumstances (and extra bad if they're nested multiple levels deep and trigger the slow paths).

So the best solution is to address that & make their reflows cheaper. bug 1524056 covers some of that.

The SVG thing here just happens to be something that triggers frequent reflows, which in an ideal world would be fast even if they traversed the flex/grid content. So bug 1550535 helps here by letting us skip reflowing the sluggish flex/grid ancestors in response to changes inside the SVG -- but other sorts of actions outside of the SVG that trigger reflows will probably still be slow.

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

bug 1524056 and bug 1550535 are higher priority and once those are fixed, this should be re-evaluated.

Comment 12

[Sean Feng [:sefeng]]

I see the red dot trails my cursor well in the latest Nightly. looks like it's fixed?

Comment 13

[Markus Stange [:mstange]]

It might have been fixed by bug 1492538. Sean, can you test a build from before that fix and compare?

Comment 14

[Sean Feng [:sefeng]]

So...I can reliably reproduce it in 2019-05-20's mozilla-central, and the sluggish is gone in Release 70.

I narrowed it down to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=29bdbbe89264b91d1bd4205c45536c9222a8d6ee&tochange=257f2c96cef502a1d674df56c8e39d76d8ed4d89, however, mozregression doesn't allow me to bisect any further.

Comment 15

[Markus Stange [:mstange]]

Oh, bug 1550535 is in that range, which explicitly mentions this bug.

Comment 16

[Sean Feng [:sefeng]]

Ah, gotcha. I am going to close this bug as we can't reproduce it anymore, please re-open if you disagree! :)