From fe0400c3dfa38f7b3aec4fe12189fca0cc326f08 Mon Sep 17 00:00:00 2001 From: Mihaela Mihaljevic Date: Sun, 12 Jul 2026 12:27:00 +0200 Subject: [PATCH] perf(parsing): byte-path quote handling, lazy expanded-name check, byte-scan entity guard Third profile-guided pass (#139, docs/benchmarks.md pass 27), targeting scanContentMarkup(), which inlines the start-tag/attribute/namespace scanners and carries ~46% of parse time. scanAttributeValue's opening and closing quote checks ran on the Character path (peek + advance, buffering the quote as a Character only to compare and discard it) on every attribute value in the document; both now check the raw byte first, reaching the byte fast path with no Character buffering, with the Character path kept as the off-fast-path fallback for streaming sources. The quote bytes are ASCII and non-CR/LF, so the raw byte and the normalized stream agree, the same reasoning peekByte's other callers already rely on. NamespaceContext.checkExpandedNames allocated a Set's native storage and built a concatenated URI+local key string per element even when at most one attribute carries a namespace URI, where no duplicate is possible (the default namespace never applies to attributes, and xmlns declarations carry no URI). Counting URI-bearing attributes first, with no allocation, and returning early below two skips all of it on the overwhelming majority of elements; for xml:* attributes specifically the key exceeded the small-string limit, so this also removes a real per-element heap allocation. EntityDecoder.decode/decodeLenient guarded with raw.contains("&"), a grapheme-by-grapheme walk to find one ASCII byte, once per ampersand-bearing text node or attribute value; raw.utf8.contains(0x26) finds the same byte without decoding graphemes, the same shape as checkNameShape's colon check. The quote-handling extraction into two helpers also resolves the cyclomatic complexity the inline version tripped. Six new regression tests pin the quote edge cases (empty value, single-quoted, a quote kind nested inside the other, unterminated at end of input) plus byte/Character-path agreement for each in the parser path differential. Verified: full local suite (1858 tests) and full W3C XSTS suite (14383 groups) pass unchanged. Parse ~6.00x -> ~4.75x versus libxml2 at the 20000-item corpus, best of 5. --- CHANGELOG.md | 22 +++++++ Sources/Parsing/EntityDecoder.swift | 13 +++- Sources/Parsing/EventReaderAttributes.swift | 71 +++++++++++++++------ Sources/Parsing/NamespaceContext.swift | 16 +++++ Tests/ByteFastPathTests.swift | 40 ++++++++++++ Tests/ParserPathDifferentialTests.swift | 7 ++ docs/benchmarks.md | 3 +- 7 files changed, 150 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4718faf..8fc00cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- 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 + `scanStartTag`/`scanAttributes`/`scanAttributeValue`/`NamespaceContext.enterElement`): + `scanAttributeValue`'s opening and closing quote checks now use the byte + fast path (`peekByte()` + `reader.consume(_:)`/a byte peek before the + Character peek) instead of the Character path, for every attribute value in + the document; `NamespaceContext.checkExpandedNames` skips its `Set` + allocation and key-string building entirely when fewer than two attributes + carry a namespace URI (the check can only ever throw on a collision between + two such attributes, so 0 or 1 can never trigger it, and this is the + overwhelming majority of elements); and `EntityDecoder`'s `raw.contains("&")` + guard is now a UTF-8 byte scan instead of a `Character`-based one, the same + shape as `NamespaceContext.checkNameShape`'s colon check in pass 26. All + three are same-observable-behavior changes, verified against the full local + suite (1858 tests, 6 new covering the quote-handling edge cases directly: + empty value, single-quoted, a quote-kind nested inside the other, and + unterminated-at-end-of-input, plus byte/Character-path agreement for each) + and the full W3C XSTS suite (14383 groups), both unchanged. Measured parse + ~6.00x -> ~4.5-4.75x versus libxml2 at the 20000-item/4.2MB corpus, best of + 5; see `docs/benchmarks.md` for the full before/after and reasoning. + - Two parse-path optimization passes (#139, `docs/benchmarks.md` passes 25-26), found by sample-profiling a 42MB generated corpus after re-measuring the parse/libxml2 ratio fresh rather than trusting the doc's last-recorded diff --git a/Sources/Parsing/EntityDecoder.swift b/Sources/Parsing/EntityDecoder.swift index 2b7cbbe..b8b5638 100644 --- a/Sources/Parsing/EntityDecoder.swift +++ b/Sources/Parsing/EntityDecoder.swift @@ -18,7 +18,14 @@ extension PureXML.Parsing { budget: inout Int, at mark: Mark, ) throws -> String { - guard raw.contains("&") else { return raw } + // A `Character` literal here resolves to `Sequence.contains(_:)`, + // walking extended grapheme clusters to find one ASCII byte; called + // once per ampersand-bearing text node or attribute value in the + // whole document. `utf8.contains(0x26)` finds the same `&` via the + // UTF-8 byte view, since 0x26 is never a lead or continuation byte + // of a multi-byte sequence (matches the reasoning already applied + // to `NamespaceContext.checkNameShape`'s colon check). + guard raw.utf8.contains(0x26) else { return raw } var expander = EntityExpander(entities: entities, mark: mark, budget: budget) try expander.expand(raw, visiting: [], counts: false) budget = expander.budget @@ -37,7 +44,9 @@ extension PureXML.Parsing { at mark: Mark, undeclared: inout [String], ) throws -> String { - guard raw.contains("&") else { return raw } + // See `decode(_:entities:budget:at:)`'s matching guard: byte-level + // scan, not a grapheme walk, for the same reason. + guard raw.utf8.contains(0x26) else { return raw } var expander = EntityExpander(entities: entities, mark: mark, budget: budget, undeclared: []) try expander.expand(raw, visiting: [], counts: false) budget = expander.budget diff --git a/Sources/Parsing/EventReaderAttributes.swift b/Sources/Parsing/EventReaderAttributes.swift index a254ac0..ef1e4dc 100644 --- a/Sources/Parsing/EventReaderAttributes.swift +++ b/Sources/Parsing/EventReaderAttributes.swift @@ -51,11 +51,7 @@ extension PureXML.Parsing.EventReader { mutating func scanAttributeValue() throws -> String { let mark = reader.mark - guard let quote = reader.peek(), quote == "\"" || quote == "'" else { - throw PureXML.Parsing.ParseError.unquotedAttributeValue(reader.mark) - } - reader.advance() - let quoteByte: UInt8 = quote == "\"" ? 0x22 : 0x27 + let (quoteByte, quoteCharacter) = try consumeAttributeValueOpenQuote() var raw = "" var length = 0 // Whether any '&' was seen: without one the value has no references, so @@ -72,7 +68,14 @@ extension PureXML.Parsing.EventReader { raw += run continue } - guard let character = reader.peek(), character != quote else { break } + // Byte-level check before the Character peek: whenever + // `attributeRunBytes` stopped because the next byte is the closing + // quote (its only non-empty-input stopping reason besides '<', CR, + // 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 } + guard let character = reader.peek(), character != quoteCharacter else { break } length += 1 try checkContent(length, mark) if character == "<" { @@ -88,21 +91,51 @@ extension PureXML.Parsing.EventReader { raw.append(character.isXMLWhitespace ? " " : character) reader.advance() } - // The loop above only exits (breaks) when `reader.peek()` is nil (end of - // input) or equals `quote`: there is no third case, so the closing quote - // is already known and buffered from that last peek. `reader.consume(String(quote))` - // would re-derive the same answer through `matches`/`consume`'s general - // path (which allocates a fresh `String(quote)` and iterates it twice, - // once to compare and once to count how many times to advance), on - // every single attribute value in the document; checking the - // already-buffered peek directly and advancing once is the same - // observable behavior for less work. - guard reader.peek() == quote else { - throw PureXML.Parsing.ParseError.unexpectedEndOfInput(reader.mark) - } - reader.advance() + // The loop above only exits (breaks) when the next position is nil (end + // of input) or the closing quote: there is no third case. + try consumeAttributeValueCloseQuote(byte: quoteByte, character: quoteCharacter) // No '&' means no references to decode: the normalized run is the value. guard sawAmpersand else { return raw } return try decodeReferences(raw, at: mark) } + + /// Detects and consumes the attribute value's opening quote, returning + /// both its byte and `Character` forms: `quoteByte` drives + /// `attributeRunBytes`'s bulk scan and the closing-quote byte check; + /// `quoteCharacter` is the fallback for wherever the byte fast path does + /// not apply (a streaming source with no owned byte storage, or a + /// lookahead buffer some earlier scan already populated). Byte fast path: + /// `peekByte()` only returns non-nil when the lookahead buffer is empty, + /// so `reader.consume(_:)` below reaches its own byte fast path + /// (`matchesBytes`) directly, with no Character buffering at all. + private mutating func consumeAttributeValueOpenQuote() throws -> (byte: UInt8, character: Character) { + if let byte = reader.peekByte(), byte == 0x22 || byte == 0x27 { + reader.consume(byte == 0x22 ? "\"" : "'") + return (byte, byte == 0x22 ? "\"" : "'") + } + guard let quote = reader.peek(), quote == "\"" || quote == "'" else { + throw PureXML.Parsing.ParseError.unquotedAttributeValue(reader.mark) + } + reader.advance() + return (quote == "\"" ? 0x22 : 0x27, quote) + } + + /// Consumes the attribute value's closing quote. Called only once the + /// value-scanning loop has already confirmed (via a peek that consumed + /// nothing) the next position is either the quote or end of input, so + /// whichever check fired that break already buffered nothing new; + /// re-checking the byte first, then falling back to the already-buffered + /// Character peek, reaches the same verdict `reader.consume(String(quote))` + /// would, without that function's fresh `String` allocation and double + /// `Character` iteration on every single attribute value in the document. + private mutating func consumeAttributeValueCloseQuote(byte quoteByte: UInt8, character quoteCharacter: Character) throws { + if reader.peekByte() == quoteByte { + reader.advance() + return + } + guard reader.peek() == quoteCharacter else { + throw PureXML.Parsing.ParseError.unexpectedEndOfInput(reader.mark) + } + reader.advance() + } } diff --git a/Sources/Parsing/NamespaceContext.swift b/Sources/Parsing/NamespaceContext.swift index 288ba33..b8f46c0 100644 --- a/Sources/Parsing/NamespaceContext.swift +++ b/Sources/Parsing/NamespaceContext.swift @@ -161,6 +161,22 @@ extension PureXML.Parsing { /// Attributes must be distinct by expanded name: two prefixes bound to /// the same URI cannot carry the same local name on one element. private func checkExpandedNames(_ attributes: [PureXML.Model.Attribute], at mark: Mark) throws { + // The only observable effect below is throwing `duplicateAttribute`, + // which needs two attributes with an equal (URI, localName) pair; an + // unprefixed attribute never carries a URI (the default namespace + // does not apply to attributes) and neither does an `xmlns`/ + // `xmlns:*` declaration, so with fewer than two URI-bearing + // attributes no collision is possible. Counting first, with no + // allocation, skips the `Set` storage allocation and the + // per-attribute key string (which, for the built-in `xml` + // namespace URI, is long enough to need its own heap allocation) + // on every element that has at most one namespaced attribute, the + // overwhelming majority in most real documents. + var namespacedCount = 0 + for attribute in attributes where attribute.name.namespaceURI != nil { + namespacedCount += 1 + } + guard namespacedCount > 1 else { return } var seen: Set = [] for attribute in attributes where attribute.name.namespaceURI != nil { let key = (attribute.name.namespaceURI ?? "") + "\u{0}" + attribute.name.localName diff --git a/Tests/ByteFastPathTests.swift b/Tests/ByteFastPathTests.swift index ee94882..cf09d77 100644 --- a/Tests/ByteFastPathTests.swift +++ b/Tests/ByteFastPathTests.swift @@ -99,6 +99,46 @@ struct ByteFastPathTests { } } + @Test("An empty attribute value parses on the byte fast path") + func test_emptyAttributeValue() throws { + // attributeRunBytes returns nil immediately (empty run), so the + // closing-quote check must fire on the very first byte peek. + let node = try PureXML.parse("") + #expect(node.firstElement?.attributes == [.init("v", "")]) + } + + @Test("A single-quoted attribute value uses the byte fast path") + func test_singleQuotedAttributeValue() throws { + let node = try PureXML.parse("") + #expect(node.firstElement?.attributes == [.init("v", "x")]) + } + + @Test("A double quote is legal content inside a single-quoted value") + func test_doubleQuoteInsideSingleQuotedValue() throws { + let node = try PureXML.parse("") + #expect(node.firstElement?.attributes == [.init("v", "it\"s")]) + } + + @Test("A single quote is legal content inside a double-quoted value") + func test_singleQuoteInsideDoubleQuotedValue() throws { + let node = try PureXML.parse("") + #expect(node.firstElement?.attributes == [.init("v", "it's")]) + } + + @Test("An attribute value unterminated at end of input is rejected") + func test_unterminatedAttributeValueAtEndOfInput() { + #expect(throws: PureXML.Parsing.ParseError.self) { + try PureXML.parse("a > b

") diff --git a/Tests/ParserPathDifferentialTests.swift b/Tests/ParserPathDifferentialTests.swift index 7efc062..4e7ea22 100644 --- a/Tests/ParserPathDifferentialTests.swift +++ b/Tests/ParserPathDifferentialTests.swift @@ -70,6 +70,13 @@ struct ParserPathDifferentialTests { expectAgreement("char-ref LF survives", "") expectAgreement("literal tab plus ref tab", "") expectAgreement("multi-line attr then attr", "") + expectAgreement("empty value", "") + expectAgreement("single-quoted value", "") + expectAgreement("double quote inside single-quoted value", "") + expectAgreement("single quote inside double-quoted value", "") + expectAgreement("empty then non-empty value", "") + expectAgreement("unterminated value at end of input", "` (a grapheme compare per character); since every trigger is an ASCII marker, or in ASCII-only mode a non-ASCII lead byte, `value.utf8.firstIndex(where:)` locates the next one without decoding the run into graphemes (the marker byte lands on a grapheme boundary, so the string slice and subscript stay valid). Byte-identical output (the oracle differential extended with combining-mark, ZWJ, and flag clusters + the gold/C14N suites + adversarial grapheme-alignment review; same-state A/B, ~23%) | unchanged | unchanged | 0.022 s (~2.2x) | | 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`'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 | -Passes 25-26 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, 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-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. ## Profile findings (first sample, parse path)