Closed Bug 1228176 Opened 6 years ago Closed 2 years ago

offsetWidth/offsetHeight calculation is slow in vertical flexbox, with flex item that has default "min-height:auto" and tons of child nodes

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: dkitt, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:42.0) Gecko/20100101 Firefox/42.0
Build ID: 20151030084315

Steps to reproduce:

I found this when debugging an issue with CodeMirror (https://codemirror.net/) having performance problems on a site I develop.  After fiddling with CSS for a bit, it seemed like layout calculations within a flexbox were the culprit.

I created a JSFiddle that shows performance differences between browsers.  Basic setup is a span within two levels of flexboxes changes content and the time to calculate the offsetWidth/Height afterward is measured.  There are 100k empty divs in another flex element in the outer flexbox.
Link: https://jsfiddle.net/zsfec3dL/4/

The performance difference between FF and Chrome is easily reproducible in the above JSFiddle.  I was able to see a significant difference on both Ubuntu and OS X.


Actual results:

On FF42, the time to calculate offsetWidth/offsetHeight is 500-600ms.


Expected results:

On latest Chrome, the time to calculate offsetWidth/offsetHeight is 0-1ms.  This difference seems consistent with the issue I was having with CodeMirror.
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Flags: needinfo?(dholbert)
Product: Firefox → Core
Version: 42 Branch → Trunk
Keywords: perf
Things get much faster (7-8ms instead of 300ms, on my system) if I add "min-height: 0;" to the .flex-inner-remainder style rule.

Demo with that change: https://jsfiddle.net/zsfec3dL/5/

(dkitt, if you're hitting this in the real world, please consider/try that as a workaround.)

This indicates that the slowdown is from us resolving "min-height:auto" on that element. Not sure why it's so significant of a slowdown, though, given that there shouldn't be any exponential blowup from nesting here...
Thanks for the suggestion, Daniel!  Your workaround helps out a lot with our real world performance issue.  It is still a little slower than Chrome, but definitely usable, where before it was borderline unusable.  FWIW, it was also important that the flex-basis was set to something other than auto, as well as min-height.

I think the "nested flexbox" I added to the title is a red herring.  Removing the "flex-inner" element from my original fiddle has very similar timing behavior, with just one flexbox: https://jsfiddle.net/zsfec3dL/6/

One interesting observation is that the slowdown (in the fiddle, at least) seems directly proportional to the number of elements added to the "flex-inner-remainder" element.  Halving the number of elements added in the loop in the fiddle seems to halve the time to compute offsetHeight/offsetWidth, and doubling doubles the time, etc.
Summary: offsetWidth/offsetHeight calculation is slow in nested flexbox → offsetWidth/offsetHeight calculation is slow in vertical flexbox, with flex item that has default "min-height:auto" and tons of child nodes
Just for the record I want to point out that the example adds the elements to the DOM in a very inefficient way:

1. If adding multiple elements, it is almost always a good idea to use a document fragment as an intermediate parent element.
2. The remainderElement should be searched only once outside the loop. In the original implementations 99999 unnecessary calls to getElementsByClassName() are made.

Both changes have nothing to do with flexbox, but on my machine the speed up the demo by a factor 300. 

Here is the fiddle with the changes applied: https://jsfiddle.net/zsfec3dL/7/

jsfiddle has evolved its fancy UI such that it doesn't work with ~2015-era Firefox anymore.

For the record, here's the original testcase ( https://jsfiddle.net/zsfec3dL/4/ ) as a standalone attachment, which can be loaded successfully in old builds as well as in new builds.

Using the attached testcase:

  • Firefox Nightly 2015-11-26 reports "Time: 357"
  • Firefox Nightly 2020-06-12 reports "Time: 7"
  • Chrome 83 (current version) also reports "Time: 7" for me (note it takes a long time to load, but probably spinning its wheels doing unrelated stuff, which maybe is what comment 3 is discussing).

So we're on-par with Chrome now, and way better than we were when this bug was filed.
--> Calling this WORKSFORME.

Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(dholbert)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.