Bug 98982 – Setting className attribute causes slow performance

• Component: DOM
• Status: UNCONFIRMED

[Brad Vogel]

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/536.26.14 (KHTML, like Gecko) Version/6.0.1 Safari/536.26.14

Steps to reproduce the problem:
1. Run the attached test case.
2. View the console.

In the console you'll see:
with setting className: 342.205ms
without setting className: 1.680ms

What went wrong?
Setting the className to it same value should be a no-op. However, webkit sets the node as dirty even though the className is the same. It would be a nice performance enhancement if webkit compared the new value to the old value before setting the dirty flag to re-render the node.

[Brad Vogel]

Created attachment 168131 [details]
The test case

[Alexey Proskuryakov]

Do you expect this pattern to be used on actual web sites? If this never happens, then even the tiny overhead of checking whether className didn't change would be detrimental.

[Ryosuke Niwa]

My expectation is that things like this happen a lot in a large Web where there are multiple layers of abstractions. Having said that, it seems like this can be easily resolved in script side as well. My gut tells me we should implement this optimization just like we for setAttribute.

[Takashi Sakamoto]

Created attachment 169330 [details]
WIP

[Brad Vogel]

Yes, this is extremely useful. A lot of Javascript libraries implement a naive addClass() function like:
function addClass(newClass){
     element.className = element.className + ' ' + newClass;
}

If newClass already exists in element.classList, then it'll still dirty the node.

[Brad Vogel]

Cross-filed for Blink at https://code.google.com/p/chromium/issues/detail?id=226854

[Erik Arvidsson]

(In reply to comment #5)
> Yes, this is extremely useful. A lot of Javascript libraries implement a naive addClass() function like:
> function addClass(newClass){
>      element.className = element.className + ' ' + newClass;
> }

This case will always be slow since it is always changing the attribute and the list.

[Brad Vogel]

In the above example, I was hoping that if the CSS class being added happened to already be on the element, no relayout would occur. A lot of JS libraries are naive and always append the class to the className attribute. Also, a lot of web developers use these libraries to add classes that already exist on the element, usually in a loop (e.g. striping a table). Other attributes such as those set by setAttribute have logic to only dirty the DOM if needed. Fixing className would provide nice symmetry.

[Antti Koivisto]

We should already have do-nothing optimization for adding the same class again. Have you verified recently it is not working?

[Erik Arvidsson]

(In reply to comment #9)
> We should already have do-nothing optimization for adding the same class again. Have you verified recently it is not working?

The case that Brad is talking about is changing the class attribute but not in a way that impacts style resolution. For example `element.className += ' '`. This would of course impact `[class='abc']` selectors but I believe we already keep track of attribute selectors.

[Antti Koivisto]

(In reply to comment #10)
> (In reply to comment #9)
> > We should already have do-nothing optimization for adding the same class again. Have you verified recently it is not working?
> 
> The case that Brad is talking about is changing the class attribute but not in a way that impacts style resolution. For example `element.className += ' '`. This would of course impact `[class='abc']` selectors but I believe we already keep track of attribute selectors.

We do put some effort into optimizing class list change. Check Element::classAttributeChanged() and checkSelectorForClassChange() to see if there is something obvious we could still do.

[Brad Vogel]

Created attachment 196683 [details]
New test case with className += ' '

> The case that Brad is talking about is changing the class attribute but not in a way that impacts style resolution.

Yes, that's correct. I added a new test case with the `className += ' '` example. My results from the console in Version 28.0.1466.0 canary:
    with setting className in a way that does not affect styles: 227.365ms
    without setting className: 9.272ms

I'd expect the "with setting className" performance to be much closer to "without setting className" since it's not changing styles or requiring a relayout.

[Ahmad Saleem]

*** Safari 15.6 on macOS 12.5 ***

with setting className in a way that does not affect styles: 230.043ms
without setting className: 0.814ms


*** Firefox Nightly 105 ***

with setting className in a way that does not affect styles: 63ms
without setting className: 1ms

*** Chrome Canary 106 ***

with setting className in a way that does not affect styles: 276.712890625 ms
without setting className: 2.52392578125 ms

____________

Safari is faster than Chrome but slow than Firefox Nightly (might be due to Rust based Stylo). Might be something worth investigating. Thanks!