Answer an id selector from the id map instead of walking

[jdalton]

byId() falls back to walking the subtree when document.all is missing, which is every id lookup under jsdom, and that walk measures milliseconds where getElementById measures nanoseconds. getElementById settles two cases in constant time: whether the id exists at all, and where the first one is, which is all querySelector wants.

Detail, and how it was checked

byId() reaches for document.all and falls back to walking the subtree element by element when it is missing. jsdom does not implement document.all, so every #id takes the walk: 2.4ms on a 6300-element document against 43ns for getElementById. jsdom is where most of nwsapi's traffic is, so this is the common case rather than the fallback.

getElementById cannot answer on its own, since a document may carry an id more than once and querySelectorAll matches all of them. It does settle two things in constant time, and each buys back one case:

• whether the id exists anywhere. If the document has none, no descendant
  of any context has one either, so select(#missing) returns immediately: 2.19ms to 0.0007ms.
• where the first one is, in tree order. querySelector wants exactly that,
  so a lone #id against a document is answered by the id map: first(#title) 3.74ms to 0.0007ms.

For select() against a hit the walk still runs, because the duplicates have to be found, but it starts at the first match since none can precede it. Element-scoped queries keep the old path, because the first document-order match may sit outside the context and a match inside it would be missed. So does a detached subtree, which the document's id map knows nothing about.

Extracted from #167 as a standalone change: one file, applies to master on its own, and checked against the benchmark fixture to confirm the results are identical and nothing else moves.

References: the spec, the browser source, and what each part was reasoned from

This patch applies to master on its own. The sixteen in this series were checked by cherry-picking them onto master one after another, in this order and in reverse, and all sixteen land without a conflict.

• Spec: dom nonelementparentnode getelementbyid — getElementById returns the first element in tree order.
• Spec: scope match a selectors string — what a scoped query has to match.
• MDN: getElementById.

[dperini]

JD this is where we can workout the differences between specs and real world.
The configuration system allows for this granularity of choices and has worked well for the users. In this case the flag ID_DUPES disallow "Duplicated ID" in the same document which seems adequate behaviour. I suggest maintaining the flag but keep it "false" as configured default. For ID lookup this change alone increment speed 2x.

[jdalton]

Thank you for the review. Flipping IDS_DUPES to false by default is worth more than 2x — I measured 3446x on a lone #id — but it is a separate change from this one rather than a part of it, because it changes what a query answers and this patch does not. I would send it as its own PR if you want it.

What flipping the default buys, and what it costs

On the 6344-element fixture in test/speed/example/selectors.html, a lone #id selector, median of five interleaved rounds:

build	IDS_DUPES: true	IDS_DUPES: false
master 2e9498f	2.8361 ms	0.0008 ms	3446x
with this patch	2.4108 ms	0.0004 ms	5433x

It is that large because false turns the whole query into a single getElementById call, which is a hash lookup, while true has to keep looking in case the document carries the id twice.

The cost is the answer in that case. With IDS_DUPES: false, a document holding <p id=x> twice answers select('#x') with one element, where the reference engine and every browser answer two, because querySelectorAll matches every element carrying that id. That is the deviation the flag exists for, and whether it is the better default is your call.

What this patch does on its own, with the default left at `true`

It changes no answers at either setting. What it adds is that getElementById settles two questions without a walk — whether the id exists anywhere in the document, and which element carrying it comes first in tree order — so a lookup that misses and a querySelector that wants only the first hit both become constant time:

query	master	with this patch
select('#missing')	2.5604 ms	0.0010 ms	2498x
first('#title')	2.6169 ms	0.0004 ms	6218x
select('#title')	2.3599 ms	2.5101 ms	no gain

The last row is the limit of it. select('#title') has to return every match, a duplicate can sit anywhere below the context, and so the walk still happens. Removing that walk is exactly what flipping the default would do, which is the other half of your suggestion.