diff --git a/CHANGELOG.md b/CHANGELOG.md index 20852cf..304f913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The immutable value `Model.Node` tree type is now depth-safe. An `Element` previously stored its children inline (the enum was `indirect`), so a `Node`'s recursive layout made building, comparing, hashing, and releasing a deeply-nested tree recurse one native frame per level and overflow the stack (on a constrained stack such as a Swift Task, as shallow as a few thousand deep). `Element` now holds its children behind a copy-on-write reference (`ElementStorage`): the `Node` enum is no longer `indirect`, so it is a fixed-size value, and `==`, `hashValue`, and release all walk the tree iteratively through explicit work stacks. Release flattens only storage it solely owns, so a copy-on-write sibling keeps its subtree. Value semantics are unchanged (copying a node still copies its children, lazily) and `Element`/`Node` remain `Equatable`, `Hashable`, and `Sendable`. A 50000-deep tree now builds, compares, hashes, and releases in a fraction of a second with no crash. (Completes the value-`Node` half of #341.) + - 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. diff --git a/Sources/Model/Element.swift b/Sources/Model/Element.swift index eeaed3a..97cbd36 100644 --- a/Sources/Model/Element.swift +++ b/Sources/Model/Element.swift @@ -1,9 +1,41 @@ public extension PureXML.Model { /// An XML element: a qualified name, ordered attributes, and ordered child nodes. - struct Element: Equatable, Hashable, Sendable { + /// + /// A value type with full value semantics: copying an element copies its + /// children (lazily, copy-on-write). The children are held behind a reference + /// (``ElementStorage``) so a deeply-nested tree neither overflows the stack on + /// release nor on equality and hashing, which walk the tree iteratively. + struct Element: Sendable { public var name: QualifiedName public var attributes: [Attribute] - public var children: [Node] + /// Copy-on-write backing; `children` is the public, value-semantic view. + var storage: ElementStorage + + /// The element's child nodes. Reads share the backing storage; a write + /// first copies the storage if it is shared, preserving value semantics. + /// + /// The `_modify` accessor yields the backing array in place (after the + /// copy-on-write check), so an in-place mutation such as + /// `element.children.append(_:)` does not copy the whole array on every + /// call. Without it a computed property would read, mutate a temporary, + /// and write back, making repeated appends quadratic (which is exactly the + /// pattern the parser uses while building an element). + public var children: [Node] { + get { storage.children } + _modify { + if !isKnownUniquelyReferenced(&storage) { + storage = ElementStorage(storage.children) + } + yield &storage.children + } + set { + if isKnownUniquelyReferenced(&storage) { + storage.children = newValue + } else { + storage = ElementStorage(newValue) + } + } + } public init( name: QualifiedName, @@ -12,7 +44,7 @@ public extension PureXML.Model { ) { self.name = name self.attributes = attributes - self.children = children + storage = ElementStorage(children) } public init( @@ -40,3 +72,16 @@ public extension PureXML.Model { } } } + +extension PureXML.Model.Element: Equatable, Hashable { + /// Value equality over the whole subtree, compared iteratively (see + /// ``PureXML/Model/Node`` `==`) so a deeply-nested element does not recurse. + public static func == (lhs: Self, rhs: Self) -> Bool { + PureXML.Model.Node.treesEqual(.element(lhs), .element(rhs)) + } + + /// Hashes the whole subtree iteratively, consistent with `==`. + public func hash(into hasher: inout Hasher) { + PureXML.Model.Node.element(self).hash(into: &hasher) + } +} diff --git a/Sources/Model/ElementStorage.swift b/Sources/Model/ElementStorage.swift new file mode 100644 index 0000000..cb674d6 --- /dev/null +++ b/Sources/Model/ElementStorage.swift @@ -0,0 +1,40 @@ +extension PureXML.Model { + /// Copy-on-write backing for an ``Element``'s children. + /// + /// `Element` is a value type, but its children must live behind a reference so + /// the tree's depth does not become part of any value's recursive layout: a + /// chain of value-nested children would otherwise release one native frame per + /// level and overflow the stack on a deeply-nested document. Holding children + /// in this class gives the tree exactly one place to release them, where a + /// custom `deinit` can flatten the subtree iteratively. + /// + /// `@unchecked Sendable`: the class is mutable, but ``Element`` only ever + /// mutates `children` after copying a shared box (the copy-on-write check in + /// `Element.children`'s setter), so a box reachable from more than one value is + /// never written. That is the same discipline the standard library's own + /// copy-on-write containers rely on to be `Sendable`. + final class ElementStorage: @unchecked Sendable { + var children: [PureXML.Model.Node] + + init(_ children: [PureXML.Model.Node]) { + self.children = children + } + + /// Releases the subtree without recursing on depth. A naive release walks + /// one native frame per level (each box releasing its children's boxes); + /// instead each uniquely-owned descendant's children are moved into a flat + /// heap worklist and cleared before that node is released, so every box + /// frees with empty children and the teardown stays flat. A node whose + /// storage is still shared (a copy-on-write sibling holds it) is left + /// intact and tears down later through this same `deinit`. + deinit { + guard !children.isEmpty else { return } + var pending = children + children = [] + while !pending.isEmpty { + var node = pending.removeLast() + node.drainUniquelyOwnedChildren(into: &pending) + } + } + } +} diff --git a/Sources/Model/Node.swift b/Sources/Model/Node.swift index 6f05c68..a40d41d 100644 --- a/Sources/Model/Node.swift +++ b/Sources/Model/Node.swift @@ -2,7 +2,13 @@ public extension PureXML.Model { /// A node in an XML tree. The model preserves document order and the /// distinction between text, CDATA, comments, and processing instructions /// so that emitting can round-trip a parsed document. - indirect enum Node: Equatable, Hashable, Sendable { + /// + /// Not `indirect`: an element holds its children behind a reference + /// (``ElementStorage``), so a `Node` is a fixed-size value and the tree's + /// depth is not part of any value's layout. Equality, hashing, and release + /// all walk the tree iteratively, so an arbitrarily deep document never + /// overflows the call stack. + enum Node: Sendable { /// The root of a parsed document, holding its prolog and root element. case document([Node]) /// An element with a name, attributes, and children. @@ -23,3 +29,103 @@ public extension PureXML.Model { } } } + +extension PureXML.Model.Node: Equatable, Hashable { + public static func == (lhs: Self, rhs: Self) -> Bool { + treesEqual(lhs, rhs) + } + + /// Whether two node trees are equal, compared iteratively through an explicit + /// pair stack so a deeply-nested tree does not recurse one frame per level. + static func treesEqual(_ lhs: Self, _ rhs: Self) -> Bool { + var stack: [(Self, Self)] = [(lhs, rhs)] + while let (left, right) = stack.popLast() { + switch (left, right) { + case let (.element(leftElement), .element(rightElement)): + guard leftElement.name == rightElement.name, + leftElement.attributes == rightElement.attributes, + leftElement.children.count == rightElement.children.count + else { return false } + for pair in zip(leftElement.children, rightElement.children) { + stack.append(pair) + } + case let (.document(leftChildren), .document(rightChildren)): + guard leftChildren.count == rightChildren.count else { return false } + for pair in zip(leftChildren, rightChildren) { + stack.append(pair) + } + default: + if !leavesEqual(left, right) { return false } + } + } + return true + } + + /// Equality for the non-branch node kinds (and a kind mismatch); the branch + /// kinds are walked iteratively by ``treesEqual(_:_:)``. + private static func leavesEqual(_ left: Self, _ right: Self) -> Bool { + switch (left, right) { + case let (.text(leftValue), .text(rightValue)): leftValue == rightValue + case let (.cdata(leftValue), .cdata(rightValue)): leftValue == rightValue + case let (.comment(leftValue), .comment(rightValue)): leftValue == rightValue + case let (.processingInstruction(leftTarget, leftData), .processingInstruction(rightTarget, rightData)): + leftTarget == rightTarget && leftData == rightData + default: false + } + } + + /// Hashes the whole subtree iteratively (pre-order), consistent with + /// ``treesEqual(_:_:)``: equal trees feed the hasher the same sequence. + public func hash(into hasher: inout Hasher) { + var stack: [Self] = [self] + while let node = stack.popLast() { + switch node { + case let .element(element): + hasher.combine(0) + hasher.combine(element.name) + hasher.combine(element.attributes) + hasher.combine(element.children.count) + for child in element.children { + stack.append(child) + } + case let .document(children): + hasher.combine(1) + hasher.combine(children.count) + for child in children { + stack.append(child) + } + case let .text(value): + hasher.combine(2) + hasher.combine(value) + case let .cdata(value): + hasher.combine(3) + hasher.combine(value) + case let .comment(value): + hasher.combine(4) + hasher.combine(value) + case let .processingInstruction(target, data): + hasher.combine(5) + hasher.combine(target) + hasher.combine(data) + } + } + } +} + +extension PureXML.Model.Node { + /// Used by ``ElementStorage`` teardown: if this is an element whose backing + /// storage this node solely owns, move its children into `pending` and clear + /// them so the node releases flat; otherwise leave it untouched. Setting + /// `self` aside before the uniqueness check drops this node's own hold on the + /// storage, so the check counts only references held elsewhere (shared + /// copy-on-write siblings), which must keep their subtree. + mutating func drainUniquelyOwnedChildren(into pending: inout [Self]) { + guard case var .element(element) = self else { return } + self = .text("") + if isKnownUniquelyReferenced(&element.storage), !element.storage.children.isEmpty { + pending.append(contentsOf: element.storage.children) + element.storage.children = [] + } + self = .element(element) + } +} diff --git a/Tests/ValueNodeDepthSafetyTests.swift b/Tests/ValueNodeDepthSafetyTests.swift new file mode 100644 index 0000000..8c9b5f7 --- /dev/null +++ b/Tests/ValueNodeDepthSafetyTests.swift @@ -0,0 +1,69 @@ +import Testing +@testable import PureXML + +/// Depth safety and value semantics for the immutable value ``Node`` (#341). +/// An element holds its children behind a copy-on-write reference, so a `Node` +/// is a fixed-size value and building, comparing, hashing, and releasing a +/// deeply-nested tree all run in bounded native stack rather than recursing one +/// frame per level. These trees are built bottom-up (not parsed) so the test +/// exercises the value type directly and stays fast. +@Suite("Value Node depth safety") +struct ValueNodeDepthSafetyTests { + /// A chain of `depth` nested single-child elements with a text leaf. + private func deepNode(_ depth: Int) -> PureXML.Model.Node { + var node: PureXML.Model.Node = .text("leaf") + for _ in 0 ..< depth { + node = .element(.init("a", attributes: [.init("x", "1")], children: [node])) + } + return node + } + + @Test("building, comparing, hashing, and releasing a 50k-deep tree do not overflow") + func test_deepTreeIsBounded() { + var node: PureXML.Model.Node? = deepNode(50000) + let other = deepNode(50000) + #expect(node == other) // iterative equality over the whole depth + var hasher = Hasher() + node.hash(into: &hasher) // iterative hashing over the whole depth + _ = hasher.finalize() + node = nil // iterative release + #expect(node == nil) + } + + @Test("equal deep trees hash equally; an unequal one differs") + func test_hashMatchesEquality() { + let first = deepNode(2000) + let second = deepNode(2000) + let shorter = deepNode(1999) + #expect(first == second) + #expect(first != shorter) + #expect(first.hashValue == second.hashValue) + } + + @Test("copy-on-write: mutating a copy leaves the original untouched") + func test_copyOnWriteValueSemantics() { + var original = PureXML.Model.Element("root", children: [.element(.init("child"))]) + let copy = original + original.children.append(.text("added")) + #expect(original.children.count == 2) + #expect(copy.children.count == 1) + #expect(original != copy) + } + + @Test("a shared subtree survives when one owner is released") + func test_sharedSubtreeSurvivesPartialRelease() { + let shared = deepNode(3000) + var holder: PureXML.Model.Node? = .element(.init("wrap", children: [shared])) + holder = nil // releasing the wrapper must not tear down `shared` + #expect(holder == nil) + // `shared` is still usable: walking it does not crash and finds the leaf. + var current = shared + var steps = 0 + while case let .element(element) = current, let first = element.children.first { + current = first + steps += 1 + } + #expect(steps == 3000) + #expect(current == .text("leaf")) + } +} diff --git a/docs/validation-coverage-registry.txt b/docs/validation-coverage-registry.txt index f50c5b8..69ea933 100644 --- a/docs/validation-coverage-registry.txt +++ b/docs/validation-coverage-registry.txt @@ -11,6 +11,7 @@ SUBJECT PureXML.Schema.SchemaTypeFact xsd-schema-consistency SUBJECT PureXML.Schema.ResolvedElement xsd-streaming-shallow EXCLUDE PureXML.Model.QualifiedName value type; lexical invariants at construction +EXCLUDE PureXML.Model.ElementStorage copy-on-write backing for Element children; validated via Node EXCLUDE PureXML.Model.TreeNode mutable editor view of Node; validated via Node EXCLUDE PureXML.Model.TreeNodeKind tag enum; no invariants EXCLUDE PureXML.LintDiagnostic editor output; not user input