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

Status: RESOLVED DUPLICATE of bug 1209697

(Core :: Layout, defect)

Description

[warcraftthreeft]

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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Thank you for the bug report.

Daniel, I'm still seeing slowness here even on nightly, so this looks different from bug 1142686...

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (WARNING: hangs firefox)

I'll take a look in the next day or so. In the meantime, here's a partly reduced testcase with no JS required.

Comment 3

[Mayank Bansal]

the testcase opens without any jank for me.. Should this be closed?

Comment 4

[Boris Zbarsky [:bzbarsky]]

That's Daniel's call.  Did you test the original testcases, or the one from comment 2?

Comment 5

[Daniel Holbert [: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.

Comment 6

[Daniel Holbert [:dholbert]]

Fix range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f985243bb630b2c78cd57731c8d8ab191aa09527&tochange=bf15d4078c2a6db7df37ab466d28a1e075c9eb4d

--> fixed by bug 1209697

Comment 7

[Daniel Holbert [:dholbert]]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/82ed5b8d9f61
Add crashtest for (now-fixed) flexbox hang bug. (no review, test-only)

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/82ed5b8d9f61