Closed Bug 1178783 Opened 9 years ago Closed 6 years ago

Flex performance with nested flex containers is very poor compared to other browser

Categories

(Core :: Layout, defect)

38 Branch
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 1209697

People

(Reporter: warcraftthreeft, Assigned: dholbert)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0 Build ID: 20150525141253 Steps to reproduce: Test Case Size: 10 http://jsfiddle.net/xbg9xk7d/3/ Test Case Size: 20 http://jsfiddle.net/xbg9xk7d/4/ Test Case Size: 50 http://jsfiddle.net/xbg9xk7d/5/ You can change the value to like 15 or whatever value in the setup code to test different ideas. This seems to be a good way to test though. Might not want to open the 50 example except on other browsers. Javascript Setup Code: var Separator = function(separator, previous, next) { var vertical = separator.parent().css('flex-direction') == 'row'; separator.addClass(vertical ? 'vertical-separator' : 'horizontal-separator'); var mouseDown = function(mousedownEvent) { // If the user left clicks on a separator then start the resizing operation if (mousedownEvent.which == 1) { var cursor = vertical ? 'col-resize' : 'row-resize'; var page = vertical ? 'pageX' : 'pageY'; var dimension = vertical ? 'flex-basis' : 'flex-basis'; var oldPreviousDimension = parseInt(separator.prev().css(dimension), 10); var oldNextDimension = parseInt(separator.next().css(dimension), 10); $('html, body').css('cursor', cursor); var SetDimension = function(offset) { if (previous) { separator.prev().css(dimension, oldPreviousDimension + offset + 'px'); } if (next) { separator.next().css(dimension, oldNextDimension - offset + 'px'); } } var mouseMove = function(mousemoveEvent) { SetDimension(mousemoveEvent[page] - mousedownEvent[page]); } $(window).mousemove(mouseMove); // mouseup and blur catch every possible case where the mouse is released. This ensures the events are unregistered correctly. $(window).one('mouseup blur', function(mouseupEvent) { $(window).off('mousemove', mouseMove); SetDimension(mouseupEvent[page] - mousedownEvent[page]); $('html, body').css('cursor', ''); separator.one('mousedown', mouseDown); }); } else { separator.one('mousedown', mouseDown); } }; separator.one('mousedown', mouseDown); } var currentTag = $('body'); for (var i = 0; i < 50; ++i) { currentTag.append('<ul><li></li><li></li><li></li></ul>'); if (i % 2 == 0) { // Row currentTag.find('ul').addClass('horizontal'); } else { // Column currentTag.find('ul').addClass('vertical'); } currentTag = currentTag.find('li:last-child'); } $('.horizontal > li:nth-child(2)').each(function() { $(this).next().css('flex-basis', $(this).parent().width() / 2 + 'px'); Separator($(this), false, true); }); $('.vertical > li:nth-child(2)').each(function() { $(this).next().css('flex-basis', $(this).parent().height() / 2 + 'px'); Separator($(this), false, true); }); CSS: * { margin: 0; padding: 0; } html { height: 100%; display: flex; flex-flow: row nowrap; } body { flex: 1 1 0px; min-width: 0; display: flex; flex-flow: row nowrap; } ul { list-style: none; } .horizontal { flex: 1 1 0px; min-height: 0; display: flex; flex-flow: row nowrap; } .horizontal > li:first-child { flex: 1 1 0px; min-width: 0; background-color: #f00; } .horizontal > li:last-child { flex: 0 0 0px; min-width: 0; background-color: #0f0; display: flex; flex-flow: row nowrap; } .vertical { flex: 1 1 0px; min-width: 0; display: flex; flex-flow: column nowrap; } .vertical > li:first-child { flex: 1 1 0px; min-height: 0; background-color: #0ff; } .vertical > li:last-child { flex: 0 0 0px; min-height: 0; background-color: #f0f; display: flex; flex-flow: row nowrap; } .horizontal-separator { flex: 0 0 5px; cursor: row-resize; background-color: #fff; user-select: none; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; } .vertical-separator { flex: 0 0 5px; display: inline-block; cursor: col-resize; background-color: #fff; user-select: none; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; } Actual results: It's very slow, to the point of locking up the browser, compared to every other browser. Other browsers are unaffected by the number of nested flex containers, but Firefox gets progressively slower fast. Expected results: Should perform like Chrome, IE11 or Microsoft Edge which have no issue with nested flex containers.
Component: Untriaged → Layout
Product: Firefox → Core
Thank you for the bug report. Daniel, I'm still seeing slowness here even on nightly, so this looks different from bug 1142686...
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dholbert)
Attached file testcase 1 (WARNING: hangs firefox) (deleted) —
I'll take a look in the next day or so. In the meantime, here's a partly reduced testcase with no JS required.
Assignee: nobody → dholbert
the testcase opens without any jank for me.. Should this be closed?
Flags: needinfo?(dholbert)
That's Daniel's call. Did you test the original testcases, or the one from comment 2?
Flags: needinfo?(dholbert)
Nice! Thanks for the heads-up -- this does indeed seem to be fixed! The testcase from comment 2 renders pretty much instantly for me, and "Test Case Size: 50" (the biggest one from comment 0) loads in a few seconds. I'm seeing if I can track down when/where this was fixed... I'll close after I've done that.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(dholbert)
Resolution: --- → DUPLICATE
In old (before-the-fix) builds, the attached testcase (from comment 2) takes at least 5 minutes to load (I gave it that long and it was still hanging). So I think it's suitable as a crashtest [i.e. it'd trigger failures due to timeouts if we regressed this.] Hence, I'll go ahead and push it as a crashtest.
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/82ed5b8d9f61 Add crashtest for (now-fixed) flexbox hang bug. (no review, test-only)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: