1150123 - Flex-basis settings can cause major layout rendering delays

Status: RESOLVED WORKSFORME

(Core :: Layout, defect)

Description

[Luke May]

Attachment: reporter's testcase

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.101 Safari/537.36

Steps to reproduce:

I have a test-case in a JsFiddle http://jsfiddle.net/cw7h5nt8/2/ and an included html file with the same code as the fiddle that demonstrates the behavior.  In the test-case I have a deeply nested flex-structure that includes label divs that default to `flex: 1 1 10px;`.  If you change the flex basis to "auto", "0px", "1px", or any percentatge (%) then resize the browser window you can see the delays.  Interestingly, the fiddle does have the same issues as running the html file directly (perhaps because it's in an iframe?).  If you resize the viewport via sliding the developer tools window you experience smaller rendering delays (1-3 seconds) where as if you resize the entire browser you experience massive delays (can be as long as 40 seconds in my tests).


Actual results:

Serious rendering delays occur when you follow the steps above.  The build from 03/21/2015 however does not have these rendering delays, but the builds from 03/22/2015 on do have the rendering issues.


Expected results:

No rendering delays should occur for these settings (chrome does not have any rendering delays).

Comment 1

[Luke May]

Correction: the fiddle **DOES NOT** have the same issues as running the html file directly.

Comment 2

[Daniel Holbert [:dholbert]]

I can confirm that I see ~5 second hang on window-resizes (e.g. maximizing the window), if I put in the values "200" and "0px", "1px", or "2px" into the attached testcase.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (partly reduced)

Here's a partly reduced testcase, with all scripting removed to simplify things a bit. (I used a hardcoded value of "2px" in the flex-basis field of the previous testcase, when starting out generating this testcase.)

Hover the nested divs to change the height of the outer div -- current Firefox Nightly hangs for a second or two while rendering that update.  There's no noticeable delay in Chrome.

Comment 4

[Daniel Holbert [:dholbert]]

FWIW: In my reduced testcase at least, I get much better performance if I add "min-height:0px" on .outer and .child elements.  If this perf issue ends up being unavoidable on the gecko side (given this content), that may be the best hackaround, on the content side.

(This prevents us from having to do a "measuring" reflow to establish the default "min-height:auto" value for these elements and all their descendants. It also should have no effect on Chrome's rendering, since they still have "min-height:0" being the default value, per https://code.google.com/p/chromium/issues/detail?id=426898 )

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 3: original testcase, with more extreme values, and with 'min-height:0px' on .outer & .child

For demonstration purposes -- here's the original testcase that Luke attached, modified as follows:
 (1) I put in the values 200 & 2px from comment 1, which make it much slower to render for me.
 (2) I gave .outer and .child "min-height:0px"

With modification #2, the testcase updates after a window resize with only a small amount of lag (on the order of 300ms, instead of ~5 seconds). So, still not perfect, but much better.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Luke May from comment #0)
> The build from 03/21/2015 however does not have these rendering delays, but the builds
> from 03/22/2015 on do have the rendering issues.

My attached 'testcase 2' (reduced from the original testcase) behaves exactly the same in these two nightlies, FWIW.  So there may be multiple issues here.

Comment 7

[Daniel Holbert [:dholbert]]

...and the issues with testcase 2 seem to be unavoidable (on that testcase), with our current setup, I think.

Basically, the flex item has a different size on its "measuring reflow" vs. its final size (after the flex layout algorithm has run). So, we have to reflow to notify its children of the size change.

We may be able to eventually optimize this by caching min-content sizes from one reflow to the next, at least when the available space hasn't changed (which may remove some 'measuring' reflows and cut down on exponential blowup).

I probably won't get to doing that right away, so for now, I'd suggest using the workaround described in comment 4 & comment 5.  Luke, does that help, for your purposes?

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: diagnostic patch: warn on double-reflows

Here's a patch I'm using for diagnosis here, BTW -- spamming a NS_WARNING (in debug builds) if we have to reflow a flex item twice.  We may want to take this or a version of this (with the bool that guards these warnings off by default).

Comment 9

[Jon Keller]

Attachment: firefoxFlex3.html

Daniel,
Here's my result of distilling our real code down to as small as I can get it while still exhibiting the flex lag.  I've marked areas of the CSS and HTML as A, B, C, D, E, and F.

FF38 can handle (any 3 of ABCD)+E+F but not all of it.
FF39 can handle all of it.
But when I add repetition of areas E and F, FF39 can handle C+EEE+FFF or A+B+D+EEE+FFF, but it hangs and needs to be force-quit if you give it C+(any of ABD)+EEE+FFF.

In keeping with Luke's naming scheme I called it firefoxFlex3.html.

Thanks,
Jon

Comment 10

[Jon Keller]

I should elaborate on my definition above of "handle".
When FF38 is given A+B+C+D+E+F, resizing the window takes a second or two before it paints.
When FF39 is given A+B+C+D+EEE+FFF, just loading the page causes the browser to hang...I'm not sure for how long as I force-quit it after about a minute or two.

Comment 11

[Luke May]

Ok, sorry for the delay in the response, but I was tracking down a possibly related issue.  I have boiled down Jon's findings and here is the simplest example I can find to demonstrate the flex issues (however this new test-case doesn't necessarily have anything to do with flex basis, so this may need to be filed as a separate bug).  The issue specifically surrounds flexbox not getting along with absolute positioning.  I believe this test-case and the original test-case are related because there are no noticeable problems for me on firefox developer edition 38.0a2 (2015-03-21), but both test-cases have rendering delays for me on all the builds after.  I'll be going through and addressing your comments momentarily, but I wanted you to see this new test-case.  Again, thanks for looking at this!!

Comment 12

[Luke May]

Attachment: firefoxFlexAbsolute.html

Comment 13

[Luke May]

When I run the above test-case in firefox nightly 40.0a1 (2015-04-02), I get the following rendering delays at these depths:

depth: 19  --  delay: 18s
depth: 20  --  delay: 35s
depth: 21  --  delay: 67s
depth: 22  --  delay: 134s

Which means the rendering delay is essentially doubling for each layer of depth.  Since a depth os 22 is really not much at all to speak of it seems this might help identify the root of this problem better than my original test-case was able to.  Let me know if you think this should be separate bug; I'm only including it here because they seem to be related only in the fact that I am experiencing no problems with either test-case in the build from 3/21/2015, but I am experiencing issues with the later builds.

Comment 14

[Daniel Holbert [:dholbert]]

(In reply to Luke May from comment #11)
> The issue specifically surrounds
> flexbox not getting along with absolute positioning.  I believe this
> test-case and the original test-case are related because there are no
> noticeable problems for me on firefox developer edition 38.0a2 (2015-03-21),
> but both test-cases have rendering delays for me on all the builds after.

This makes sense -- absolute positioned children are considered as having a "relative height", because their height may depend on the parent (e.g. with "top:0; bottom:0; height:auto") in the same way that a percent-height child's height depends on the parent. So they set the NS_FRAME_CONTAINS_RELATIVE_HEIGHT flag which we use to disable the optimization in bug 1128354 (post-3/21).

I think I have an idea for a workaround that you should be able to use; basically, put each abspos element in an explicit wrapper-div, which you give "min-width:0; min-height:0". That should disable the need for two-pass layout at each level.

(Right now, each abspos element ends up generating an *anonymous flex item*, which is impossible for you to style because it's anonymous, so it has the default "min-height:auto" and needs a "measuring reflow", and since it has a relative-height child -- the anonymous flex item -- it ends up needing a final reflow too. If you instead wrap each anonymous flex item in an *explicit* flex item (with no styling beyond "min-height:0;min-width:0"), you should get the same behavior but without the 2-pass layout.)

I'll make some tweaked testcases locally and post them to demonstrate (& sanity-check) the workaround -- but if you want to give this a shot on your end, this information may be sufficient to get you started.

Comment 15

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #14)
> If you instead wrap each anonymous flex item in an
> *explicit* flex item (with no styling beyond "min-height:0;min-width:0"),
> you should get the same behavior but without the 2-pass layout.)

Sorry -- I meant to say: "If you instead wrap each *absolutely positioned element* [...]"

(And in case it's not clear -- by "explicit flex item", I just mean a <div>.)

Comment 16

[Luke May]

Interestingly, when I create your suggested wrapper around the the element with `position: absolute;` I still see the lag, but when I place the wrapper around the element with `position: relative;` then the lag disappears.  Is this what you meant or could this possibly be a bug?  I feel like `position: relative;` shouldn't really have any influence on whether or not an anonymous flex item is created around it.  Instead, though, an element with `position: absolute;` does seem like a candidate for an anonymous flex item to wrap it.  Can you confirm what I'm seeing for sanity?

Comment 17

[Daniel Holbert [:dholbert]]

I can confirm that, though I don't immediately know why that fixes it.

I think my initial workaround isn't sufficient because we still need a "measuring reflow" to resolve "flex-basis:auto" on each nested flex container. And then we need the final reflow because we still have flex items that are absolutely positioned with respect to the flex container.

An extended version of my comment 15 - 16 workaround does things, though -- add wrappers with
 min-height:0; position:relative
around each abspos element.

Then, the abspos elements are positioned with respect to that wrapper instead of the flex container (which is probably good, since abspos-stuff-tied-to-a-flex-container will likely change as a result of bug 874718, though perhaps not in a way that would matter here). And this means you no longer trip the "relative height child" check that forces the second reflow.

Comment 18

[Daniel Holbert [:dholbert]]

Attachment: static version of "firefoxFlexAbsolute.html" which hangs for a few seconds on load

Comment 19

[Daniel Holbert [:dholbert]]

Attachment: static version of "firefoxFlexAbsolute.html", with wrappers around abspos elements, which does *not* hang

Comment 20

[Daniel Holbert [:dholbert]]

hmm, that seems to break the sizing of your abspos overlays, though, so your workaround may be better.

Comment 21

[Jon Keller]

Daniel's descriptions in #18, #19 concur with what I see when I launch the files.

Comment 22

[Luke May]

Ok, we've tested thew wrapper div with `min-height/min-width: 0;` around the elements in our production codebase with `position: relative;` defined, and it does appear to fix the issue.  It does present an unfortunate side effect; because of the nature of the parent-child flexbox relationships this additional div sits between the parent flex item and child flex item, so an intermediate mechanism must be used.  But it is doable.  Thanks for the tips on the work-around Daniel!  Keep us posted on how to proceed with this bug. :)

Comment 23

[Daniel Holbert [:dholbert]]

Great, I'm glad you've got a functional workaround!

RE how to proceed here -- so, we've successfully isolated at least two distinct pathological scenarios For simplicity's sake, I'll spin off fresh bugs for each of those.  Then I (or someone else) can pick up either/both of those bugs at some point in the future, when time/prioritization permits (without having to re-skim through the backlog of diagnosis/discussion comments here to remember what's going on).

This bug should probably depend on those helper-bugs. (Or we could morph it into a tech evangelism "our site is hitting a flexbox performance issue" bug & resolve it as fixed, since you've got a workaround).

Comment 24

[Daniel Holbert [:dholbert]]

Good news! The latest "bad" testcase here (attachment 8588059 [details], "static version of "firefoxFlexAbsolute.html" which hangs for a few seconds on load"), seems to load just fine in current Firefox Nightly.

Here's a profile: https://share.firefox.dev/2Am3MAc
The reflow is only 2.2ms long, and there are only 2 samples that are in flexbox reflow at all.

I also tried the prior testcase ("firefoxFlexAbsolute.html") and dialed it up to "number of components: 70" with no noticeable delay at any of the increments (as compared to the >100s delays in comment 13 at only ~20 components).

Comment 25

[Daniel Holbert [:dholbert]]

Similarly, the next-older testcase ("firefoxFlex3.html") loads very quickly, with only a 3ms reflow: https://share.firefox.dev/3e0QYxq

And the next-older intended-to-be-slow testcase ("testcase 2 (partly reduced)"), which used to have multi-second reactions for hovers (per comment 3), now responds to hover events instantaneously, with <5ms reflow-time for those actions. Profile: https://share.firefox.dev/2YwfVuk

And the reporter's original testcase is now fixed, too -- I can put in values "200px" and "0px" or "1px" into its boxes, and then resize the window, and we re-render pretty rapidly (where previously we'd hang for a few seconds per comment 2).

So: bottom line, I think this is WORKSFORME, fixed by some change over the past 5 years.