1762796 - slowdown on a website due to slow Element.classList.values() (which slows down uBlock)

Status: RESOLVED FIXED

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

Description

[drsdk102]

User Agent: Mozilla/5.0 (Android 11; Mobile; rv:98.0) Gecko/98.0 Firefox/98.0

Steps to reproduce:

1. Install ublock origin stable 1.42, update its filters
2. visit website https://moviewatchonline.com.pk/
3. after loading website completely, page becomes unresponsive followed by firefox dialog ublock origin is slowing down the firefox,to speedup your browser, stop that extension
4. addon developer is contacted here https://github.com/uBlockOrigin/uBlock-issues/discussions/2076

Actual results:

firefox becomes unresponsive

Expected results:

firefox should run normally

Comment 1

[Emilio Cobos Álvarez (:emilio)]

DOMTokenList is DOM code. I can take a look anyways. It seems from the profile that nsDOMTokenList::RemoveDuplicates takes most of the time, because it can be quadratic when there are duplicate tokens.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: Reduced-ish test-case.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1762796 - Deduplicate TokenList values at parse time. r=smaug

This prevents DOMTokenList from being quadratic / needing to iterate
over all values on the indexed getter, which can get slow with long
class lists.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1762796 - Deduplicate TokenList values faster. r=smaug

Remember whether we have already de-duplicated them once and avoid doing
that again.

This is an alternative approach that doesn't add overhead to attribute
setting in the general case.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/49b1e879a2c9
Deduplicate TokenList values faster. r=smaug

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/49b1e879a2c9

Comment 7

[Ben Campbell]

Sorry to post on a closed bug, but I've got a loose end:

It looks to me that the patch here makes it so code can't use nsAttrValue without having a previous #include "nsAtom.h". This is because the new struct AttrAtomArray has inline code which requires the full nsAtom definition rather than just the forward declaration.

It caused some (easily fixed) build bustage over in Thunderbird (Bug 1763034). I've landed a fix there, but it seems like it might be more correct to replace the nsAtom forward declaration in nsAttrValue.h with #include "nsAtom.h"?
Anyway, I'll post my proposed patch and you can decide :-)

Comment 8

[Ben Campbell]

Attachment: Bug 1762796 - Allow nsAttrValue use without requiring a prior #include "nsAtom.h". r=emilio

Comment 9

[mac198442]

This also fixes SeaMonkey builds.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/675457b38270
Allow nsAttrValue use without requiring a prior #include "nsAtom.h". r=emilio

Comment 11

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/675457b38270