Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Four commits, all about walking the tree and turning it into a string.
escape(): skip the pass when there is nothing to escape. Every text node and every attribute value of a non-HTML document went throughreplace.call(es, /[<>&\xA0]/g, pe), a regex replace with a callback, although most strings contain none of those characters. Atestwith a non-global copy of the same character class decides first (teston the global regex would move itslastIndex).Attr.toStringgets the same treatment for its"replace, which ran on every attribute.childNodesandchildren: one pass over the list. Both getters callednextSibling()(andchildrenadditionallynextElementSibling(), which loops over the non-element nodes in between) for every entry. Since the list already stores an element followed by its subtree and its end marker, one walk that jumps from an element toelement[END][NEXT]collects the same nodes in the same order, without the helper calls. It also removes thenextElementSiblingimport from the mixin.Element.toString(): build one string. The serializer pushed every tag, attribute and text fragment into an array and joined it at the end; for a 2 MB document that is hundreds of thousands of array entries. Concatenating into a string instead lets V8 keep a rope and flatten once. The attribute case now compares'id'/'class'/'style'instead of' id'/' class'/' style', because the leading space is added where the attribute is appended.textContent,innerText,wholeText: the same change for the text getters.wholeTextalso drops anunshiftper preceding text node, which moved the whole array each time.Verification (this branch vs
main)maindocumentElement.outerHTML)body.textContent,document.title)childNodesandchildren)content.jssequence ondom.html)These runs were taken while the machine had other load, so the absolute milliseconds are higher
than in the other PRs of this campaign; the ratios are what matters, and the two runs agree on
every case that is claimed. The control run with identical code on both sides measured -1.13%
on the total under the same conditions.
Observable surface
escape()returns the input string itself when nothing needs escaping, instead of a new string with identical content. Only observable through identity (===on two separately produced strings), never through content.outerHTMLof 14 real pages plusw3c.htmlanddom.html, andnpm testcovers the void, SVG, XML and empty-attribute branches.childNodesandchildrenreturn the sameNodeListcontents in the same order, including attribute nodes being skipped and text, comment and CDATA nodes being kept. Both still return a freshNodeListon every access, solinkedom/cached's memoisation of these getters is unaffected.textContent,innerTextandwholeTextreturn the same strings, includinginnerText's whitespace collapsing and its\nbetween block elements.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:
Stacked, the four together measure 2.02x on the suite total and -62% on retained heap per document; on
npm run benchmark:html(the 12 MB page) parsing goes from 767/711 ms to 259/235 ms,cloneNode(true)from 591/527 ms to 84/85 ms and the total benchmark time from 4.23/4.09 s to 2.43/2.41 s. Each PR can be taken or left on its own; the numbers in each body are that branch measured alone againstmain.