perf(parsing): reuse one scratch set for the attribute duplicate-name check

Fourth profile-guided pass (#139, docs/benchmarks.md pass 28). scanAttributes
allocated a fresh Set<String>'s native storage on every element with
attributes, just to check a handful of names for duplicates; the set is now a
stored EventReader scratch property cleared with removeAll(keepingCapacity:)
per element, so each element's check reuses the previous element's storage.
Safe because scanAttributes cannot re-enter itself (nothing inside an
attribute scan starts another start-tag scan), so the shared storage never
sees two interleaved scans; duplicate detection, recovering-mode behavior,
and thrown errors are unchanged.

Verified in an isolated worktree at the previous commit: full local suite
(1858 tests) and full W3C XSTS suite (14383 groups) pass unchanged. Measured
with an interleaved same-state A/B, baseline/patched/baseline/patched each
best-of-5, because the machine carried concurrent load this session: both
patched runs (0.1073s, 0.1067s) beat both baseline runs (0.1091s, 0.1117s),
~2-4% on parse.

Recorded alongside, per the benchmarks log's discipline: a sibling candidate
(reserving each tree frame's children capacity on first append) measured
neutral under the same protocol and was reverted rather than landed, and the
TreeNode weak-parent side-table cost (~9% of parse, swift_weakAssign per
child plus a runtime side table per parent) is documented under Known
structure of the gap as needing a design decision, since parent's
nil-after-release semantics are documented public API. Touched lines carry
repo-standard swiftformat rewraps applied by a concurrent formatter run.

CHANGELOG.md

@@ -50,6 +50,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- A fourth parse-path optimization pass (#139, `docs/benchmarks.md` pass 28):
+  `scanAttributes`' duplicate-name check reuses one stored scratch
+  `Set<String>` (cleared with `removeAll(keepingCapacity: true)` per element)
+  instead of allocating fresh native Set storage for every element with
+  attributes. Safe because `scanAttributes` cannot re-enter itself; duplicate
+  detection, recovering-mode behavior, and thrown errors are unchanged (full
+  local suite and full W3C XSTS pass unchanged). Measured ~2-4% on parse with
+  an interleaved same-state A/B (both patched runs beat both baseline runs),
+  the protocol chosen because the session had concurrent machine load. Two
+  companion outcomes from the same profiling round, recorded rather than
+  hidden: a children-capacity reservation candidate measured neutral and was
+  reverted per the benchmarks log's ship-only-measured-wins rule, and the
+  `TreeNode` weak-parent side-table cost (~9% of parse) is documented in
+  `docs/benchmarks.md` as a structural finding that needs a design decision,
+  not an optimization pass, since `parent`'s nil-after-release semantics are
+  documented public API.
+
 - A third parse-path optimization pass (#139, `docs/benchmarks.md` pass 27),
   found by sample-profiling `scanContentMarkup()` in isolation after passes
   25-26 (it dominates parse time, ~46%, since it inlines

Sources/Parsing/EventReader.swift

@@ -26,6 +26,13 @@ public extension PureXML.Parsing {
         var entityBudget: Int
 
         var pending: [Event] = []
+        /// Scratch storage for `scanAttributes`' duplicate-name check, hoisted
+        /// to a stored property so one element's check reuses the previous
+        /// element's `Set` storage (`removeAll(keepingCapacity: true)`) instead
+        /// of allocating fresh native storage per element with attributes.
+        /// Safe to share because `scanAttributes` is not reentrant: nothing
+        /// inside an attribute scan can start another start-tag scan.
+        var attributeNamesSeen: Set<String> = []
         private var sawRoot = false
         private var primed = false
         /// When true, ``next()`` repairs malformed input in place instead of
@@ -81,12 +88,16 @@ public extension PureXML.Parsing {
                 guard recovering else {
                     return open.isEmpty ? try nextAtTopLevel() : try nextInContent()
                 }
-                if reachedEnd { return nil }
+                if reachedEnd {
+                    return nil
+                }
                 let start = offset
                 do {
                     return open.isEmpty ? try nextAtTopLevel() : try nextInContent()
                 } catch let error as ParseError {
-                    if let event = recoveredEvent(from: error, startingAt: start) { return event }
+                    if let event = recoveredEvent(from: error, startingAt: start) {
+                        return event
+                    }
                 }
             }
         }
@@ -153,16 +164,24 @@ public extension PureXML.Parsing {
                 // spliced entity text, or a non-ASCII lead.
                 if let byte = reader.peekByte() {
                     eventStart = reader.mark
-                    if byte == 0x3C { return try scanContentMarkup() } // '<'
-                    if let text = try scanText() { return text }
+                    if byte == 0x3C {
+                        return try scanContentMarkup()
+                    } // '<'
+                    if let text = try scanText() {
+                        return text
+                    }
                     continue
                 }
                 guard let lead = reader.peek() else {
                     throw ParseError.unexpectedEndOfInput(reader.mark)
                 }
                 eventStart = reader.mark
-                if lead == "<" { return try scanContentMarkup() }
-                if let text = try scanText() { return text }
+                if lead == "<" {
+                    return try scanContentMarkup()
+                }
+                if let text = try scanText() {
+                    return text
+                }
             }
         }
 
@@ -220,7 +239,9 @@ public extension PureXML.Parsing {
                 // Detect the closing '<' on the byte path so the buffer stays
                 // empty: a buffered '<' would force the next element's dispatch,
                 // matches(), and name scan onto the slow Character path.
-                if reader.peekByte() == 0x3C { break }
+                if reader.peekByte() == 0x3C {
+                    break
+                }
                 guard let character = reader.peek(), character != "<" else { break }
                 length += 1
                 try checkContent(length, mark)
@@ -230,7 +251,9 @@ public extension PureXML.Parsing {
                 guard character.unicodeScalars.allSatisfy(XMLCharacter.isChar) else {
                     throw ParseError.invalidCharacter(reader.mark)
                 }
-                if character == "&" { sawAmpersand = true }
+                if character == "&" {
+                    sawAmpersand = true
+                }
                 raw.append(character)
                 reader.advance()
             }

Sources/Parsing/EventReaderAttributes.swift

@@ -3,13 +3,20 @@
 extension PureXML.Parsing.EventReader {
     mutating func scanAttributes() throws -> [PureXML.Model.Attribute] {
         var attributes: [PureXML.Model.Attribute] = []
-        var seen: Set<String> = []
+        // Reuses the stored scratch set (see its declaration) so an element
+        // does not allocate fresh Set storage just to check its attribute
+        // names; clearing keeps the capacity. `scanAttributes` cannot re-enter
+        // itself (nothing inside an attribute scan starts another start tag),
+        // so the shared storage never sees two interleaved scans.
+        attributeNamesSeen.removeAll(keepingCapacity: true)
         while true {
             reader.skipSpace()
             // Byte dispatch keeps the buffer empty so scanName() reaches its
             // byte fast path; the Character peek is the off-fast-path fallback.
             if let byte = reader.peekByte() {
-                if byte == 0x3E || byte == 0x2F { return attributes } // '>' or '/'
+                if byte == 0x3E || byte == 0x2F {
+                    return attributes
+                } // '>' or '/'
             } else {
                 guard let next = reader.peek(), next != ">", next != "/" else {
                     return attributes
@@ -23,7 +30,7 @@ extension PureXML.Parsing.EventReader {
             }
             reader.skipSpace()
             let value = try scanAttributeValue()
-            guard seen.insert(name.description).inserted else {
+            guard attributeNamesSeen.insert(name.description).inserted else {
                 if recovering {
                     // Keep the element: record the duplicate and drop the later
                     // occurrence rather than failing the whole start tag.
@@ -74,7 +81,9 @@ extension PureXML.Parsing.EventReader {
             // or a control/non-ASCII byte the Character path must inspect),
             // this avoids buffering that quote as a Character only to compare
             // and discard it a moment later.
-            if reader.peekByte() == quoteByte { break }
+            if reader.peekByte() == quoteByte {
+                break
+            }
             guard let character = reader.peek(), character != quoteCharacter else { break }
             length += 1
             try checkContent(length, mark)
@@ -84,7 +93,9 @@ extension PureXML.Parsing.EventReader {
             guard character.unicodeScalars.allSatisfy(PureXML.Parsing.XMLCharacter.isChar) else {
                 throw PureXML.Parsing.ParseError.invalidCharacter(reader.mark)
             }
-            if character == "&" { sawAmpersand = true }
+            if character == "&" {
+                sawAmpersand = true
+            }
             // 3.3.3: a literal whitespace character in an attribute value
             // normalizes to a space; this happens before reference decoding,
             // so whitespace that arrives via a character reference survives.

docs/benchmarks.md

@@ -79,8 +79,9 @@ offsets throughout; libxml2's xmlReadMemory takes an int byte count).
 | 25 | `scanAttributeValue`'s closing-quote check: after the content loop peeks the next character to decide whether it is the closing quote (breaking either on a match or on end of input, the only two cases), `reader.consume(String(quote))` re-derived the same answer through `matches`/`consume`'s general path, which allocates a fresh `String(quote)` and iterates it via `Sequence`/`Character` machinery twice (once to compare, once to count how many times to `advance()`), on every single attribute value in the document. Checking the already-buffered peek directly (`reader.peek() == quote`) and calling `advance()` once is the same observable behavior (same throw on end-of-input, same consumption on a match) for far less work. Verified: full local suite (1852 tests) and the full W3C XSTS suite (14383 groups, ratchet unchanged) pass unchanged. | 0.129 s (fresh session baseline; see note below) | unchanged | unchanged |
 | 26 | `NamespaceContext.enterElement`, called once per element: (a) `checkNameShape`'s `name.localName.contains(":")` used a `Character` literal, resolving to `Sequence.contains(_:)`'s grapheme-by-grapheme walk to find one ASCII byte; `localName.utf8.contains(0x3A)` finds the same colon via the UTF-8 byte view, since 0x3A is never a lead or continuation byte of a multi-byte sequence. (b) `attributes.map { try resolveAttribute($0, at: mark) }` allocated a fresh backing array on every element even though `resolveAttribute` returns its argument completely unchanged whenever the attribute has no prefix or the `xmlns` prefix (the common case: most attributes in most documents are unprefixed); checking `attributes.contains { prefix != nil && prefix != "xmlns" }` first (no allocation) and reusing `attributes` directly when nothing would change gives the identical `resolvedAttributes` value for the common case without the allocation. Verified: full local suite and full W3C XSTS suite pass unchanged. | 0.121 s (~6.00x from 0.129 s at pass 25's baseline) | unchanged | unchanged |
 | 27 | Sample-profiling isolated `scanContentMarkup()` (the inlined dispatcher covering `scanStartTag`/`scanAttributes`/`scanAttributeValue`/`NamespaceContext.enterElement`) at ~46% of total parse time. Three further findings in the same shapes as passes 25/26: (a) `scanAttributeValue`'s opening quote check still ran on the Character path (`reader.peek()` + `reader.advance()`) even though the byte is known ASCII and non-CR/LF; `peekByte()` plus `reader.consume(_:)` reaches the byte fast path directly (`peekByte()` only returns non-nil when the lookahead buffer is already empty, so `consume`'s own `matchesBytes` engages with no Character buffering at all), extended to the closing-quote check too (a byte peek before the Character peek, avoiding buffering the quote as a Character only to compare and discard it). (b) `NamespaceContext.checkExpandedNames` allocated a `Set<String>`'s storage and built a concatenated key per element even when 0 or 1 attribute carries a namespace URI, the overwhelming majority of elements; the check can only ever throw when two URI-bearing attributes collide, so counting first (no allocation) and returning early below 2 skips the allocation entirely in that case (for the built-in `xml` namespace URI specifically, the key string is long enough to need its own heap allocation, so this also cuts a real per-`xml:*`-attribute allocation). (c) `EntityDecoder.decode`/`decodeLenient`'s `raw.contains("&")` guard was the same grapheme-walk-for-one-ASCII-byte shape pass 26(a) already fixed elsewhere; `raw.utf8.contains(0x26)` finds the same `&` via the UTF-8 byte view. Verified: full local suite (1858 tests, 6 new covering the quote-handling edge cases: empty value, single-quoted, a quote-kind nested inside the other, unterminated at end of input, and byte/Character-path agreement for all of them) and full W3C XSTS suite (14383 groups) pass unchanged. | 0.099 s (~4.75x from 0.121 s at pass 26's baseline) | unchanged | unchanged |
+| 28 | `scanAttributes` allocated a fresh `Set<String>`'s native storage per element with attributes, just to check its few names for duplicates; the set is now a stored `EventReader` scratch property cleared with `removeAll(keepingCapacity: true)` per element, so one element's check reuses the previous element's storage. Safe because `scanAttributes` cannot re-enter itself (nothing inside an attribute scan starts another start-tag scan), so the shared storage never sees two interleaved scans; duplicate detection, recovering-mode `continue` behavior, and thrown errors are unchanged (full local suite, 1858 tests, and full W3C XSTS, 14383 groups, both unchanged). Measured with an interleaved same-state A/B (baseline, patched, baseline, patched, each best-of-5) because the machine was under concurrent load that session: both patched runs (0.1073 s, 0.1067 s) beat both baseline runs (0.1091 s, 0.1117 s), ~2-4%. A sibling candidate the same session, reserving each tree frame's children capacity on first append, measured NEUTRAL in the same protocol and was reverted rather than landed, per this log's rule that a pass ships only with a demonstrated win. | ~2-4% (same-state interleaved A/B; absolute ratios that session inflated by concurrent machine load) | unchanged | unchanged |
 
-Passes 25-27 were measured in a fresh session on different hardware from the original baseline and passes 1-24 (this doc does not know what machine recorded which historical entry), so the absolute seconds are not directly comparable across the gap; the ratio column (parse ~6.59x before pass 25, ~6.30x after pass 25, ~6.00x after pass 26, ~4.5-4.75x after pass 27, all at the 20000-item/4.2MB corpus, best of 5) is the honest same-session A/B this log's discipline requires. Reconciling the whole table against one pinned machine is open work, not assumed.
+Passes 25-28 were measured in a fresh session on different hardware from the original baseline and passes 1-24 (this doc does not know what machine recorded which historical entry), so the absolute seconds are not directly comparable across the gap; the ratio column (parse ~6.59x before pass 25, ~6.30x after pass 25, ~6.00x after pass 26, ~4.5-4.75x after pass 27, all at the 20000-item/4.2MB corpus, best of 5; pass 28 recorded as a same-state interleaved delta only, since its session had concurrent machine load) is the honest same-session A/B this log's discipline requires. Reconciling the whole table against one pinned machine is open work, not assumed.
 
 ## Profile findings (first sample, parse path)
 
@@ -102,6 +103,20 @@ patched.
 - Suspects to profile, in order: Character/grapheme-based scanning versus
   byte scanning, per-scalar reader buffering (#135), ARC traffic in tree
   construction, XPath node-set materialization on every axis step.
+- **Weak-parent side tables (measured pass-28 session, ~9% of parse,
+  deliberately NOT changed).** `TreeNode.init(adopting:)`'s `child.parent =
+  self` runs `swift_weakAssign` per child, and the first weak reference to
+  any object makes the Swift runtime allocate a side table on it, so every
+  parent node pays a side-table allocation during tree building (and its
+  release goes through the slow side-table path at teardown, visible in the
+  first profile's `doDecrementSlow`). This is the structural price of the
+  documented safe design: `TreeNode.parent` is public API declared `weak`,
+  and its nil-after-release behavior is documented and load-bearing ("a
+  child held on its own loses its ancestry the moment the rest of the tree
+  is released"). `unowned` would avoid the side table but trades nil for a
+  trap, a public behavior change; `unowned(unsafe)` is a dangling pointer.
+  Recovering this ~9% requires a design decision (an arena/index-based tree,
+  or an API-versioned semantics change), not an optimization pass.
 
 ## Rules