perf(model): make the TreeNode tree type depth-safe (#343)
Building a TreeNode from a value Node, projecting it back via `node`, and releasing it each recursed one native frame per tree level, so a deeply nested document overflowed the call stack (signal 10 around 30-60k deep, and far shallower on a constrained stack such as a Swift Task). None of these is fixed by the serializer, which was already iterative. - `TreeNode(_:)` and `node`/`projectedChildren` now drive explicit post-order work stacks (entity-reference splicing, doctype/namespace dropping, and the root special-cases are unchanged). - `deinit` flattens the subtree into a heap worklist before releasing, so each node frees with empty children and the teardown stays flat. Only a node the teardown solely owns (isKnownUniquelyReferenced) is flattened, so a child still referenced elsewhere keeps its subtree. - The validator walk and the XPath descendant axis were made iterative for the same reason. A 60000-deep tree now releases in well under 0.1s; 1778 tests pass and no accepted output changes. Part of #341; the value Node's own recursive release and XPath result ordering/dedup remain depth-bounded and tracked there.
This commit is contained in:
parent
b05b428e30
commit
a77ffbd952
@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- The mutable `Model.TreeNode` tree type is now depth-safe: building one from a value `Node`, projecting it back with `node`, and releasing it all run in bounded native stack instead of recursing one frame per level. Construction (`TreeNode(_:)`) and projection (`node`/`projectedChildren`) drive explicit post-order work stacks, and `deinit` flattens its subtree into a heap worklist before releasing, so a deeply-nested tree no longer overflows the stack through a recursive chain of child releases. The teardown only flattens nodes it solely owns (`isKnownUniquelyReferenced`), so a child still referenced elsewhere keeps its subtree. A 60000-deep tree releases in well under a tenth of a second; behaviour for ordinary trees is unchanged. The validator walk and the XPath descendant axis (`AxisNavigation.appendDescendants`) were made iterative for the same reason. (Part of #341. The immutable value `Node`'s own recursive release, and XPath result ordering/dedup, are still depth-bounded and tracked there.)
|
||||
|
||||
- Rejecting a billion-laughs document is no longer exponential in nesting depth. The "does this entity produce markup" check (`splitAtMarkupEntity` / `producesMarkup`) recursed into every reference of every replacement without memoizing, so a chain `lol8 -> 10x lol7 -> 100x lol6 -> ...` was traversed O(references^depth) times. The check is now memoized per entity name (the property depends only on the fixed entity graph), making it O(entities). A classic eight-level billion-laughs with DTD processing enabled drops from about 18 s to about 0.6 s (now bounded by the expansion budget, not the traversal), with no change to which documents are accepted; a cyclic entity is still rejected.
|
||||
|
||||
- The No-Recursion check (`checkRecursion`) no longer re-traverses shared sub-chains. It validated the reference graph with only an in-progress (grey) set, so a billion-laughs graph was walked O(references^depth) times; a markup-producing eight-level document took about 31 s to reject. It is now a three-colour DFS that records each fully-verified entity (black set) and skips it on revisit, one pass over the distinct edges. The same document drops to about 11 s; cyclic entities are still rejected and no accepted document changes. (The residual cost is the re-injection re-read of the growing remainder on the markup-inclusion path, where `Reader.inject` re-maps the unconsumed tail each cycle; tracked in #339.)
|
||||
|
||||
@ -111,6 +111,35 @@ public extension PureXML.Model {
|
||||
}
|
||||
}
|
||||
|
||||
/// Tears the subtree down iteratively so releasing a deeply-nested tree
|
||||
/// stays in bounded native stack. The implicit release walk would
|
||||
/// otherwise recurse one frame per level (a chain of child releases), and
|
||||
/// a naive `deinit` that let each child deallocate in turn would just nest
|
||||
/// the same way. Instead, each node's children are moved into a flat heap
|
||||
/// worklist and cleared before that node is released, so its own `deinit`
|
||||
/// finds an empty `children` and returns at once: the teardown is flat.
|
||||
///
|
||||
/// Only a node this teardown uniquely owns is drained; a node still held
|
||||
/// elsewhere (an external reference into the tree) keeps its subtree
|
||||
/// intact and tears down later through this same `deinit`.
|
||||
deinit {
|
||||
guard !children.isEmpty else { return }
|
||||
var pending = children
|
||||
children = []
|
||||
while !pending.isEmpty {
|
||||
// `removeLast` moves the node into `node` as the sole binding, so
|
||||
// `isKnownUniquelyReferenced` sees no spurious reference (a
|
||||
// `while let`/`popLast` binding would itself count). A node still
|
||||
// referenced elsewhere keeps its subtree intact; only a node this
|
||||
// teardown solely owns is flattened, so its release stays shallow.
|
||||
var node = pending.removeLast()
|
||||
if isKnownUniquelyReferenced(&node), !node.children.isEmpty {
|
||||
pending.append(contentsOf: node.children)
|
||||
node.children = []
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: Factories
|
||||
|
||||
/// Creates a document node holding the given top-level children.
|
||||
|
||||
@ -1,15 +1,19 @@
|
||||
public extension PureXML.Model.TreeNode {
|
||||
/// Builds a mutable tree from a parsed value ``Node``, wiring up parent links.
|
||||
///
|
||||
/// The conversion is iterative, not recursive on depth: a deeply-nested
|
||||
/// document is built bottom-up through an explicit work stack so construction
|
||||
/// stays in bounded native stack (see ``buildForest(_:)``).
|
||||
convenience init(_ node: PureXML.Model.Node) {
|
||||
switch node {
|
||||
case let .document(children):
|
||||
self.init(adopting: .document, children: children.map(PureXML.Model.TreeNode.init))
|
||||
self.init(adopting: .document, children: Self.buildForest(children))
|
||||
case let .element(element):
|
||||
self.init(
|
||||
adopting: .element,
|
||||
name: element.name,
|
||||
attributes: element.attributes,
|
||||
children: element.children.map(PureXML.Model.TreeNode.init),
|
||||
children: Self.buildForest(element.children),
|
||||
)
|
||||
case let .text(value):
|
||||
self.init(kind: .text, value: value)
|
||||
@ -22,17 +26,46 @@ public extension PureXML.Model.TreeNode {
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds the `TreeNode` children for a forest of value ``Node``s without
|
||||
/// recursing on tree depth. A frame holds a branch node (element or document)
|
||||
/// being converted: the position of the next source child to process and the
|
||||
/// `TreeNode` children built so far. Leaves convert directly; a branch is
|
||||
/// assembled (adopting its built children, which sets their parent links) only
|
||||
/// once all its source children are done, then handed to its own parent frame.
|
||||
/// A synthetic document frame carries the roots; it is never assembled (that
|
||||
/// would re-parent them), so its built children are the result.
|
||||
private static func buildForest(_ roots: [PureXML.Model.Node]) -> [PureXML.Model.TreeNode] {
|
||||
let rootFrame = BuildFrame(.document(roots), roots)
|
||||
var stack: [BuildFrame] = [rootFrame]
|
||||
while let frame = stack.last {
|
||||
guard frame.next < frame.children.count else {
|
||||
stack.removeLast()
|
||||
guard let parent = stack.last else { break }
|
||||
parent.built.append(buildBranch(frame))
|
||||
continue
|
||||
}
|
||||
let child = frame.children[frame.next]
|
||||
frame.next += 1
|
||||
if let grandchildren = branchChildren(child) {
|
||||
stack.append(BuildFrame(child, grandchildren))
|
||||
} else {
|
||||
frame.built.append(buildLeaf(child))
|
||||
}
|
||||
}
|
||||
return rootFrame.built
|
||||
}
|
||||
|
||||
/// Rebuilds an immutable value ``Node`` from this subtree, for serialization
|
||||
/// or value comparison.
|
||||
/// or value comparison. Iterative on depth (see ``projectedChildren(of:)``).
|
||||
var node: PureXML.Model.Node {
|
||||
switch kind {
|
||||
case .document:
|
||||
.document(projectedChildren)
|
||||
.document(Self.projectedChildren(of: self))
|
||||
case .element:
|
||||
.element(PureXML.Model.Element(
|
||||
name: name ?? PureXML.Model.QualifiedName(""),
|
||||
attributes: attributes,
|
||||
children: projectedChildren,
|
||||
children: Self.projectedChildren(of: self),
|
||||
))
|
||||
case .text:
|
||||
.text(value)
|
||||
@ -52,19 +85,127 @@ public extension PureXML.Model.TreeNode {
|
||||
}
|
||||
}
|
||||
|
||||
/// The value-`Node` children of this subtree: doctype and namespace nodes are
|
||||
/// The value-`Node` children of `root`'s subtree, projected into a pure
|
||||
/// content tree without recursing on depth: doctype and namespace nodes are
|
||||
/// dropped (they carry no content), and an entity-reference node is spliced
|
||||
/// open into its replacement nodes, so the projection is a pure content tree.
|
||||
private var projectedChildren: [PureXML.Model.Node] {
|
||||
children.flatMap { child -> [PureXML.Model.Node] in
|
||||
/// open into its replacement nodes. A frame accumulates a branch node's
|
||||
/// projected children; on completion an element or document frame wraps them
|
||||
/// in the corresponding value `Node`, while an entity-reference frame splices
|
||||
/// them into its parent (no wrapper).
|
||||
private static func projectedChildren(of root: PureXML.Model.TreeNode) -> [PureXML.Model.Node] {
|
||||
let rootFrame = ProjectFrame(root, splice: false)
|
||||
var stack: [ProjectFrame] = [rootFrame]
|
||||
while let frame = stack.last {
|
||||
guard frame.next < frame.tree.children.count else {
|
||||
stack.removeLast()
|
||||
guard let parent = stack.last else { break }
|
||||
finishProjection(frame, into: parent)
|
||||
continue
|
||||
}
|
||||
let child = frame.tree.children[frame.next]
|
||||
frame.next += 1
|
||||
switch child.kind {
|
||||
case .doctype, .namespace:
|
||||
[]
|
||||
continue
|
||||
case .entityReference:
|
||||
child.projectedChildren
|
||||
stack.append(ProjectFrame(child, splice: true))
|
||||
case .element, .document:
|
||||
stack.append(ProjectFrame(child, splice: false))
|
||||
default:
|
||||
[child.node]
|
||||
frame.built.append(projectLeaf(child))
|
||||
}
|
||||
}
|
||||
return rootFrame.built
|
||||
}
|
||||
|
||||
// MARK: Build helpers
|
||||
|
||||
private static func branchChildren(_ node: PureXML.Model.Node) -> [PureXML.Model.Node]? {
|
||||
switch node {
|
||||
case let .document(children): children
|
||||
case let .element(element): element.children
|
||||
default: nil
|
||||
}
|
||||
}
|
||||
|
||||
private static func buildLeaf(_ node: PureXML.Model.Node) -> PureXML.Model.TreeNode {
|
||||
switch node {
|
||||
case let .text(value): PureXML.Model.TreeNode(kind: .text, value: value)
|
||||
case let .cdata(value): PureXML.Model.TreeNode(kind: .cdata, value: value)
|
||||
case let .comment(value): PureXML.Model.TreeNode(kind: .comment, value: value)
|
||||
case let .processingInstruction(target, data):
|
||||
PureXML.Model.TreeNode(kind: .processingInstruction, name: PureXML.Model.QualifiedName(target), value: data)
|
||||
// Unreachable: `buildLeaf` is only called when `branchChildren` returned
|
||||
// nil (a non-branch node). An empty text node keeps the conversion total.
|
||||
case .document, .element: PureXML.Model.TreeNode(kind: .text, value: "")
|
||||
}
|
||||
}
|
||||
|
||||
private static func buildBranch(_ frame: BuildFrame) -> PureXML.Model.TreeNode {
|
||||
switch frame.node {
|
||||
case let .element(element):
|
||||
PureXML.Model.TreeNode(adopting: .element, name: element.name, attributes: element.attributes, children: frame.built)
|
||||
// Only branch nodes (document or element) get a frame; a non-branch frame
|
||||
// is unreachable, so the document shape is a total fallback.
|
||||
default:
|
||||
PureXML.Model.TreeNode(adopting: .document, children: frame.built)
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: Projection helpers
|
||||
|
||||
private static func projectLeaf(_ tree: PureXML.Model.TreeNode) -> PureXML.Model.Node {
|
||||
switch tree.kind {
|
||||
case .cdata: .cdata(tree.value)
|
||||
case .comment: .comment(tree.value)
|
||||
case .processingInstruction:
|
||||
.processingInstruction(target: tree.name?.description ?? "", data: tree.value)
|
||||
// doctype/namespace are dropped before reaching here; everything else
|
||||
// projects as the text leaf.
|
||||
default: .text(tree.value)
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolves a completed frame into `parent`'s children: an entity-reference
|
||||
/// frame splices its projected nodes in, an element wraps them, and any other
|
||||
/// branch (a document) wraps them as a document.
|
||||
private static func finishProjection(_ frame: ProjectFrame, into parent: ProjectFrame) {
|
||||
if frame.splice {
|
||||
parent.built.append(contentsOf: frame.built)
|
||||
} else if case .element = frame.tree.kind {
|
||||
parent.built.append(.element(PureXML.Model.Element(
|
||||
name: frame.tree.name ?? PureXML.Model.QualifiedName(""),
|
||||
attributes: frame.tree.attributes,
|
||||
children: frame.built,
|
||||
)))
|
||||
} else {
|
||||
parent.built.append(.document(frame.built))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A branch node being converted from a value ``Node`` to a ``TreeNode``: the
|
||||
/// source children, the cursor into them, and the converted children so far.
|
||||
private final class BuildFrame {
|
||||
let node: PureXML.Model.Node
|
||||
let children: [PureXML.Model.Node]
|
||||
var next = 0
|
||||
var built: [PureXML.Model.TreeNode] = []
|
||||
init(_ node: PureXML.Model.Node, _ children: [PureXML.Model.Node]) {
|
||||
self.node = node
|
||||
self.children = children
|
||||
}
|
||||
}
|
||||
|
||||
/// A ``TreeNode`` being projected back to a value ``Node``: `splice` marks an
|
||||
/// entity-reference frame whose projected children fold into its parent.
|
||||
private final class ProjectFrame {
|
||||
let tree: PureXML.Model.TreeNode
|
||||
let splice: Bool
|
||||
var next = 0
|
||||
var built: [PureXML.Model.Node] = []
|
||||
init(_ tree: PureXML.Model.TreeNode, splice: Bool) {
|
||||
self.tree = tree
|
||||
self.splice = splice
|
||||
}
|
||||
}
|
||||
|
||||
@ -149,40 +149,43 @@ public extension PureXML.Validation {
|
||||
if !toThrow.isEmpty { throw ValidationErrorCollection(values: toThrow) }
|
||||
}
|
||||
|
||||
private func walk(_ node: PureXML.Model.Node, path: [PathKey], _ document: Document, _ findings: inout [ValidationError]) {
|
||||
apply(node, at: path, in: document, into: &findings)
|
||||
switch node {
|
||||
case let .document(children):
|
||||
walkChildren(children, path: path, document, &findings)
|
||||
case let .element(element):
|
||||
apply(element, at: path, in: document, into: &findings)
|
||||
for attribute in element.attributes {
|
||||
apply(attribute, at: path + [.attribute(attribute.name.localName)], in: document, into: &findings)
|
||||
}
|
||||
walkChildren(element.children, path: path, document, &findings)
|
||||
case .text, .cdata, .comment, .processingInstruction:
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
private func walkChildren(
|
||||
_ children: [PureXML.Model.Node],
|
||||
path: [PathKey],
|
||||
_ document: Document,
|
||||
_ findings: inout [ValidationError],
|
||||
) {
|
||||
let elementNames = children.compactMap { child -> String? in
|
||||
guard case let .element(element) = child else { return nil }
|
||||
return element.name.description
|
||||
}
|
||||
var steps = PathKey.steps(forChildNames: elementNames).makeIterator()
|
||||
for child in children {
|
||||
guard case .element = child else {
|
||||
walk(child, path: path, document, &findings)
|
||||
/// Validates `root` and its subtree in document order. The walk is
|
||||
/// iterative, driven by an explicit `(node, path)` stack, so a deeply
|
||||
/// nested document does not overflow the call stack; children are pushed
|
||||
/// reversed so they pop in document order, giving the same pre-order
|
||||
/// finding sequence a recursive descent would.
|
||||
private func walk(_ root: PureXML.Model.Node, path rootPath: [PathKey], _ document: Document, _ findings: inout [ValidationError]) {
|
||||
var stack: [(node: PureXML.Model.Node, path: [PathKey])] = [(root, rootPath)]
|
||||
while let (node, path) = stack.popLast() {
|
||||
apply(node, at: path, in: document, into: &findings)
|
||||
let children: [PureXML.Model.Node]
|
||||
switch node {
|
||||
case let .document(documentChildren):
|
||||
children = documentChildren
|
||||
case let .element(element):
|
||||
apply(element, at: path, in: document, into: &findings)
|
||||
for attribute in element.attributes {
|
||||
apply(attribute, at: path + [.attribute(attribute.name.localName)], in: document, into: &findings)
|
||||
}
|
||||
children = element.children
|
||||
case .text, .cdata, .comment, .processingInstruction:
|
||||
continue
|
||||
}
|
||||
let step = steps.next() ?? .element("")
|
||||
walk(child, path: path + [step], document, &findings)
|
||||
let elementNames = children.compactMap { child -> String? in
|
||||
guard case let .element(element) = child else { return nil }
|
||||
return element.name.description
|
||||
}
|
||||
var steps = PathKey.steps(forChildNames: elementNames).makeIterator()
|
||||
var childItems: [(node: PureXML.Model.Node, path: [PathKey])] = []
|
||||
childItems.reserveCapacity(children.count)
|
||||
for child in children {
|
||||
if case .element = child {
|
||||
childItems.append((child, path + [steps.next() ?? .element("")]))
|
||||
} else {
|
||||
childItems.append((child, path))
|
||||
}
|
||||
}
|
||||
stack.append(contentsOf: childItems.reversed())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -72,9 +72,15 @@ extension PureXML.XPath {
|
||||
/// wrapped ``Node`` means the traversal builds no intermediate per-level
|
||||
/// arrays and wraps each node exactly once, when it is appended.
|
||||
private static func appendDescendants(of tree: PureXML.Model.TreeNode, into result: inout [Node]) {
|
||||
for child in tree.children {
|
||||
result.append(.tree(child))
|
||||
appendDescendants(of: child, into: &result)
|
||||
// Iterative pre-order walk: a deeply-nested tree would overflow the
|
||||
// call stack if descended recursively. Children are pushed reversed so
|
||||
// they pop in document order.
|
||||
var stack = Array(tree.children.reversed())
|
||||
while let node = stack.popLast() {
|
||||
result.append(.tree(node))
|
||||
if !node.children.isEmpty {
|
||||
stack.append(contentsOf: node.children.reversed())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -104,9 +110,14 @@ extension PureXML.XPath {
|
||||
where keep: (PureXML.Model.TreeNode) -> Bool,
|
||||
into result: inout [Node],
|
||||
) {
|
||||
for child in tree.children {
|
||||
if keep(child) { result.append(.tree(child)) }
|
||||
appendDescendants(of: child, where: keep, into: &result)
|
||||
// Iterative pre-order walk (see the unfiltered overload); children are
|
||||
// pushed reversed so they pop in document order.
|
||||
var stack = Array(tree.children.reversed())
|
||||
while let node = stack.popLast() {
|
||||
if keep(node) { result.append(.tree(node)) }
|
||||
if !node.children.isEmpty {
|
||||
stack.append(contentsOf: node.children.reversed())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
59
Tests/TreeNodeDepthSafetyTests.swift
Normal file
59
Tests/TreeNodeDepthSafetyTests.swift
Normal file
@ -0,0 +1,59 @@
|
||||
import Testing
|
||||
@testable import PureXML
|
||||
|
||||
/// Depth safety for the mutable ``TreeNode`` tree type (#341): building a tree
|
||||
/// from a value ``Node``, projecting it back, and releasing it must all run in
|
||||
/// bounded native stack, so a deeply-nested document cannot overflow the call
|
||||
/// stack through a recursive walk or a recursive chain of child releases.
|
||||
///
|
||||
/// The release test builds its tree bottom-up through the adopting initializer
|
||||
/// rather than from a parsed value ``Node``: a value `Node` that deep still
|
||||
/// releases recursively (its storage change is tracked separately in #341), so
|
||||
/// using one as a fixture would itself overflow on teardown and mask what is
|
||||
/// under test here.
|
||||
@Suite("TreeNode depth safety")
|
||||
struct TreeNodeDepthSafetyTests {
|
||||
/// A chain of `depth` nested elements, built bottom-up so construction never
|
||||
/// recurses and holds no deep value `Node`.
|
||||
private func deepChain(_ depth: Int) -> PureXML.Model.TreeNode {
|
||||
var node = PureXML.Model.TreeNode(adopting: .element, name: PureXML.Model.QualifiedName("a"), children: [])
|
||||
for _ in 0 ..< depth {
|
||||
node = PureXML.Model.TreeNode(adopting: .element, name: PureXML.Model.QualifiedName("a"), children: [node])
|
||||
}
|
||||
return node
|
||||
}
|
||||
|
||||
@Test("releasing a 60k-deep tree does not overflow the stack")
|
||||
func test_deepReleaseIsBounded() {
|
||||
var root: PureXML.Model.TreeNode? = deepChain(60000)
|
||||
root = nil
|
||||
#expect(root == nil)
|
||||
}
|
||||
|
||||
@Test("a held attached child keeps its subtree when the root is released")
|
||||
func test_heldChildKeepsSubtreeOnRootRelease() {
|
||||
let leaf = PureXML.Model.TreeNode.element("leaf")
|
||||
let mid = PureXML.Model.TreeNode.element("mid", children: [leaf])
|
||||
var root: PureXML.Model.TreeNode? = PureXML.Model.TreeNode.element("root", children: [mid])
|
||||
root = nil
|
||||
// The teardown only flattens nodes it solely owns; `mid` is still held, so
|
||||
// its subtree must remain intact rather than be cleared.
|
||||
#expect(mid.children.count == 1)
|
||||
#expect(mid.children.first === leaf)
|
||||
#expect(leaf.name?.description == "leaf")
|
||||
}
|
||||
|
||||
@Test("Node to TreeNode and back round-trips through the iterative conversions")
|
||||
func test_conversionRoundTrip() throws {
|
||||
// Held at the default depth bound: a value `Node` this deep still releases
|
||||
// recursively, and a Swift Task's stack (this test runs on one) overflows
|
||||
// far shallower than the main thread, so a deeper fixture would crash on
|
||||
// teardown for a reason unrelated to the conversions under test (#341).
|
||||
let depth = 200
|
||||
let xml = String(repeating: "<a x=\"1\">", count: depth) + "leaf" + String(repeating: "</a>", count: depth)
|
||||
let node = try PureXML.parse(xml)
|
||||
let tree = PureXML.Model.TreeNode(node)
|
||||
let back = tree.node
|
||||
#expect(PureXML.serialize(back, options: .compact) == PureXML.serialize(node, options: .compact))
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user