Speedometer backbone.js footer updating code is slower than in Chrome/Safari

NEW
Unassigned

Status

()

Core
DOM
P3
normal
a year ago
9 months ago

People

(Reporter: jandem, Unassigned)

Tracking

(Depends on: 1 bug, Blocks: 2 bugs, {meta, perf})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qf:meta])

Attachments

(2 attachments)

(Reporter)

Description

a year ago
Created attachment 8856972 [details]
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.

Updated

a year ago
Depends on: 1351857

Updated

a year ago
Depends on: 1355441

Updated

a year ago
Depends on: 1355479

Updated

a year ago
Depends on: 1355540

Updated

a year ago
Depends on: 1355787

Updated

a year ago
Depends on: 1217436

Updated

a year ago
Depends on: 1361274

Updated

a year ago
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p1]

Updated

a year ago
Blocks: 1366777

Updated

a year ago
No longer blocks: 1366777
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.
Flags: needinfo?(jdemooij)
(Reporter)

Comment 2

a year ago
(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.
Flags: needinfo?(jdemooij)
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.
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().
Could you share a profile without any random addon stuff, or perhaps upload your modified testcase?
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.
Created attachment 8880183 [details]
10k iterations testcase

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

Comment 8

a year ago
smaug, maybe you can profile the DOM bits here?

Updated

a year ago
Depends on: 1347643
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?
(Reporter)

Comment 10

a year ago
(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.
Flags: needinfo?(bugs)
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.
(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.
Flags: needinfo?(bugs)
(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).

Updated

a year ago
Depends on: 1378218

Updated

11 months ago
Component: JavaScript Engine → DOM
Whiteboard: [qf:p1] → [qf:meta]
Keywords: meta
Priority: -- → P1
(meta so I'm marking this as P3. Bugs where work will happen that block this should remain P1)
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.