Replace SymbolTree with a DOM-specific tree - #4308
Conversation
domenic
left a comment
There was a problem hiding this comment.
This looks very promising. I haven't gone into full depth, but I think we could do better from a code readability perspective. To me this looks too much like the old shape.
For example:
- I don't think we need to use a symbol; we can just use
node._linksor similar. - I don't like the
internal-constants.jsfile or the various indirections like threading it throughshadow-dom.js. - At the very least, we should change helper calls from e.g.
domSymbolTree.index(childImpl)totreeHelpers.index(childImpl)ortreeOps.index(childImpl). But I think it'd be nicer if we could do something likechildImpl._treeIndex().
I'd also prefer to avoid adding unit tests of internals in test/api. if we think they're really valuable, we can consider a new test/unit/ or something, but I'm hopeful the coverage isn't needed since WPTs cover everything exhaustively.
|
@domenic its a really interesting area! Looking around for inspiration that already exists was pretty neat. Switched to _links, removed the import indirection, and moved index calculation onto NodeImpl. Removed the internal tests and kept the useful coverage in WPTs. |
3848ea0 to
72520b3
Compare
|
I set Astra Extra High loose on this and it found more cleanups and performance improvements, especially in ranges. I'm going to move github.com//pull/4308/commits/1feedc0cd62401ac6aceafbb194497606e840b2a to another PR, along with the same fix for processing instructions, then rebase this on that... |
|
Also using astra xhigh I ran the benchmarks before and after your commits on Node 26:
Pushed a follow-up to skip unrelated subtrees in |
Move tree storage and traversal into jsdom, keeping bookkeeping lazy for unattached nodes. Preserve unaffected sibling indexes across mutations and calculate mutation indexes only when live Ranges need them. Handle shadow-including traversal directly in the tree layer, avoiding recursive generators while preserving mutation lookahead. Fixes jsdom#4291 Co-authored-by: codex <codex@openai.com>
Use node fields and getters directly and remove the SymbolTree import indirection. Move the useful internal test coverage into WPTs. Co-authored-by: codex <codex@openai.com>
Remove unused traversal options and walk sibling links directly in the element getters. Add benchmarks for adjacent elements and mixed nodes. Co-authored-by: codex <codex@openai.com>
Check that indexed subtrees can be collected after removal or moves, and that error listeners can insert light children during shadow traversal without repeating their insertion steps. Co-authored-by: codex <codex@openai.com>
Keep unallocated links as null and read allocated records directly. Reuse the existing position constants and remove the redundant traversal wrapper and unused selector option. Co-authored-by: codex <codex@openai.com>
Co-authored-by: codex <codex@openai.com>
Expose traversal, collection building, tree position, and common-ancestor operations as underscored methods on NodeImpl instances. Keep link allocation, index caching, and raw link mutations in dom-tree.js, with NodeImpl as its only consumer, so DOM algorithms can work with nodes without depending on the tree storage implementation. Distinguish children, descendants, and inclusive descendants in the method names. Update collection queries to request descendants directly instead of repeatedly filtering out the root. Use standard Iterator subclasses for traversal and native iterator helpers for searches, while retaining direct array builders for collection refreshes to avoid allocating iterator results and intermediate filter iterators on these frequent paths. Share common-ancestor discovery between Range operations and tree-position comparisons. Walk the ancestor paths together and retain the diverging children for comparisons, avoiding repeated ancestor searches in deeply nested ranges. Reuse bounded next-node and next-after-subtree operations in Range and NodeIterator instead of maintaining separate traversal implementations. Use sibling and parent links directly for simple walks, including node equality, normalization, cache invalidation, and label lookup. Remove the unused ancestor and sibling iterator classes, simultaneousIterators, and redundant root and traversal helpers. Preserve normalization's initial snapshot and the traversal iterators' behavior when consumers mutate the tree, including shadow-tree traversal. Add public-facing coverage for Range common ancestors, cloning, and extraction after subtree moves, plus benchmarks for common-ancestor lookup and live collection refreshes. Replace benchmarks that invoke internal shadow traversal directly with customElements.upgrade() over the same tree shapes, and remove the artificial early-exit cases. Co-authored-by: codex <codex@openai.com>
Use the root and containment operations on node implementations throughout the DOM algorithms. Keep Document-root caching and path compression in getRootNode(), with detached roots left uncached because their mutations do not invalidate descendant root caches. Represent the DOM concept of node length with an internal getter overridden by CharacterDataImpl. Retain a separate child-count getter for algorithms such as selectAllChildren() that explicitly count children, even on character-data nodes. This also fixes CDATA range offsets, which previously treated CDATA sections as having zero length. Recognize Text and CharacterData subclasses throughout Range operations so CDATA boundaries support cloning, extraction, deletion, insertion, and stringification. Return the common ancestor together with its child on each boundary path. Reuse those children for boundary comparisons, collapsed positions, and shared clone/extract preparation, avoiding repeated ancestor walks and scans through unrelated siblings. Snapshot fully contained children and reject contained doctypes before mutations begin. Normalize only descendant Text nodes. Calling normalize() on a Text node must leave that node and its siblings unchanged; the previous inclusive traversal could remove an empty receiver or merge adjacent siblings into it. Add public Range benchmarks for deep and unequal boundary paths, deep shared ancestors, and unrelated siblings. Cover the normalization and CDATA fixes with web platform regressions, and enable four existing Range tests that now pass. Co-authored-by: codex <codex@openai.com>
Run nine Range WPT files previously skipped for failures or timeouts. The cloning fixes and improvements to character data handling and tree traversal allow their fixtures and assertions to pass in normal test runs. Keep the insertion suite skipped until its incorrect exception realms are fixed. Co-authored-by: codex <codex@openai.com>
Check the common ancestor's two partial boundary children in surroundContents() instead of scanning its whole subtree. A non-Text partial child is enough to reject the operation, while a Text partial child cannot have descendants. This removes work proportional to unrelated content under the common ancestor. When collecting nodes for deleteContents(), skip descendants of each contained node selected for removal. This produces only the outermost contained nodes without a separate parent-containment check. Preserve the snapshot before mutations so subsequent removal and boundary adjustments retain their ordering. Add public API benchmarks for deleting a fully contained deep subtree and surrounding adjacent text with unrelated siblings. Co-authored-by: codex <codex@openai.com>
Start stringification and deletion at the boundary offsets instead of walking unselected subtrees. Return immediately for collapsed ranges and preserve the deletion snapshot. Co-authored-by: codex <codex@openai.com>
Give deletion its own tree so stringification can run without resetting the fixture before each sample. Co-authored-by: codex <codex@openai.com>
93193b8 to
535b7d7
Compare
This came out of profiling jsdom in my DOM benchmark suite.
Replace SymbolTree with a DOM-specific tree stored in
NodeImpl._links. Keep link bookkeeping in the tree helper and expose traversal and tree comparisons as methods on nodes.Allocate links only when needed, preserve sibling indexes before mutations, and calculate mutation indexes only when live Ranges need them. Walk shadow trees with an explicit stack while preserving behavior when callbacks change the tree.
Limit Range stringification and deletion to the selected content, and return immediately when stringifying a collapsed Range. Reuse common-ancestor results in Range operations. Skip descendants of nodes being deleted, and check the two partially contained children in
surroundContents()instead of scanning unrelated content.A few places we looked for inspiration:
Execution time measured at 39edbb73 versus the SymbolTree base, measured on Node 26.8.2 in two paired runs. Percentages combine both runs; cloning varied enough to show the observed range.
A live Range is a DOM
Rangewhose start and end positions automatically update when the tree changes. Keeping those positions up to date adds work to mutations.customElements.upgrade(), flat/deep/field-heavy treesRange.deleteContents(), fully contained deep subtreeRange.surroundContents(), adjacent text with 1,000 unrelated siblingsFixes #4291