1427926 - getElementsByAttribute("attribute", "*") much faster than querySelectorAll("[attribute]")

Status: RESOLVED INVALID

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

Description

[Myk Melez [:myk] [@mykmelez]]

This microbenchmark I wrote for about:preferences shows that document.getElementsByAttribute("attribute", "*") is much faster (~25x) than document.querySelectorAll("[attribute]"):

performance.mark("getElementsByAttribute-start");
for (let i = 0; i < 1000; i++) {
  const elements = document.getElementsByAttribute("preference", "*");
}
performance.mark("getElementsByAttribute-end");
performance.measure("getElementsByAttribute", "getElementsByAttribute-start", "getElementsByAttribute-end");
performance.mark("querySelectorAll-start");
for (let i = 0; i < 1000; i++) {
  const elements = document.querySelectorAll("[preference]");
}
performance.mark("querySelectorAll-end");
performance.measure("querySelectorAll", "querySelectorAll-start", "querySelectorAll-end");
performance.getEntries().forEach(e => console.log(JSON.stringify(e)));

Fluent uses document.querySelectorAll("[attribute]") extensively, and interfaces like about:preferences will eventually need to use it in order to migrate to HTML, so it might be helpful to optimize it for this (common?) case, or provide an HTML equivalent for document.getElementsByAttribute("preference", "*").

Comment 1

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

This is not very surprising. querySelector* is by design, I'd say, slower than getElement(s)By*. It after all needs to parse and run the selector and *By* methods just need to check whether the value is '*' or something else and traverse the DOM, or in getElementById case just do a hashtable lookup.
Also, getElementsByAttribute just returns a NodeList which hasn't yet searched anything.
The test would be a tad more reasonable if one did something like
document.getElementsByAttribute("preference", "*").length;
and
document.querySelectorAll("[preference]").length;

The nice thing with querySelectorAll is that it returns non-live NodeList.

A testcase attached to this bug would be nice for profiling, as always.

Comment 2

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

Attachment: a testcase for qsa

Comment 3

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

Hmm, that might not be realistic enough testcase.

Comment 4

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

myk, could you test with .length; 
since right now the test in the initial comment is sort of comparing apples and oranges.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

(In reply to Olli Pettay [:smaug] from comment #1)
> This is not very surprising. querySelector* is by design, I'd say, slower
> than getElement(s)By*. It after all needs to parse and run the selector and
> *By* methods just need to check whether the value is '*' or something else
> and traverse the DOM, or in getElementById case just do a hashtable lookup.
> Also, getElementsByAttribute just returns a NodeList which hasn't yet
> searched anything.
> The test would be a tad more reasonable if one did something like
> document.getElementsByAttribute("preference", "*").length;
> and
> document.querySelectorAll("[preference]").length;
> 
> The nice thing with querySelectorAll is that it returns non-live NodeList.
> 
> A testcase attached to this bug would be nice for profiling, as always.

I could attach a testcase for profiling, but it'd be hard to run, since getElementsByAttribute only works in XUL, and Firefox no longer loads remote XUL (not even if the "remote" XUL is a local file).

However, an easy way to run the testcase in comment #0 is to load about:preferences in a tab, open the Web Console, and evaluate the testcase code in the console.


(In reply to Olli Pettay [:smaug] from comment #4)
> myk, could you test with .length; 
> since right now the test in the initial comment is sort of comparing apples
> and oranges.

Indeed, testing with .length as in the following testcase results in querySelectorAll becoming a few milliseconds *faster* (at ~35ms) than getElementsByAttribute (at ~38ms) on my machine:

performance.mark("getElementsByAttribute-start");
for (let i = 0; i < 1000; i++) {
  const elements = document.getElementsByAttribute("preference", "*").length;
}
performance.mark("getElementsByAttribute-end");
performance.measure("getElementsByAttribute", "getElementsByAttribute-start", "getElementsByAttribute-end");
performance.mark("querySelectorAll-start");
for (let i = 0; i < 1000; i++) {
  const elements = document.querySelectorAll("[preference]").length;
}
performance.mark("querySelectorAll-end");
performance.measure("querySelectorAll", "querySelectorAll-start", "querySelectorAll-end");
performance.getEntries().forEach(e => console.log(JSON.stringify(e)));

So perhaps this is actually a non-issue.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Upon further consideration, I think we should close this as a non-issue. If we identify a specific impact in the future, then we can reopen (or, more likely, file a new bug).