1943746 - Testcase generating large number of editable elements is 6x slower in Nightly. The time increases superlinearly with increase in number of element (45x slower with 3x element count)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mayank Bansal]

Attachment: content editable.HTML

Open attached testcase
Enter 50000 and click on generate

Nightly: https://share.firefox.dev/3WwTJPo (5.4s)
Chrome: https://share.firefox.dev/40N2g3g (0.9s)

Maybe something to improve?
Testcase is generated using chatgpt.

Comment 1

[Mayank Bansal]

N=150000
Nightly: https://share.firefox.dev/3E4NFqY (135s)
Chrome: https://share.firefox.dev/4audQUj (3s)

Comment 2

[Mayank Bansal]

Attachment: about:support

Comment 3

[Mayank Bansal]

Hmm, this isnt really content-editable. It is just editable content.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like checking spellcheck attr is too slow due to checking ancestors and it may mishit especially in testcases.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

PresShell::GetSelection does not need to use RefPtr.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Should we even spellcheck if the element is not focused?

In a test-case like:

data:text/html,<div contenteditable>foo bar baz</div><div contenteditable autofocus>foo bar baz</div>

Chrome only shows the spellcheck marks on the focused node. That would basically fix this, right?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Another alternative would be to use SelectNodeContents rather than SelectNode. Which should be faster and also work, right?

Comment 8

[Mayank Bansal]

Surprising to me that this testcase highlights very different things compared to the "input element =text" testcase in bug 1943230

Comment 9

[Emilio Cobos Álvarez (:emilio)]

They use very different code-paths, so not that crazy :)

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Looks like we can delete them:

• https://searchfox.org/mozilla-central/rev/a965e3c683ecc035dee1de72bd33a8d91b1203ed/editor/libeditor/HTMLEditor.cpp#4964
• https://searchfox.org/mozilla-central/rev/a965e3c683ecc035dee1de72bd33a8d91b1203ed/editor/libeditor/HTMLEditor.cpp#5044

Comment 11

[Mayank Bansal]

The improvements from bug 1943832 are glorious.

150000 elements : https://share.firefox.dev/4gdPiAc (4.1s) !
So from 135s -> 4.1s (32x faster)

Even better, in both the before and after profiles the spellchecking stuff takes ~3s . If you remove that, the after profile takes 971ms!
So 131s -> 971ms (134x faster!!)

Comment 12

[Mayank Bansal]

Here is a partial profile for 10x the testcase 1500000 elements:
https://share.firefox.dev/3PR3nIZ

This is the breakdown:

• Actual generation: 4.8s
• Reflow: 5.3s.
• Spellcheck stuff: 53s (partial)

Anything that can be fixed for the long tail of spellcheck stuff? I can file a new bug if needed.
Else, is there anything more to fix here?

Comment 13

[Mayank Bansal]

ni? for comment 12.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

I think it's worth filing a bug for the spellcheck stuff... At the very least to try to spellcheck less (comment 6), but also there's some low hanging fruilt there (e.g. it seems mRealWords could benefit from being an AutoTArray, etc).

Comment 15

[Mayank Bansal]

Filed bug 1944275 for comment 14.

Comment 16

[Mayank Bansal]

Partial Profile with 1500000 elements: https://share.firefox.dev/3WNC1HD

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Mayank Bansal from comment #16)

Partial Profile with 1500000 elements: https://share.firefox.dev/3WNC1HD

Looks like there are only issues of the heavy allocations of the spellchecker. I don't know whether it's reasonable cost or not.

Comment 18

[Mayank Bansal]

The only dependency is Bug 1944275, which doesnt have a clear path forward. But it can be independently tracked.

I will close this bug as FIXED. Thanks for all the work on this Masayuki and Emilio!