diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f77528..e9ffe70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- A fifth serialize-path optimization pass (#139, `docs/benchmarks.md` pass + 31): `textNeedsEscaping`/`attributeNeedsEscaping` and the `comment`/ + `processingInstruction` existence fast-paths now scan a raw contiguous + buffer (`String.utf8.withContiguousStorageIfAvailable`) instead of going + through `UTF8View`'s default `Collection` iteration, falling back to the + original scan when no contiguous storage is available (every string this + library builds is natively contiguous, so the fallback is believed dead in + practice). Found by profiling a new, deeply-nested/attribute-dense corpus + the standard flat catalog corpus has no density to indict; a standalone + micro-benchmark confirmed the raw-buffer scan ~2-2.75x faster across + several call shapes, order-swap-checked against warm-up bias. Two + abstraction attempts were tried and rejected: a shared generic helper + taking a predicate closure recovered most but not all of the win, and + calling a separate small function from inside the buffer loop (to dodge a + cyclomatic-complexity lint violation) erased nearly the entire win even + though it is not a closure value; the violation is instead resolved by + splitting each function into a contiguous/collection pair, so the call + boundary sits outside the loop rather than inside it. Output is + byte-identical by construction (same predicate, same trigger bytes, only + the iteration mechanism changed); full local suite (1925 tests) and full + lint unchanged. Measured with interleaved same-state A/B under real + concurrent machine load (load average ~2.4): clean with zero overlap + across 5 pairs at the new attribute-heavy nested corpus (~9%); noisier at + the 100k-item standard corpus (2 of 5 pairs individually reversed by a + small margin) but the means still separate clearly (~4%), the same + trust-the-aggregate-under-diagnosed-noise call this log made for pass 28. + - Two serialize-path optimization passes (#139, `docs/benchmarks.md` passes 29-30), found by sample-profiling the serialize loop: the attribute writer precomputes the quote character as a `String` once per element instead of diff --git a/Sources/Emitting/Escaping.swift b/Sources/Emitting/Escaping.swift index 90b2ddb..c3dc0e9 100644 --- a/Sources/Emitting/Escaping.swift +++ b/Sources/Emitting/Escaping.swift @@ -59,8 +59,42 @@ extension PureXML.Emitting { /// Whether ``text`` would change `value`: it escapes `&`, `<`, `>` always, /// a carriage return when asked, and (in ASCII-only mode) any non-ASCII /// byte. Scanned at the byte level, since every trigger is ASCII or, for - /// ASCII-only mode, a non-ASCII lead byte. + /// ASCII-only mode, a non-ASCII lead byte. Run on every text node in a + /// document, so it scans a raw contiguous buffer when the string + /// provides one: `UTF8View`'s default `Collection` iteration measured + /// markedly slower than a raw-buffer scan for this shape (~2x on a + /// dedicated micro-benchmark), and every string this library builds + /// (parsed from its own byte-backed reader) is natively contiguous, so + /// the fallback below only exists for a foreign/bridged string. private static func textNeedsEscaping(_ value: String, asciiOnly: Bool, escapeCarriageReturn: Bool) -> Bool { + textNeedsEscapingContiguous(value, asciiOnly: asciiOnly, escapeCarriageReturn: escapeCarriageReturn) + ?? textNeedsEscapingCollection(value, asciiOnly: asciiOnly, escapeCarriageReturn: escapeCarriageReturn) + } + + /// The raw contiguous-buffer scan, split into its own function (rather + /// than inlined alongside the ``textNeedsEscapingCollection`` fallback in + /// one body) purely to keep each function's cyclomatic complexity low; + /// the switch itself must stay duplicated, not shared through a per-byte + /// helper call, since that measured erasing almost the entire win. + private static func textNeedsEscapingContiguous(_ value: String, asciiOnly: Bool, escapeCarriageReturn: Bool) -> Bool? { + value.utf8.withContiguousStorageIfAvailable { buffer -> Bool in + for byte in buffer { + switch byte { + case 0x26, 0x3C, 0x3E: return true + case 0x0D where escapeCarriageReturn: return true + case 0x80... where asciiOnly: return true + default: continue + } + } + return false + } + } + + /// `Collection`-based fallback for a string with no contiguous storage + /// (a foreign/bridged string); every string this library builds is + /// natively contiguous, so this path is believed dead in practice but + /// kept for correctness. + private static func textNeedsEscapingCollection(_ value: String, asciiOnly: Bool, escapeCarriageReturn: Bool) -> Bool { for byte in value.utf8 { switch byte { case 0x26, 0x3C, 0x3E: return true @@ -145,8 +179,33 @@ extension PureXML.Emitting { /// Whether ``attribute`` would change `value`: it escapes the quote, the /// markup characters, the whitespace characters, and (in ASCII-only mode) - /// any non-ASCII byte. Every trigger is ASCII bar the non-ASCII lead. + /// any non-ASCII byte. Every trigger is ASCII bar the non-ASCII lead. Run + /// on every attribute value in a document, so it prefers the same raw + /// contiguous-buffer scan as ``textNeedsEscaping``, for the same measured + /// reason. private static func attributeNeedsEscaping(_ value: String, quoteByte: UInt8, asciiOnly: Bool) -> Bool { + attributeNeedsEscapingContiguous(value, quoteByte: quoteByte, asciiOnly: asciiOnly) + ?? attributeNeedsEscapingCollection(value, quoteByte: quoteByte, asciiOnly: asciiOnly) + } + + /// The raw contiguous-buffer scan; see ``textNeedsEscapingContiguous`` + /// for why this is a separate function rather than sharing the switch + /// with the fallback through a per-byte helper call. + private static func attributeNeedsEscapingContiguous(_ value: String, quoteByte: UInt8, asciiOnly: Bool) -> Bool? { + value.utf8.withContiguousStorageIfAvailable { buffer -> Bool in + for byte in buffer { + switch byte { + case quoteByte, 0x26, 0x3C, 0x3E, 0x09, 0x0A, 0x0D: return true + case 0x80... where asciiOnly: return true + default: continue + } + } + return false + } + } + + /// `Collection`-based fallback; see ``textNeedsEscapingCollection``. + private static func attributeNeedsEscapingCollection(_ value: String, quoteByte: UInt8, asciiOnly: Bool) -> Bool { for byte in value.utf8 { switch byte { case quoteByte, 0x26, 0x3C, 0x3E, 0x09, 0x0A, 0x0D: return true @@ -163,7 +222,15 @@ extension PureXML.Emitting { static func comment(_ value: String) -> String { // Fast path: a comment with no hyphen has no `--` and no trailing `-`, // so it is returned unchanged rather than rebuilt character by character. - if !value.utf8.contains(0x2D) { + // A raw contiguous-buffer scan for the same measured reason as + // textNeedsEscaping/attributeNeedsEscaping above. + let hasHyphen = value.utf8.withContiguousStorageIfAvailable { buffer -> Bool in + for byte in buffer where byte == 0x2D { + return true + } + return false + } ?? value.utf8.contains(0x2D) + if !hasHyphen { return value } var result = "" @@ -201,8 +268,16 @@ extension PureXML.Emitting { /// (the XSLT 1.0 7.3 recovery, so `a?>b` becomes `a? >b`). static func processingInstruction(_ data: String) -> String { // Fast path: data with no '?' cannot contain `?>`, so it is returned - // unchanged rather than rebuilt character by character. - if !data.utf8.contains(0x3F) { + // unchanged rather than rebuilt character by character. A raw + // contiguous-buffer scan for the same measured reason as + // textNeedsEscaping/attributeNeedsEscaping above. + let hasQuestion = data.utf8.withContiguousStorageIfAvailable { buffer -> Bool in + for byte in buffer where byte == 0x3F { + return true + } + return false + } ?? data.utf8.contains(0x3F) + if !hasQuestion { return data } var result = "" diff --git a/docs/benchmarks.md b/docs/benchmarks.md index c399630..ed69cc8 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -78,6 +78,7 @@ offsets throughout; libxml2's xmlReadMemory takes an int byte count). | 24 | find the characters to escape by scanning bytes, not graphemes: the bulk-escape loop still iterated `Character`s and compared each to `&`/`<`/`>` (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) | | 29 | precompute the attribute quote as a `String` once per element instead of interpolating the `Character` into every attribute's `"...\(quote)...\(quote)"`: a `Character` interpolation goes through `Character.write(to:)` and the string's grapheme-breadcrumb machinery, work repeated twice per attribute for a value that is the same one or two bytes every time in a given element. `quoteString` is built once outside the attribute loop and interpolated as a `String` (a plain code-unit append, no grapheme handling). Output is unchanged (a `Character` and its single-scalar `String` render identically); full local suite (1921 tests, including the gold/C14N/Xalan serializer suites) unchanged. Same-state interleaved A/B: clean at the 100k-item corpus (two pairs, patched 0.1696-0.1698 s vs baseline 0.1717-0.1728 s, both patched runs beating both baseline runs, ~1.5%); noisier but still favorable at the 20k corpus (4 pairs, one round overlapped). Trust the larger, cleaner corpus per this log's rule. | unchanged | unchanged | ~1.5% at 100k corpus | | 30 | `Escaping.comment`/`processingInstruction`'s escaping fallback (taken whenever a comment has a hyphen or PI data has a `?`, which pass 8 could only skip past, not speed up): the fallback still rebuilt the whole value one `Character` at a time even though the space it must insert is rare (only at a `--`/`?>` collision), so a long comment with one hyphen anywhere still paid a full character-by-character copy. Rewritten in the same bulk-copy shape as passes 23/24: scan the byte view for the candidate trigger byte (`-` for comment; `?` or `>` for PI, since either can matter to the adjacency check), copy the verbatim run before it in one append, and resolve the actual decision through `Character` equality at that position exactly as the original loop did, so a hyphen/`?`/`>` that is not its own grapheme (a combining mark attached to it, changing the composed `Character`) is copied verbatim exactly as before, never misread from its lone UTF-8 byte. Verified by a dedicated differential suite (`Tests/CommentAndPIEscapingDifferentialTests.swift`) that reimplements the original character-loop as an independent oracle and checks equality over hand-picked adjacency/trailing/combining-mark cases plus two 20000-case seeded fuzzes (one per function); full local suite (1925 tests) passes. No comment/PI content exists in the standard benchmark corpus, so this was measured with a dedicated micro-benchmark (a document of 300000 alternating realistic comment/PI nodes - hyphenated prose, copyright lines, `?`-bearing PI data - driven through the public `PureXML.serialize` entry point, since `Escaping` is internal): same-state interleaved A/B, clean separation across 4 runs, old 0.451-0.455 s vs new 0.223-0.224 s, ~2x. | unchanged | unchanged | ~2x on comment/PI-bearing content (no effect on the standard corpus, which has none) | +| 31 | `textNeedsEscaping`/`attributeNeedsEscaping` and the `comment`/`processingInstruction` existence fast-paths (`value.utf8.contains(byte)`) all scan `String.utf8` through `UTF8View`'s default `Collection` iteration, run unconditionally on every text node, attribute value, comment, and PI in a document. Sample-profiling a new, differently-shaped corpus (deep 9-level nesting, breadth 4, 5 attributes per element, ~350k elements, since the standard flat catalog corpus has almost no attribute/depth density to indict this) put `textNeedsEscaping` and `attributeNeedsEscaping` as the two largest PureXML-specific frames (897 and 613 of ~7800 top-of-stack samples). A standalone micro-benchmark (outside PureXML, comparing `for byte in value.utf8 { switch ... }` against `value.utf8.withContiguousStorageIfAvailable { buffer in ... }` over a raw `UnsafeBufferPointer`) measured the buffer scan ~2-2.75x faster across the loop, `.contains`, and `.firstIndex(where:)` shapes alike, order-swap-checked to rule out warm-up bias. Two abstraction attempts were measured and rejected before landing the shipped shape: a shared generic helper taking a `predicate` closure recovered most but not all of the win (closure-calling-closure indirection cost some back, ~0.26 s vs ~0.25 s fully inlined); calling a separate small `static func` from inside the buffer loop (to share the switch with the `Collection` fallback and dodge a cyclomatic-complexity lint violation) erased nearly the entire win (~0.38 s, barely better than the ~0.40 s original) even though it is not a closure value, so the switch itself has to stay duplicated inline in both paths. The complexity violation was instead resolved by splitting each function into a `*Contiguous`/`*Collection` pair (the call boundary moves outside the loop, once per invocation, not once per byte), which measurably preserved the win. Scope for this pass is the four existence/needs-escaping checks only (a plain `Bool`, no `String.Index` to preserve); the bulk-copy `firstIndex(where:)` rebuild loops (passes 23/24/30) are left as-is, a candidate for a future pass. Every site falls back to the `Collection`-based scan when no contiguous storage is available (every string this library builds is natively contiguous, so the fallback is believed dead in practice but kept for correctness). Output is provably byte-identical (same predicate, same trigger bytes, only the iteration mechanism changed); full local suite (1925 tests), full lint (`swiftformat --lint`, `swiftlint --strict`, no new violations), unchanged, and both A/B corpora produced byte-identical serialized output (20995400 and 64225220 bytes respectively, matched exactly pre/post). Final measurement (same-state interleaved A/B, the actual shipped split-function shape, run under real concurrent machine load, load average ~2.4): the attribute-heavy nested corpus is clean across 5 pairs with zero overlap (patched 0.3409-0.3454 s vs baseline 0.3769-0.3791 s, ~9%); the 100k-item standard corpus is noisier under this session's load (2 of 5 pairs individually reversed by a small margin) but the means still separate clearly (patched mean 0.1670 s vs baseline mean 0.1744 s over 5 pairs each, ~4%), the same "trust the aggregate under diagnosed noise" call this log made for pass 28. | unchanged | unchanged | ~4% at the standard corpus (noisy this session), ~9% clean at the attribute-heavy nested corpus | | 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 |