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)
Tracking
()
RESOLVED
DUPLICATE
of bug 1209697
People
(Reporter: warcraftthreeft, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/html
|
Details |
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.
Reporter | ||
Updated•9 years ago
|
Component: Untriaged → Layout
Product: Firefox → Core
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
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
Assignee | ||
Updated•8 years ago
|
Blocks: flexbox-perf-issues
Comment 3•6 years ago
|
||
the testcase opens without any jank for me.. Should this be closed?
Flags: needinfo?(dholbert)
Comment 4•6 years ago
|
||
That's Daniel's call. Did you test the original testcases, or the one from comment 2?
Flags: needinfo?(dholbert)
Assignee | ||
Comment 5•6 years ago
|
||
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.
Assignee | ||
Comment 6•6 years ago
|
||
Fix range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f985243bb630b2c78cd57731c8d8ab191aa09527&tochange=bf15d4078c2a6db7df37ab466d28a1e075c9eb4d
--> fixed by bug 1209697
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(dholbert)
Resolution: --- → DUPLICATE
Assignee | ||
Comment 7•6 years ago
|
||
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)
Comment 9•6 years ago
|
||
bugherder |
You need to log in
before you can comment on or make changes to this bug.
Description
•