Closed Bug 1355472 Opened 7 years ago Closed 4 months ago

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

Categories

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

enhancement

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Unassigned)

References

(Blocks 2 open bugs)

Details

(Keywords: meta, perf)

Attachments

(2 files)

Attached file 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.
Whiteboard: [qf]
Whiteboard: [qf] → [qf:p1]
No longer blocks: TimeToFirstPaint_FB
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)
(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.
Longer-running testcase. Profile with no add-ons: http://bit.ly/2sChTck.
smaug, maybe you can profile the DOM bits here?
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?
(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).
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
Keywords: perf
Component: DOM → DOM: Core & HTML
Summary: Speedometer backbone.js footer updating code is slower than in Chrome/Safari → [meta] Speedometer backbone.js footer updating code is slower than in Chrome/Safari
Performance Impact: --- → ?
Whiteboard: [qf:meta]

All dependencies are resolved. Can we close this?

Performance Impact: ? → ---
Flags: needinfo?(jdemooij)
Severity: normal → S3

(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.

Status: NEW → RESOLVED
Closed: 4 months ago
Flags: needinfo?(jdemooij)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: