Repository navigation
Conversation
|
I think 1.0X perf improvements are not worth your time mate, I appreciate the effort though. |
|
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. |
|
Four independent reasons, one per commit:
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 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. |
|
would this fail if a classname has <>"' chars in it? I don't see tests around that |
c7bb078 to
c076aef
Compare
|
No. Escaping never happened at parse time, in either version. I diffed the branch against You are right that the tests were missing, so I pushed two blocks. I also dropped the |
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 againstmainat #339.What this PR does
Normalise a class value while parsing without building a token list.
attribute()inparse-from-string.jscalledelement.className = value, which built aDOMTokenList, split the value, added every token, joined them again and wrote the result back through theAttrvalue setter. The only lasting effect of that round trip is the normalised attribute value (no empty entries, no duplicates, single spaces), whichclassTokens()now produces directly. The token list itself is built on demand, by the existingclassListgetter, for the elements that need one. The same commit replaceshtmlClasses.has(localName)followed byhtmlClasses.get(localName)with a singlegetincreateHTMLElement.Tests for class values that contain markup characters.
test/html/element.jsparsesclass=" a<b &c a<b d"e "and asserts the attribute value,className, the three tokens, the serialized<div class="a<b &c d"e">and a round trip through a second parse.test/xml/document.jscovers theescape()path, where the same value serializes asclass="a<b &c". Both blocks pass onmainas 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.
main/pages/*)content.jssequence onw3c.html)documentElement.cloneNode(true))The suite total is weak here because
bench-domalone 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-deepabove). The two parse cases were therefore re-timed standalone, one process per revision, three rounds each (min ms):mainObservable surface
classattribute values are normalised exactly as before: the tokens, their order and the single-space joining are those aDOMTokenListproduces.getAttribute('class'),classNameand the serialized output are unchanged (the characterisation guard hashes the fullouterHTMLof 14 real pages, and the new tests cover<,>,&,",',\xA0, duplicate and empty tokens).attributeChangedCallback(element, name, null, value)); the removed round trip through theAttrsetter fired no observer records during parsing.npm test's MutationObserver benchmark reports the same count (2) before and after.element.classListis now created on first use instead of during parsing. It is the same object on every later access, as before.Reproducing the numbers
The harness lives on the campaign branch of my fork:
zirkelc/linkedom@perf/autoresearch. It holdsperf/*.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) andperf/experiments.tsv(the log, including the 10 discarded experiments and their numbers).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
GEOMEANweights every case equally. The fixtures are live pages, so a fresh download changes the absolute milliseconds (not the ratios);perf/fetch-fixtures.shlists 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.