1575054 - Investigate why maximum nested balancing levels of 2 doesn't improve the performance after enabling column-span

Status: RESOLVED FIXED

(Core :: Layout: Columns, defect, P3)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

In bug 725376, we limited the maximum level of nested balancing columns to 2 levels.

With column-span enabled, the maximum balancing level doesn't seem to help. It takes long time to load this test case https://bug725376.bmoattachments.org/attachment.cgi?id=595472

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

After bug 1575106, the testcase starts to hang on Firefox Nightly (2019-08-27), which takes about 14.5 seconds to load on my Linux machine. We need to fix this before shipping column-span.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1575054 - Disable column balancing for nested multicol containers of depth more than two.

That is, for the multicol container of depth two and more, we lay them
out by using "column-fill:auto" and "column-count:1".

I've check bug 725376 comment 9 for the previous approaches. Thanks to
bug 1555818, this solution is feasible because the fragmentation with
"column-fill:auto" is now possible.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Partial-screenshot-with-patches-in-this-bug-and-bug1583639.png

This screenshot is taken from my local debug build with my patch in this bug and in bug 1583639 for the test case in comment 0. It should look identical if you load it in current Nightly (warning: it can take a while to load).

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Partial-screenshot-without-patches-in-bug1583639.png

For comparison, this screenshot is taken with the patch applied in this bug, but without the patches in bug 1555818. The layout result is similar to (2) or (3) described in bug 725376 comment 9

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e10a3c249943
Disable column balancing for nested multicol containers of depth more than two. r=dbaron

Comment 6

[Cristina Coroiu [:ccoroiu]]

Backed out changeset e10a3c249943 (bug 1575054) for crashtest failures at reftest/tests/layout/generic/crashtests/812893.html

Backout: https://hg.mozilla.org/integration/autoland/rev/2bc477bd65f2a0169572f46528296568cadd0793

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e10a3c2499439232e95ecff9b90c3689c48d7ae2

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268649529&repo=autoland&lineNumber=70884

task 2019-09-26T20:26:26.700Z] 20:26:26 INFO - REFTEST TEST-PASS | file:///Users/cltbld/tasks/task_1569528958/build/tests/reftest/tests/layout/generic/crashtests/812893.html | (LOAD ONLY)
[task 2019-09-26T20:26:26.701Z] 20:26:26 INFO - REFTEST TEST-END | file:///Users/cltbld/tasks/task_1569528958/build/tests/reftest/tests/layout/generic/crashtests/812893.html
[task 2019-09-26T20:26:26.701Z] 20:26:26 INFO - ++DOMWINDOW == 72 (0x1434a1000) [pid = 1644] [serial = 7534] [outer = 0x11a042020]
[task 2019-09-26T20:26:26.704Z] 20:26:26 INFO - REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1569528958/build/tests/reftest/tests/layout/generic/crashtests/812893.html | assertion count 26 is more than expected 4 to 13 assertions

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Oops. The assertion count of 812893.html increases to 26 on MacOS and Windows. That's bug 1581653. I'll update the test expectation and reland.

Comment 8

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1575054 - Disable column balancing for nested multicol containers of depth more than two.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d55ef6322184
Disable column balancing for nested multicol containers of depth more than two. r=dbaron

Comment 10

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/d55ef6322184