1194155 - Slow relayout on hover-triggered "box-shadow" transition, with deeply nested flexboxes

Status: RESOLVED WORKSFORME

(Core :: Layout, defect, P3)

Description

[baxxabit]

Attachment: sample.zip

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.130 Safari/537.36

Steps to reproduce:

Firefox 41.0a2 and stable versions below.

I've made a simple page, but it has a complex structure of flex containers.

I move mouse over the image (the link to image is broken, doesn't matter) and on hover there is transition of box-shadow.


Actual results:

Transition of box-shadow accordingly with http://csstriggers.com/#box-shadow there shouldn't be "layout" action, but I can be wrong here.

Here is what I see in dev tools. http://i.imgur.com/VruUjU7.png

Same for 20 'li' items http://i.imgur.com/gncpM8j.png You can see that "layout" takes >100ms and for a single hover we have 3 layout actions, so >300ms

Also I see a "shake" of the blocks and it looks like the cause is a recalculation of flexbox dimensions. Here is an example of "shake" http://recordit.co/DjIwqbph37


Expected results:

There is no any "layout" actions in Chrome and IE11 with the same flex containers.

I guess that the problem exact in the flex-es, because if I disable on of the top flex containers, there will not be a long "layout".

Comment 1

[baxxabit]

Firefox 43 Nightly also affected. There is no "shake", but performance after image's hover is bad.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (standalone version of original testcase)

Here's a standalone version of the reporter's testcase (unzipped, with the CSS moved directly into the HTML file), for easier viewing.

Comment 3

[Daniel Holbert [:dholbert]]

 (In reply to baxxabit from comment #1)
> Firefox 43 Nightly also affected. There is no "shake", but performance after
> image's hover is bad.

(It actually looks pretty smooth to me in a Nightly build, but I can definitely see some jank in a debug build when hovering. My machine may just be too fast to show the jank with an opt build.)


(In reply to baxxabit from comment #0)
> Actual results:
> 
> Transition of box-shadow accordingly with http://csstriggers.com/#box-shadow
> there shouldn't be "layout" action, but I can be wrong here.

FWIW, in our style-system code, we *do* show that box-shadow differences trigger reflow. But that might no longer be necessary.

First: here's where we determine what sort of recomputation is needed after a "box-shadow" change:
> 521   nsChangeHint shadowDifference =
> 522     CalcShadowDifference(mBoxShadow, aOther.mBoxShadow);
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.cpp?rev=96d67fd3c056#521

...and CalcShadowDifference is defined lower down in that file and returns NS_STYLE_HINT_REFLOW if anything is different:
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.cpp?rev=96d67fd3c056#3374

I suspect we have NS_STYLE_HINT_REFLOW there just because "box-shadow" influences the overflow areas (the region that needs to be repainted when a frame is changed), and we compute overflow areas during reflow.  However -- we have a better hint for these sorts of updates now, which doesn't require reflow.

This is similar to bug 1131371 (which avoided similar unnecessary reflows for 'outline')

Comment 4

[Daniel Holbert [:dholbert]]

It looks like we already tried to make 'box-shadow' changes trigger the more-targeted "UpdateOverflow" hint, back in bug 719177, but that bug was backed out in bug 724432.  (Because the original changes caused bug 723669.)

Comment 5

[Daniel Holbert [:dholbert]]

I spun off bug 1194480 to hopefully make "box-shadow" really not trigger reflow. (That might be all that's required to fix this bug here; not sure.)

Comment 6

[baxxabit]

Daniel, 

the problem isn't only in the "box-shadow", but in every user action that triggers "layout". And "layout" takes >300ms in my case (when number of li on test page > 20). 

I can provide another test case with 100 "li" and you will see the time that is needed for "layout" in this case. 

My assumption is that "layout" shouldn't take so much time. And Chrome and IE11 makes layout much more faster.

Comment 7

[baxxabit]

Just for clarifying: from my point of view the problem can be separated into two

 - transform of box-shadow on hover triggers "layout" that's sometimes not necessary
 - "layout" action with deeply nested flexboxes takes blocks the browser and takes much time

Comment 8

[Daniel Holbert [:dholbert]]

Thanks. I'm glad I spun off bug 1194480 then; that covers the first part. This bug or another helper-bug may cover the second part.

Comment 9

[baxxabit]

Daniel, can you suggest any workaround for now? Do you need any help from my side?

Comment 10

[Daniel Holbert [:dholbert]]

Sorry for the delay here. Good news, though!  I get much better performance (and no noticeable rendering difference) if I add this style rule in devtools:
 * { min-height: 0; }

This just disables the flexbox-specific "implied minimum size" feature, which only really matters when things are being shrunk below their intrinsic sizes (and that's not happening here), so I think you're safe to disable it with a rule like this. (or, better, a more targeted form of this rule to just hit flex items in the vertical flex containers that you care about)

Basically, I think a lot of the performance issue here is from us measuring the height of flex items (in a vertical flex container) to determine their "implied minimum size", before we can flex them. This ends up producing a two-pass layout, and nested two-pass layouts tend to blow up.

Can you see if that improves things for you?

Comment 11

[Daniel Holbert [:dholbert]]

Here's the experiment I'm using to test performance here, BTW -- just adjusting the <html> width to force a reflow (and flushing layout by inspecting "offsetHeight").  I'm pasting the following into the web developer console...

let h = document.documentElement; h.style.width = "1000px"; h.offsetHeight; let start = new Date(); h.style.width = "800px"; h.offsetHeight; let end = new Date(); end - start;

...and seeing what the returned number is (in units of milliseconds).

RESULTS:
 "baseline" = loading the testcase un-modified & pasting the above JS into web console.
 "workaround" = loading the testcase, adding workaround from comment 10 in style editor, & pasting the above JS into web console.

* Debug build (slow, un-optimized):
 - Baseline: 210-220ms
 - Workaround: 50-60ms

* Nightly build (optimized):
 - Baseline: 15-30ms
 - Workaround: 4-5ms

So, with the workaround from comment 10, hopefully this is much better for you.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 13

[Daniel Holbert [:dholbert]]

Using the amended STR per comment 11 (i.e. loading the testcase and pasting the let h ... script into web console), I'm seeing results of 0 and occasionally 1 in latest nightly (that's in units of milliseconds).

So, the relayout operation in question takes approximately zero time now.

So something has changed over the years to fix this. Calling this WORKSFORME.