1939344 - "Layout Cleanup" and the subsequent CC takes multiple seconds on a large page

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Mayank Bansal]

Go to https://www.emathhelp.net/calculators/linear-algebra/matrix-multiplication-calculator/
In "Size of the first matrix:", put the value as 400 rows and 400 columns
Wait for the page to create a 400x400 matrix.
Once the page loads and is stable, click on the "Reload" button on the toolbar.

Nightly to reload the page : https://share.firefox.dev/4fER4tR
Chrome to reload the page: https://share.firefox.dev/3PckOU7
(For the record, Nightly to create the 400x400 matrix: https://share.firefox.dev/3VZtYal)

Large-ish time spent in "Layout cleanup" and the subsequent cc on page reload. Maybe something to improve?

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Profile with a 400x400 first matrix, and a 400x200 second matrix : https://share.firefox.dev/4guAGNR

Profile if you just close the tab (instead of reloading): https://share.firefox.dev/3ZRjGKx

Comment 3

[Daniel Holbert [:dholbert]]

Thanks! I can reproduce. Here's a profile with screenshots, and starting from the initial pageload:
https://share.firefox.dev/3PxfWZR

Notably, we spend a bit more time on frame construction to build the 400x400 matrix (3.5s in frame construction, shown as a long light-blue bar in my profile). The "Layout Cleanup" at reload time is simply us spending time (2.6s) destroying all of those frames that we created.

Here's my profile, zoomed to focus on just that part (inside of "Layout tree destruction"): https://share.firefox.dev/4gP5ybX

There are 2384 samples in layout tree destruction there. Of those:

• 2161 samples (91%) are in nsTextControlFrame::Destroy (here's the profile focused on those: https://share.firefox.dev/4gP4FjD )
• Most of those samples (1372) are in mozilla::TextControlState::UnbindFromFrame creating some stuff, in mozilla::dom::HTMLInputElement::GetControllers, which spends most of its time in nsBaseCommandController::CreateEditorController and nsBaseCommandController::CreateEditingController (note: those two functions have nearly the same name, differing only in "editOR" and "editING"). Here's the profile filtered to see those: https://share.firefox.dev/3WhSLqi
• It seems a bit unexpected/unfortunate that we're spending time creating stuff as part of tearing down the world for this document.

Comment 4

[Mayank Bansal]

I thought that firefox was storing text in cells in case the user restores the tab after closing, or closes the browser and does a session-restore, or something.

Comment 5

[Daniel Holbert [:dholbert]]

I spun off bug 1941140 for TextControlState::UnbindFromFrame doing allocations here. (There's other measurable slowness here too, so I don't want to hyper-focus on just that one part in this bug; though it seems to be the bulk of the slowness.)

(In reply to Mayank Bansal from comment #4)

I thought that firefox was storing text in cells in case the user restores the tab after closing, or closes the browser and does a session-restore, or something.

Ah, for bfcache (back/forward cache)? I hope/suspect that's not the thing causing slowness here. In my case at least, all the cells are empty/unmodified, so I don't think we need to remember their values (and I don't see evidence in the profile that that's what we're doing).

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Mayank Bansal from comment #0)

Large-ish time spent in "Layout cleanup" and the subsequent cc on page reload.

Note, I'm not sure there's any direct connection between the Layout cleanup and the subsequent CC -- I think they just are both cleanup tasks that happen for different pieces of the document, and one has to happen first, but the CC'd stuff isn't e.g. further layout cleanup. And in my profile in comment 3, for example, there's no such long CC before the reload happens (though there is some CC'ing after the reload). So: that CC might need its own distinct investigation and might be DOM/JS objects. (I don't have time to look in detail at the moment.)

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:jwatt, could you have a look please?

For more information, please visit BugBot documentation.

Comment 8

[Mayank Bansal]

Profile with bug 1941140 fixed: https://share.firefox.dev/3PIeqnG (1 second)
So we moved from 2.5s -> 1s .

(In reply to Daniel Holbert [:dholbert] from comment #5)

I spun off bug 1941140 for TextControlState::UnbindFromFrame doing allocations here. (There's other measurable slowness here too, so I don't want to hyper-focus on just that one part in this bug; though it seems to be the bulk of the slowness.)

Daniel, what else is fixable here?

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Mayank Bansal from comment #8)

So we moved from 2.5s -> 1s .

Hooray!

Daniel, what else is fixable here?

I'm not sure much of anything. Digging into your comment 8 performance-profile, the only substantial stuff I'm seeing is actual deallocation/cleanup work, none of which is trivially avoidable.

I just compared this between Firefox and Chrome side-by-side and I'm not visually seeing much of a difference in reload time. Are you seeing much difference? If not, maybe we can consider this fixed (to the extent that there's anything fixable) by bug 1941140?

Comment 10

[Mayank Bansal]

I will go ahead and call this fixed.
Thanks Daniel for investigating!