1355472 - [meta] Speedometer backbone.js footer updating code is slower than in Chrome/Safari

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

Attachment: Testcase

For the attached testcase I get the following numbers on Mac:

Safari:  14 ms
Chrome:  17 ms
Nightly: 40 ms

We're losing 2 ms or so on the innerHTML, the rest is this:

    $('#filters li a')
        .removeClass('selected')
        .filter('[href="#/' + '' + '"]')
        .addClass('selected');

I haven't really profiled this yet, but there's probably both jQuery JS overhead and DOM stuff.

Comment 1

[Andrew Overholt [:overholt]]

Jan, can you see if this is still relevant given recent improvements? If it's only innerHTML left, we're tracking that elsewhere so we could probably close this JS bug.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Andrew Overholt [:overholt] from comment #1)
> Jan, can you see if this is still relevant given recent improvements? If
> it's only innerHTML left, we're tracking that elsewhere so we could probably
> close this JS bug.

This is still relevant. As comment 0 mentions, this is mostly unrelated to the innerHTML.

Comment 3

[Sean Stangl [:sstangl]]

I modified the testcase to iterate 10,000 times, and added markers via window.performance.mark() around the onload() handler.

Profile is here: http://bit.ly/2rSvzCx

The profile is zoomed into the area between the two markers (taking 1.7 seconds), but note that the tab has a delay of 4.9 seconds, so the onload() handler is only a small portion of that.

There is a "scripts" block immediately following the execution of the onload() handler that takes 1.4 seconds of the load time, very long. That is caused by the UBlock Origin add-on. I left that in because the add-on is popular and that seems significant.

Comment 4

[Sean Stangl [:sstangl]]

It's worth noting that in the actual onload() handler, virtually all of the time is spent in the DOM.

JS is only hot for UBlock Origin's safeObserverHandler().

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Could you share a profile without any random addon stuff, or perhaps upload your modified testcase?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

The testcase doesn't have any MutationObserver, but the profile from comment 3 shows that there is. I assume that is coming from the addon, so the profile isn't too useful.

Comment 7

[Sean Stangl [:sstangl]]

Attachment: 10k iterations testcase

Longer-running testcase. Profile with no add-ons: http://bit.ly/2sChTck.

Comment 8

[Jan de Mooij [:jandem]]

smaug, maybe you can profile the DOM bits here?

Comment 9

[Boris Zbarsky [:bzbarsky]]

So one weird thing I see in the profile: Why is setting .className on an <a> causing us to reget the link href when getting link state?  That is, why didn't we reget it earlier, when it changed?  And if it did not change, why do we not have it cached?

Comment 10

[Jan de Mooij [:jandem]]

(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #9)
> So one weird thing I see in the profile: Why is setting .className on an <a>
> causing us to reget the link href when getting link state?  That is, why
> didn't we reget it earlier, when it changed?  And if it did not change, why
> do we not have it cached?

NI smaug to make sure we look into this.

Comment 11

[Henri Sivonen (:hsivonen)]

Looks like we should bake the content creator function pointers into nsHtml5ElementName and transfer them the node creation point from there. This would avoid a second (on top of looking up nsHtml5ElementName itself) lookup through the set of possible elements.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Jan de Mooij [:jandem] from comment #10)
> (In reply to Boris Zbarsky [:bz] (if a patch has no decent message,
> automatic r-) from comment #9)
> > So one weird thing I see in the profile: Why is setting .className on an <a>
> > causing us to reget the link href when getting link state?  That is, why
> > didn't we reget it earlier, when it changed?  And if it did not change, why
> > do we not have it cached?
Because we do link updates asynchronously when for example binding element to dom tree. Attribute is set before the element is in tree.

Comment 13

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #11)
> Looks like we should bake the content creator function pointers into
> nsHtml5ElementName and transfer them the node creation point from there.
> This would avoid a second (on top of looking up nsHtml5ElementName itself)
> lookup through the set of possible elements.

Looking into this now for innerHTML purposes (not document.createElement() for now).

Comment 14

[Andrew Overholt [:overholt]]

(meta so I'm marking this as P3. Bugs where work will happen that block this should remain P1)

Comment 15

[Dave Hunt [:davehunt] [he/him] ⌚BST]

All dependencies are resolved. Can we close this?

Comment 16

[Jan de Mooij [:jandem]]

(In reply to Dave Hunt [:davehunt] [he/him] ⌚BST from comment #15)

All dependencies are resolved. Can we close this?

Yes. A lot of optimization work has been done for Speedometer 3 including Backbone, but that work is tracked elsewhere.