Skip to content

perf: skip the token list while parsing class attributes - #336

Open
zirkelc wants to merge 2 commits into
WebReflection:mainfrom
zirkelc:perf-class-attributes
Open

zirkelc wants to merge 2 commits into
WebReflection:mainfrom
zirkelc:perf-class-attributes

Conversation

@zirkelc

@zirkelc zirkelc commented Sep 23, 2026 •

Copy link
Copy Markdown

Context: performance optimization campaign. This is one of 3 open PRs from a systematic performance campaign on linkedom, run as an automated research loop: 18 isolated experiments, 8 kept, 10 discarded. Every candidate change was

  • benchmarked with an A/B harness that loads two git revisions of esm/ into one process and times 14 workloads in alternation. Module instances carry a stable load-order bias of several percent, so the harness runs both load orders in separate child processes and combines them with a geometric mean. The machine was shared and has cores of two speeds, so the reported delta is the median of the paired ratios (both sides run back to back in every iteration), not a ratio of minima.
  • measured on 14 deterministic workloads, half of them mirroring test/benchmark/content.js (parse, crawl childNodes/children, cloneNode(true), querySelectorAll, getElementsByTagName, remove, outerHTML, innerHTML round trip on w3c.html and dom.html), half a real content-extraction pipeline on 14 full Shopify storefront pages (skip-link and main detection, contains, compareDocumentPosition, section cloning, ten removal passes by selector, outerHTML). test/benchmark/*.js loads one version per process, so using it for A/B would mean comparing two standalone runs, and run-to-run drift is larger than most effect sizes here. It was kept as an external cross-check.
  • gated by a calibrated noise floor (1.8% on the suite total, measured with identical code on both sides): changes under 3.5% total, or under 6% on a targeted case, were discarded, and every keep required a second confirming run.
  • verified behaviour-preserving by a characterisation guard (hashes of the serialized output, query results, text content and document-order answers of all 14 workloads) plus npm test, both green after every commit. Coverage stays at 100% lines and branches.

Updated after #339. This PR started with two commits. The first one cached the joined value of a DOMTokenList, because css-select asked for it through the adapter for every element it tested against a class selector. #339 removed that read, so the cache no longer buys anything and I dropped it. What is left is the parser commit, plus the tests @WebReflection asked for. The numbers below are fresh runs of this branch against main at #339.

What this PR does

Normalise a class value while parsing without building a token list. attribute() in parse-from-string.js called element.className = value, which built a DOMTokenList, split the value, added every token, joined them again and wrote the result back through the Attr value setter. The only lasting effect of that round trip is the normalised attribute value (no empty entries, no duplicates, single spaces), which classTokens() now produces directly. The token list itself is built on demand, by the existing classList getter, for the elements that need one. The same commit replaces htmlClasses.has(localName) followed by htmlClasses.get(localName) with a single get in createHTMLElement.

Tests for class values that contain markup characters. test/html/element.js parses class=" a<b &amp;c a<b d&quot;e " and asserts the attribute value, className, the three tokens, the serialized <div class="a<b &c d&quot;e"> and a round trip through a second parse. test/xml/document.js covers the escape() path, where the same value serializes as class="a&lt;b &amp;c". Both blocks pass on main as well: escaping never happened at parse time, in either version, so the tests describe the current behaviour and not the change.

Verification (this branch vs main)

Two independent runs, plus an identical-source control run that measured -0.50% on the total (that is the noise). Negative = faster.

case run 1 run 2 speed-up vs main
extract-pages (extraction pipeline, /pages/*) -8.2% -6.1% 1.07x
extract-home -6.4% -7.2% 1.07x
parse-shop (parse 2.7 MB of storefront HTML) -6.6% -7.1% 1.07x
extract-products -5.6% -5.8% 1.06x
bench-w3c (full content.js sequence on w3c.html) -7.6% -15.3% n/a (runs disagree)
clone-deep (documentElement.cloneNode(true)) +6.8% -4.2% n/a (control run moved -4.4% on its own)
query-scraper, query-first, query-simple, contains-position, crawl, serialize, text-content, bench-dom within noise n/a
suite TOTAL -1.1% -3.7% 1.01x to 1.04x
GEOMEAN (every case weighted equally) -2.1% -5.1% n/a

The suite total is weak here because bench-dom alone is 430 ms of the 800 ms total and this PR barely touches it. The cases to look at are the four that parse a document per iteration.

Because two revisions share one heap in the A/B harness, an allocation change can show a false delta on a case it does not touch (clone-deep above). The two parse cases were therefore re-timed standalone, one process per revision, three rounds each (min ms):

case main this branch speed-up
parse-shop 23.87 / 23.84 / 24.03 21.09 / 21.97 / 21.77 1.09x
extract-products 69.72 / 70.03 / 66.76 67.79 / 64.78 / 64.04 1.05x

Observable surface

  • Parsed class attribute values are normalised exactly as before: the tokens, their order and the single-space joining are those a DOMTokenList produces. getAttribute('class'), className and the serialized output are unchanged (the characterisation guard hashes the full outerHTML of 14 real pages, and the new tests cover <, >, &, ", ', \xA0, duplicate and empty tokens).
  • Custom element and mutation callbacks during parsing are unchanged. The parser already reported the attribute itself (attributeChangedCallback(element, name, null, value)); the removed round trip through the Attr setter fired no observer records during parsing. npm test's MutationObserver benchmark reports the same count (2) before and after.
  • element.classList is now created on first use instead of during parsing. It is the same object on every later access, as before.
  • No new own properties on elements, no dependency change, no change to the worker build.

Reproducing the numbers

git clone https://github.com/WebReflection/linkedom && cd linkedom && npm ci
git fetch origin pull/336/head:pr-336

The harness lives on the campaign branch of my fork: zirkelc/linkedom@perf/autoresearch. It holds perf/*.mts (A/B harness, characterisation guard, memory harness, profiler, the 14 cases), perf/plan.md (method, calibration, every experiment and why it was kept or discarded) and perf/experiments.tsv (the log, including the 10 discarded experiments and their numbers).

git remote add campaign https://github.com/zirkelc/linkedom && git fetch campaign
git checkout campaign/perf/autoresearch -- perf && git reset -- perf
perf/fetch-fixtures.sh                # 14 public storefront pages, ~13 MB, not committed
node perf/guard.mts --update          # record the current behaviour of your checkout
node perf/ab.mts main main            # noise control: expect well under 2% on TOTAL
node perf/ab.mts main pr-336          # the measurement, twice

One run takes about 4.5 minutes on an idle machine. Judge a case only when both runs agree; the suite total is the headline number, and GEOMEAN weights every case equally. The fixtures are live pages, so a fresh download changes the absolute milliseconds (not the ratios); perf/fetch-fixtures.sh lists the URLs.


Companion PRs from the same campaign, independent of each other and of this one:

A fourth PR (#338, the node end marker) is closed: the mechanism I claimed for it did not hold up when I measured it in isolation.

@WebReflection

Copy link
Copy Markdown
Owner

I think 1.0X perf improvements are not worth your time mate, I appreciate the effort though.

@zirkelc

zirkelc commented Oct 12, 2026

Copy link
Copy Markdown
Author

Fair. And #339 removed the part that was not 1.0x: class lookups no longer go through classList, so the cache commit here is pointless now and I will drop it.

What is left is the parser commit, which stops building a DOMTokenList for every class attribute while parsing, something nothing reads any more after #339. Standalone on 2.7 MB of real pages: parse 27 -> 25 ms (about 1.08x), queries unchanged. On top of #335 the same commit is 19.6 -> 15.7 ms (1.24x).

1.08x on parse alone may well be below your bar. Say so and I close it, otherwise I push the reduced version.

@zirkelc

zirkelc commented Oct 12, 2026

Copy link
Copy Markdown
Author

Four independent reasons, one per commit:

  1. escape() ran String.prototype.replace with a function callback over every text node, although almost nothing needs escaping: of the 23182 text and attribute strings on one real page, 2.7% contain < > & or \xA0. A test() with a non-global copy of the class returns the input untouched, and the whole set then costs 1.92 ms instead of 2.27 ms. Non-global matters, test() on the /g regex moves lastIndex.

  2. Element.toString() pushed every tag, attribute and text fragment into an array and joined at the end. On dom.html that array holds hundreds of thousands of entries: array growth, the array itself, then a second pass at join time. This is the big one: 21.0 -> 11.9 ms for that commit alone.

  3. childNodes / children called a helper per entry (nextSibling, and for children nextElementSibling, which re-scans the non-element siblings between each pair of elements). One walk that jumps from an element to element[END][NEXT] collects the same nodes without those calls.

  4. wholeText used unshift per preceding text node, which moves the whole array each time.

Rebased on current main, standalone with one process per revision (not my paired harness):

outerHTML, 5 real documents 23.8 / 23.4 ms -> 12.0 / 12.1 ms
body.textContent 1.45 / 1.47 ms -> 1.01 / 0.87 ms
crawl childNodes + children 16.4 / 16.7 ms -> 15.5 / 13.7 ms

On benchmark:html (12 MB, single iterations, noisy): outerHTML cold 74.6 / 85.7 -> 58.4 / 57.5 ms, crawling childNodes hot 51.0 / 40.7 -> 33.2 / 34.8 ms.

Each commit stands alone, so any of them can be dropped.

@WebReflection

Copy link
Copy Markdown
Owner

would this fail if a classname has <>"' chars in it? I don't see tests around that

@zirkelc
zirkelc force-pushed the perf-class-attributes branch from c7bb078 to c076aef Compare October 13, 2026 11:00
@zirkelc zirkelc changed the title perf: cache the class token value and skip the token list while parsing perf: skip the token list while parsing class attributes Oct 13, 2026
@zirkelc

zirkelc commented Oct 13, 2026

Copy link
Copy Markdown
Author

No. Escaping never happened at parse time, in either version. element.className = value ended in update(), which does attribute.value = classList.value, so the raw string was stored, and the new code writes that same string into attribute[VALUE] directly. The escaping is in Attr.toString(), and both paths reach it with the same value.

I diffed the branch against main on 8 inputs: a<b c>d &amp;e, q&quot;uote plain, \xA0 as the separator, tabs/newlines/duplicates, empty, spaces only, backticks and slashes, and an unquoted mess with ' and <b>. getAttribute('class'), className, the token list, outerHTML, [class~=...] matching, getElementsByClassName, the XML serialization and a re-parse of the output are identical on both sides.

You are right that the tests were missing, so I pushed two blocks. test/html/element.js parses class=" a<b &amp;c a<b d&quot;e " and asserts the value, the 3 tokens, <div class="a<b &c d&quot;e"> and a round trip; test/xml/document.js covers the escape() path, where the same value serializes as class="a&lt;b &amp;c". Both pass on main too, so they document the current behaviour.

I also dropped the DOMTokenList cache commit: #339 took the classList.value read out of matching, so it no longer buys anything. This PR is now the parser change plus the tests, rebased on main, and the body has fresh numbers: about 1.07x on the four cases that parse a document per iteration (1.09x standalone on parse alone), everything else within noise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants