Closed Bug 1387125 Opened 7 years ago Closed 7 years ago

Create a performance test to compare XBL with Custom Elements

Categories

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

task

Tracking

()

RESOLVED WONTFIX

People

(Reporter: bgrins, Assigned: bgrins)

References

Details

Attachments

(1 file, 1 obsolete file)

It seems possible to migrate many of the XBL bindings we use today in Firefox to Custom Elements, but performance is going to be a really important factor. XUL and XBL both use fastload and they both have prototype cache, so loading them and opening new windows should be quite fast.  Custom Elements (and Shadow DOM) don't have anything like that right now. If we want to use them in browser chrome, we may need to figure out how to support things like this.

We could start measuring this by setting up a benchmark to compare Custom Elements and XBL. That benchmark could initially use the CE polyfill (https://github.com/webcomponents/custom-elements) and then ultimately rely on the native implementation. A benefit to using the polyfill first is we could make sure the native implementation didn't regress for sites that currently use the polyfill.
Some ideas about things to measure:

1) Compare 'time to construct' for N elements declared in markup (and in JS). I think this means how long until all the <contructor>s are done in XBL and how long until all the connectedCallbacks are done in Custom Elements. We can put the Custom Element definition wherever is best, I assume this is in <head>  so that it doesn’t need to go through the upgrade process on DOM nodes that already exist when it’s defined.
2) Compare memory usage for N elements declared in markup (and in JS)
(P3 only because we can't really do this until we have Custom Elements closer to shipping)
Priority: -- → P3
Just putting up a pass that gets things in place for a mochitest-chrome test that:

* Opens up a XUL window and creates N elements from JS with XBL bindings
* Opens up a HTML window and create N custom elements from JS (using the polyfill and not the built in support)
* Takes a profile of the test and prints out instructions for uploading to perf-html.io

It can be run with `./mach mochitest dom/base/test/chrome/test_perf_xbl_vs_customelement.xul`

It doesn't attempt to measure creation time for elements defined in markup, and also it does only the most simple possible binding / custom element
Attachment #8893528 - Attachment is obsolete: true
Comment on attachment 8896451 [details]
Bug 1387125 - Create a performance test to compare XBL with Custom Elements in XUL Documents;

Blake, I'd like to get some initial feedback on this approach for benchmarking XBL vs Custom Elements. Here's a summary of what it's doing:

1) This is a mochitest-chrome that's always skipped in CI and meant to be run locally via `./mach mochitest dom/base/test/chrome/test_perf_xbl_vs_customelement.xul`. Though it would be possible to run it in CI and report back for charting via PERFHERDER_DATA if we wanted to.
2) It's using the custom elements polyfill from https://github.com/webcomponents/custom-elements. Our native implementation doesn't yet work for this test case. I hope that as native support lands the numbers will improve.
3) It runs X iterations, creating Y elements and measuring the average time to append each into the DOM. It does this once for a trivial XBL binding created from a XUL doc and once for a trivial Custom Element created from an HTML doc.
4) It takes a profile during the test and prints out commands at the end that can be used to upload the profile to perf-html.io

I was surprised by the initial numbers, which pointed to Custom Elements + polyfill being faster, at least for this simple case without insertion points. For instance here's what I see on my machine:

  CUSTOM ELEMENTS: On average, it took 30.03 ms to create 1000 elements (median 29.52 ms, max 38.43 ms, min 24.47 ms, per element 0.03 ms)
  XBL: On average, it took 73.29 ms to create 1000 elements (median 73.35 ms, max 74.77 ms, min 71.42 ms, per element 0.073 ms)
Attachment #8896451 - Flags: feedback?(mrbkap)
(In reply to Brian Grinstead [:bgrins] from comment #6)
> I was surprised by the initial numbers, which pointed to Custom Elements +
> polyfill being faster, at least for this simple case without insertion
> points. For instance here's what I see on my machine:

Hi Brian,

I'm curious about this result. One obvious thing to consider is that XBL is doing a lot more work (and has quite a few more features) than custom elements. I'm also curious if laying out <xul:box><xul:box/></xul:box> is simply slower than <html:div><html:span>#text</html:span></html:div>. As a last thought, the XBL in your test is using anonymous content, which requires cloning nodes and doing extra work compared to the custom element which is simply manipulating existing DOM elements. It would probably be more fair to try to attach a shadow tree to contain the text in the custom element case or to have your XBL binding use its constructor to set the innerHTML of its element to text instead of using anonymous content.

Overall, this approach looks good to me. I think we'll eventually want to expand the performance tests to include removing elements from the DOM tree (which runs destructors, etc.), though that work can come later.
Comment on attachment 8896451 [details]
Bug 1387125 - Create a performance test to compare XBL with Custom Elements in XUL Documents;

https://reviewboard.mozilla.org/r/167682/#review177106

Looks good.
Attachment #8896451 - Flags: review+
Attachment #8896451 - Flags: review+
Attachment #8896451 - Flags: feedback?(mrbkap)
Attachment #8896451 - Flags: feedback+
(In reply to Blake Kaplan (:mrbkap) from comment #7)
> I'm also curious if laying out <xul:box><xul:box/></xul:box> is
> simply slower than <html:div><html:span>#text</html:span></html:div>.

That's a good point. I now have a fork of the Custom Elements polyfill that supports XUL docs (https://github.com/webcomponents/custom-elements/compare/master...bgrins:firefox-browser-chrome?expand=1), which should provide a better comparison. I can add that as a third configuration.

> As a
> last thought, the XBL in your test is using anonymous content, which
> requires cloning nodes and doing extra work compared to the custom element
> which is simply manipulating existing DOM elements. It would probably be
> more fair to try to attach a shadow tree to contain the text in the custom
> element case or to have your XBL binding use its constructor to set the
> innerHTML of its element to text instead of using anonymous content.

I don't think the test xbl binding is using anonymous content - IIUC this only happens if we use <children> tags which are not being used here.
(In reply to Brian Grinstead [:bgrins] from comment #9)
> I don't think the test xbl binding is using anonymous content - IIUC this
> only happens if we use <children> tags which are not being used here.

<content> creates anonymous content. <children> lets you put non-anonymous content in an XBL binding parent.
(In reply to Dão Gottwald [::dao] from comment #10)
> (In reply to Brian Grinstead [:bgrins] from comment #9)
> > I don't think the test xbl binding is using anonymous content - IIUC this
> > only happens if we use <children> tags which are not being used here.
> 
> <content> creates anonymous content. <children> lets you put non-anonymous
> content in an XBL binding parent.

Thanks, I didn't realize that was anonymous. Since I don't know of a way to put non-anonymous content directly in the binding, I modified the binding to have no content - i.e `<binding id="test"></binding>` - and I see the following results:

CUSTOM ELEMENTS: On average, it took 32.36 ms to create 1000 elements (median 30.72 ms, max 43.93 ms, min 21.83 ms, per element 0.032 ms)
XBL: On average, it took 43.74 ms to create 1000 elements (median 43.16 ms, max 48.51 ms, min 40.12 ms, per element 0.044 ms)

So it did speed it up significantly from Comment 6 (~70ms to ~40ms), but it's still slower than the Custom Elements polyfill and in this case it's rendering no content while the CE is still setting innerHTML. If I remove the `this.innerHTML = "Created CE";` in the Custom Element connectedCallback, I get:

CUSTOM ELEMENTS: On average, it took 21.42 ms to create 1000 elements (median 22.09 ms, max 25.33 ms, min 16.1 ms, per element 0.021 ms)
XBL: On average, it took 45.02 ms to create 1000 elements (median 46.92 ms, max 51.91 ms, min 38.84 ms, per element 0.045 ms)
Blocks: war-on-xbl
(In reply to Brian Grinstead [:bgrins] from comment #9)
> (In reply to Blake Kaplan (:mrbkap) from comment #7)
> > I'm also curious if laying out <xul:box><xul:box/></xul:box> is
> > simply slower than <html:div><html:span>#text</html:span></html:div>.
> 
> That's a good point. I now have a fork of the Custom Elements polyfill that
> supports XUL docs
> (https://github.com/webcomponents/custom-elements/compare/master...bgrins:
> firefox-browser-chrome?expand=1), which should provide a better comparison.
> I can add that as a third configuration.

I've got a new version of the test where the Custom Elements are created in a XUL document and indeed - it's much slower than in the HTML document:

CUSTOM ELEMENTS: On average, it took 77.81 ms to create 1000 elements (median 79.48 ms, max 85.64 ms, min 68.69 ms, per element 0.078 ms)
XBL: On average, it took 41.66 ms to create 1000 elements (median 42.31 ms, max 42.9 ms, min 40.15 ms, per element 0.042 ms)

I'll do a try push to see how it turns out on various platforms. What'll be really interesting is seeing these numbers without the polyfill once we don't need it anymore.
Maybe this can be run without a polyfill after bug 1334043 (at least if the Custom Elements went back into an HTML document)
Depends on: 1334043
New version is up now that includes XUL+XBL, XUL+CE, HTML+CE. Here's the numbers I see:

XBL: On average, it took 40.27 ms to create 1000 elements (median 39.86 ms, max 42.31 ms, min 39.13 ms, per element 0.04 ms)
CUSTOM ELEMENTS (XUL): On average, it took 79.25 ms to create 1000 elements (median 78.14 ms, max 84.91 ms, min 74.59 ms, per element 0.079 ms)
CUSTOM ELEMENTS (HTML): On average, it took 18.47 ms to create 1000 elements (median 16.32 ms, max 27.42 ms, min 14.59 ms, per element 0.018 ms)

I suspect that the slowness with XUL+CE may have to do with the polyfill creating a deepTreeWalker and traversing the DOM on every mutation
(In reply to Brian Grinstead [:bgrins] from comment #16)
> I suspect that the slowness with XUL+CE may have to do with the polyfill
> creating a deepTreeWalker and traversing the DOM on every mutation

Yes, this was primarily the problem. I switched to a less modified version of the polyfill (https://github.com/webcomponents/custom-elements/compare/master...bgrins:xul-alone) and get the following numbers:

XBL: On average, it took 38.56 ms to create 1000 elements (median 38.34 ms, max 39.67 ms, min 37.74 ms, per element 0.039 ms)
CUSTOM ELEMENTS (XUL): On average, it took 45.26 ms to create 1000 elements (median 45.99 ms, max 49.75 ms, min 40.23 ms, per element 0.045 ms)
CUSTOM ELEMENTS (HTML): On average, it took 17.21 ms to create 1000 elements (median 14.8 ms, max 25.48 ms, min 12.76 ms, per element 0.017 ms)

This can be run with the latest patch using `./mach mochitest dom/base/test/chrome/test_perf_xbl_vs_customelement.xul`
In order to get function names in the profile that the test made I had to pull down https://github.com/mstange/analyze-tryserver-profiles.git and then run `python symbolicate_profile.py` on the temp file it created.

The profile is a bit odd since it was taken during the mochitest-chrome run so a lot of it appears to be taken up by test harness. But I think this subset of the profile is the relevant one: https://perf-html.io/public/253159e145de27ddbea51f465ad091220de004d9/calltree/?hiddenThreads=&implementation=cpp&thread=0&threadOrder=0-1&transforms=f-cpp-04. There are three sections here - the first is XUL+CE, the second is HTML+CE, and the third is XUL+XBL.
Here's a profile that uses `display:none` for XUL+CE and HTML+CE: https://perf-html.io/public/7e5e4bb5ac16080a96b35173f90c66b321145f8f/calltree/?hiddenThreads=&thread=0&threadOrder=0-1&transforms=f-combined-04.

In this case they are almost the same perf:

CUSTOM ELEMENTS (XUL): On average, it took 8.51 ms to create 1000 elements (median 6.38 ms, max 9.99 ms, min 12.22 ms, per element 0.009 ms)
CUSTOM ELEMENTS (HTML): On average, it took 7.93 ms to create 1000 elements (median 5.61 ms, max 9.1 ms, min 13.78 ms, per element 0.008 ms)

I did not do display:none for XBL here since it prevents the constructor from being ran
Reminder to re-run these numbers with the native HTML+CE implementation to compare with polyfill version once Bug 1334043 lands
Flags: needinfo?(bgrinstead)
OK, here's a comparison between the Custom Element polyfill and the native implementation for this test:

Native HTML Custom Elements implementation:

   Code: https://reviewboard-hg.mozilla.org/gecko/raw-rev/e3d50fd5e1b0
   Profile (zoomed in on HTML CE section): https://perfht.ml/2y8uJnZ
   Measurement (varies a bit each run): On average, it took 14.87 ms to create 1000 elements (median 14.82 ms, max 16.15 ms, min 13.71 ms, per element 0.015 ms)

Polyfill HTML Custom Elements implementation:

   Code: https://reviewboard-hg.mozilla.org/gecko/raw-rev/d6a6c51002a1
   Profile (zoomed in on HTML CE section): https://perfht.ml/2yartbY
   Measurement (varies a bit each run): On average, it took 16.4 ms to create 1000 elements (median 14.14 ms, max 24.11 ms, min 13.12 ms, per element 0.016 ms)
Flags: needinfo?(bgrinstead)
When I add

* { display: block !important; }

to each of the three tests to factor out any layout related effects I get:

4 INFO XBL: On average, it took 12.24 ms to create 1000 elements (median 12.74 ms, max 14.5 ms, min 10.01 ms, per element 0.012 ms)
5 INFO CUSTOM ELEMENTS (XUL): On average, it took 17.61 ms to create 1000 elements (median 17.16 ms, max 25.88 ms, min 12.47 ms, per element 0.018 ms)
6 INFO CUSTOM ELEMENTS (HTML): On average, it took 12.97 ms to create 1000 elements (median 12.64 ms, max 15.5 ms, min 11.12 ms, per element 0.013 ms)
(In reply to Neil Deakin from comment #24)
> When I add
> 
> * { display: block !important; }
> 
> to each of the three tests to factor out any layout related effects I get:
> 
> 4 INFO XBL: On average, it took 12.24 ms to create 1000 elements (median
> 12.74 ms, max 14.5 ms, min 10.01 ms, per element 0.012 ms)
> 5 INFO CUSTOM ELEMENTS (XUL): On average, it took 17.61 ms to create 1000
> elements (median 17.16 ms, max 25.88 ms, min 12.47 ms, per element 0.018 ms)
> 6 INFO CUSTOM ELEMENTS (HTML): On average, it took 12.97 ms to create 1000
> elements (median 12.64 ms, max 15.5 ms, min 11.12 ms, per element 0.013 ms)

Thanks for checking that, good catch. Sidenote: I wonder how xul flex would compare with css flex for XUL/XBL and XUL/CE - I know your test case from https://bugzilla.mozilla.org/show_bug.cgi?id=1183163 surfaced some issues with css flex, but I think there have been some optimizations in the meantime.
Depends on: 1404420
I'd like to get a first version of this landed if possible so I can start keeping track of the times. There are definitely some follow-ups possible, like measuring the time to open and re-open new XUL windows with XBL vs CE and possibly tracking times in automation via PERFHERDER_DATA.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Comment on attachment 8896451 [details]
Bug 1387125 - Create a performance test to compare XBL with Custom Elements in XUL Documents;

Somehow mozreview wasn't setting the reviewer flag
Attachment #8896451 - Flags: review?(mrbkap)
I'll get to this review tomorrow.
Comment on attachment 8896451 [details]
Bug 1387125 - Create a performance test to compare XBL with Custom Elements in XUL Documents;

https://reviewboard.mozilla.org/r/167682/#review199748

I have a few minor nits. I think this is a good start. It would be nice to find a better home for this than a disable chrome test that must be run manually but that's obviously super low priority.

::: dom/base/test/chrome/test_perf_xbl_vs_customelement.xul:87
(Diff revision 9)
> +    xblWindow.close();
> +    done();
> +  }
> +  async function customElementHTMLDone(times) {
> +    customElementHTMLTimes = getTimes(times);
> +    customElementHTMLWindow.close();

Set this to null so we don't hold the old window alive during the next test?

::: dom/base/test/chrome/test_perf_xbl_vs_customelement.xul:89
(Diff revision 9)
> +  }
> +  async function customElementHTMLDone(times) {
> +    customElementHTMLTimes = getTimes(times);
> +    customElementHTMLWindow.close();
> +
> +    await wait(500);

What is this waiting for? Is this a general "cooling down" period? Can you add a comment or something to that effect? Should we be forcing a GC while we're waiting?

::: dom/base/test/chrome/test_perf_xbl_vs_customelement.xul:95
(Diff revision 9)
> +    customElementXULWindow = window.open("file_perf_customelement.xul", "", "chrome,width=600,height=400");
> +  }
> +  async function customElementXULDone(times) {
> +    customElementXULTimes = getTimes(times);
> +    customElementXULWindow.close();
> +    await wait(500);

Same comments here as in `customElementHTMLDone`.
Attachment #8896451 - Flags: review?(mrbkap) → review+
(In reply to Blake Kaplan (:mrbkap) from comment #32)
> I have a few minor nits. I think this is a good start. It would be nice to
> find a better home for this than a disable chrome test that must be run
> manually but that's obviously super low priority.

I talked with Joel about our options for converting this into a Talos test. It's definitely possible, although I don't know if it's valuable to port everything here (especially the XUL/XBL case but even the XUL/CE case which would be implicitly covered by other Talos tests as we begin to land CE in chrome).

So it made me think that we may want to instead have a focused set of tests for in-content Custom Elements (i.e. element creation time, registration time), and eventually Shadow DOM. We now have a capability to add very simple test cases as in http://searchfox.org/mozilla-central/source/testing/talos/talos/tests/perf-reftest-singletons where each is an HTML page that tracks one measurement.

Are there any existing plans to add performance tests for Custom Elements and/or do any of our existing benchmarks cover Custom Elements directly? I don't see anything in Bug 1396567 but want to double check before I spent time on it.
Flags: needinfo?(mrbkap)
Ben, do you know if there were existing plans to create a performance test for Custom Elements?
Flags: needinfo?(mrbkap) → needinfo?(btian)
Our plan is to test Custom Elements and measure performance on real website. We initially targeted on YouTube, however, we just realized YouTube currently is still on v0 spec [1], not migrates to v1 until 2018Q1. But we found an alternative, https://nest.com, which already use Custom Elements v1. If all goes well and there is no surprises, we will measure performance with https://nest.com.

[1] https://groups.google.com/a/mozilla.com/forum/#!topic/mozilla-youtube-discuss/avOoLIkXHC4
Flags: needinfo?(btian)
We need to also test the overhead of having CustomElements enabled when doing any random DOM operations.
Basically stuff like, does Speedometer regress with CustomElements enabled etc.
(In reply to Olli Pettay [:smaug] from comment #36)
> We need to also test the overhead of having CustomElements enabled when
> doing any random DOM operations.
> Basically stuff like, does Speedometer regress with CustomElements enabled
> etc.

I hope that doing a full talos run covers everything we care about in that regard - I filed Bug 1410536 about remaining regressions with the pref on
Talos is definitely not enough for that. We'll need to do some microbenchmark profiling.
OK, based on this discussion I'm tempted to close this bug rather than landing it, since:

1. the original goal of the bug has been accomplished through writing the test and running it locally
2. there are already plans for testing Custom Element performance directly so it won't be useful in that context
3. once we start using Custom Elements in the browser chrome then all of our existing talos tests will implicitly cover the Custom Elements + XUL case which won't be covered by (2)

If we ultimately decide that we do want to create a set of talos tests to cover Custom Elements (and/or Shadow DOM) then I can help set it up, reusing some of the code attached here
Closing, at least for now, as per Comment 39
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Component: DOM → DOM: Core & HTML
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: