1890358 - Tab freezes while viewing GitHub PR changes

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect, P2)

Description

[Claas Augner [:claas]]

I looked at https://github.com/mdn/data/pull/719/files and clicked on "Load diff..." when the tab started to freeze and I could no longer click any element on the web page nor reload the page with CTRL + R.

While this happened, I captured the following profile, and the tab got responsive again after a minute or so: https://share.firefox.dev/3PVzB6a

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::CSS Parsing and Computation' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Gregory Pappas [:gregp]]

Seeing lots of accessibility in that profile

Comment 3

[Eitan Isaacson [:eeejay]]

This looks like we are choking up when constructing the cache after tree mutations. The hot spot seems to be TextLeafPoint::FindNextLineStartSameLocalAcc and the time is spent almost exclusively in nsTextFrame::GetRenderedText.

Jamie, if nothing immediately comes to mind I can spend some time on this too.

Comment 4

[James Teh [:Jamie]]

Nothing comes to mind immediately. The changes in bug 1770981 should mean that we use a line cursor when calling GetRenderedText. As long as we're mostly walking the tree in content order, GetRenderedText should be relatively efficient. Because mutation events are stored in a linked list, this should mostly be in content order... unless the page is adding nodes in random order, but that seems highly unlikely.

If you have some cycles to look at this, that'd be awesome.

Comment 5

[James Teh [:Jamie]]

See also bug 1771060 and bug 1771579, which both relate to keeping things in order to ensure we get maximum benefit from the line cursor, but I doubt they're relevant here because they aren't related to mutation events (which are already ordered).

Comment 6

[James Teh [:Jamie]]

:claas, I can't reproduce this. Loading the diff happens within a second for me. Did this particular diff get smaller due to a PR update? Can you still reproduce this?

Comment 7

[Nathan LaPré]

I've tried to reproduce this a bunch of times with no luck. Tested on Windows since I don't have a Mac, though I haven't noticed anything in the profile that points to a platform-specific issue. I agree with Eitan's analysis of the profile.

Did this particular diff get smaller due to a PR update?

FWIW, it doesn't look to me like there's any significant difference in size between commits on this PR. I couldn't reproduce it on the first commit alone, either.

Comment 8

[Morgan Reschenberg [:morgan]]

I can't reproduce this on macOS either :(

Comment 9

[James Teh [:Jamie]]

I guess something changed here and this is now fixed, so I'm going to close this. If the problem persists, please reopen.