1726124 - high cpu usage and rendering stops for long period

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

Attachment: screenshot

Steps to reproduce.

1. open attached html
2. when tab spinner stops spinning, scroll down

Actual results:
high cpu usage and rendering stops for long period.
Profiler : https://share.firefox.dev/3CWZrhY
Screenshot: see attached

Setting accessibility.force_disabled to 1 fixes the issue.

Comment 1

[Alice0775 White]

Attachment: ユニコード表.html

Comment 2

[Alice0775 White]

Attachment: about:support

Comment 3

[Eitan Isaacson [:eeejay]]

This looks serious enough that it should be looked at soon.

Comment 4

[James Teh [:Jamie] (away returning 28 Jan)]

I ran into a similar perf problem with a similar profile just now testing a huge Searchfox file on an older computer.

The interesting thing about this profile is that we spend a lot of time in CoalesceEvents, called by PushEvent, called by PruneOrInsertSubtree. The only thing I can see in PruneOrInsertSubtree that fires an event is the firing of EVENT_TABLE_STYLING_CHANGED.

My suspicion is that a full reflow occurs, so PruneOrInsertSubtree ends up running on every node, including every table, row and cell. On a large table, that means we'll have a huge number of EVENT_TABLE_STYLING_CHANGED events, and since we're firing them on individual cells, they can't be de-duplicated. That results in a huge event queue which CoalesceEvents repeatedly walks through.

As I understand it, this event is only used on Mac and then only to invalidate the layout guess for the entire table. Eventually, we won't even need that, since this will be handled by CTW.

Morgan, is there any reason we can't fire this on the table instead of the rows/cells? We already have to walk to the table anyway for CTW and that's far cheaper than building a huge queue of events which we repeatedly try (and fail) to coalesce.

Comment 5

[Morgan Reschenberg [:morgan]]

(In reply to James Teh [:Jamie] from comment #4)

I ran into a similar perf problem with a similar profile just now testing a huge Searchfox file on an older computer.

The interesting thing about this profile is that we spend a lot of time in CoalesceEvents, called by PushEvent, called by PruneOrInsertSubtree. The only thing I can see in PruneOrInsertSubtree that fires an event is the firing of EVENT_TABLE_STYLING_CHANGED.

My suspicion is that a full reflow occurs, so PruneOrInsertSubtree ends up running on every node, including every table, row and cell. On a large table, that means we'll have a huge number of EVENT_TABLE_STYLING_CHANGED events, and since we're firing them on individual cells, they can't be de-duplicated. That results in a huge event queue which CoalesceEvents repeatedly walks through.

As I understand it, this event is only used on Mac and then only to invalidate the layout guess for the entire table. Eventually, we won't even need that, since this will be handled by CTW.

Morgan, is there any reason we can't fire this on the table instead of the rows/cells? We already have to walk to the table anyway for CTW and that's far cheaper than building a huge queue of events which we repeatedly try (and fail) to coalesce.

I don't think there's any reason we can't fire them on the table.
We use the event on non-table moz accessible's right now, but only to invalidate their containing table.
I think removing that bit of code and instead relying on this one should be sufficient.

Comment 6

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1726124: [Part 2] Only fire TABLE_STYLING_CHANGED events on tables, not table parts r?Jamie

Comment 7

[Morgan Reschenberg [:morgan]]

Attachment: Bug 1726124: [Part 1] Generalise nsAccUtils::TableFor for table parts r?Jamie

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c82b5407ff3
[Part 1] Generalise nsAccUtils::TableFor for table parts r=Jamie
https://hg.mozilla.org/integration/autoland/rev/1c5f2f67382b
[Part 2] Only fire TABLE_STYLING_CHANGED events on tables, not table parts r=Jamie

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfe1912f7a46
Backed out 5 changesets (bug 1726124, bug 1786086) for failures on browser_table.js. CLOSED TREE

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/79e305189497
[Part 1] Generalise nsAccUtils::TableFor for table parts r=Jamie
https://hg.mozilla.org/integration/autoland/rev/6e82360f0f0c
[Part 2] Only fire TABLE_STYLING_CHANGED events on tables, not table parts r=Jamie

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/79e305189497
https://hg.mozilla.org/mozilla-central/rev/6e82360f0f0c

Comment 12

[Monica Chiorean]

I can not reproduce the issue on Win10 x64 using Firefox build 93.0a1(20210816215309) and the html file from description.
Can you please confirm issue is not reproducing on latest Beta 106.0b8(https://archive.mozilla.org/pub/firefox/candidates/106.0b8-candidates/) or Nightly 107.0a1(https://archive.mozilla.org/pub/firefox/nightly/2022/10/2022-10-05-21-51-13-mozilla-central/)? Thank you.

Comment 13

[Alice0775 White]

I manage to reproduce this issue in Nightly 93.0a1(20210816215309) Windows10 w/ ATOK insight.

And I verified fix in Nightly107.0a1(20221005215113) and Firefox106.0.0b8.