Span-native tokens: drop the prevind/nextind SubString round-trip - #110
Merged
Conversation
Pin every accessor (raw, tag_name, attr_value, pi_target) against a checked index-walking reconstruction of the token's byte span, on a 2-/3-/4-byte UTF-8 torture document and an ASCII document. Also guard root-relative offsets for SubString input, empty-span reconstruction, the has_entities flag, and multibyte characters landing immediately before each ASCII delimiter. Groundwork for #109: proves the span arithmetic equivalent to checked slicing before the tokenizer switches to span-native token emission. Assisted-by: Claude (Anthropic)
Emit sites now construct tokens directly from the integer scan positions already in hand (make_token, root-relative for SubString input via _data_offset), instead of building a throwaway SubString whose checked constructor recomputes the byte length with an unconditional nextind. raw() and the tag_name/attr_value/pi_target accessors rebuild their views straight from the token's span fields through SubString's internal Val(:noshift) constructor, behind a version-guarded helper (_noshift_substring) that falls back to the checked index-walking path if Base ever drops that constructor. Safety: every span edge the scanner produces is an ASCII byte or EOF (NAME_BYTE_TABLE classifies 0x80-0xFF as name bytes), hence a UTF-8 character boundary by construction — documented at the point of reliance. Full validation is preserved under --check-bounds=yes (as in Pkg.test) via a @BoundsCheck block; production builds elide it. Entity detection stays bounded through _span_view. See #109. Assisted-by: Claude (Anthropic)
PERFORMANCE-v0.4.md: Tables 1-4 re-measured on the same protocol (BenchmarkTools defaults, 5 s budget) — Cursor stream 46 -> 31 ms, lex 37.2 -> 25.6 ms, parse -> DOM 99.2 -> 74.7 ms, Node build 98 -> 81 ms, FlatNode build 53 -> 34.9 ms; external witness rows re-measured in the same session (EzXML, LightXML within +/-2%). Two headline claims flip: FlatNode now out-builds libxml2 (~1.3x) and pure-Julia streaming runs ~2x ahead of EzXML's StreamReader. Added the span-native mechanism to the Julia-level constant-factors paragraph; footnotes re-dated. README.md: access-pattern table and LazyNode whole-tree figures updated to the same measurements (Cursor 25.3 ms, LazyNode walk 185 ms, FlatNode 34.9 ms, Node 81.1 ms); prose ratios recomputed. CHANGELOG.md: Unreleased/Changed entry for the span-native tokenizer with the measured deltas and the soundness argument (#109). Assisted-by: Claude (Anthropic)
One clean same-session run of benchmarks/benchmarks.jl (morning window, in-run monitor clean of OS maintenance; two independent runs agree within 2% on every cell). XML.jl Node parse drops 95.5 -> 77.4 ms on the medium corpus and 0.0212 -> 0.0158 ms on the small one; witness columns (EzXML, LightXML, XMLDict) re-quoted from the same run, within the documented cross-session envelope of the published values. Assisted-by: Claude (Anthropic)
The medium and read-file EzXML cells parse a 14 MB document per sample; their C trees are freed only by finalizers, and a benchmark loop outruns the collector, so gigabytes of dead trees piled up within a cell and the suite paged the machine. Those cells now free per sample (untimed finalize teardown, mirroring the existing LightXML pattern; cell timings verified unchanged across four runs), and @add_benchmark runs GC.gc() between cells so finalizer-held residue never crosses a cell boundary. XMLDict is deliberately left without a per-sample teardown: its C residue is small, and a per-sample collection would reset the young generation inside the cell and hide the GC cost its allocation-heavy conversion pays in real use. Assisted-by: Claude (Anthropic)
One clean run of the suite as it now ships (C-heap bounding in place; witness cells within the same-day band of three prior runs — EzXML medium stable at 46.1-46.3 ms across all four). Sub-millisecond rows (Parse/Write/Collect on the 4 KB document) are displayed in microseconds instead of fractional milliseconds. CHANGELOG notes the suite hygiene change. Assisted-by: Claude (Anthropic)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 95.51% 95.55% +0.04%
==========================================
Files 13 13
Lines 2205 2207 +2
==========================================
+ Hits 2106 2109 +3
+ Misses 99 98 -1
🚀 New features to boost your workflow:
|
Assisted-by: Claude (Anthropic)
The SubString convenience constructors are kept for tests but were no longer called by anything once emit sites switched to integer spans — Codecov flagged the field-mirroring line as newly uncovered. Pin their contract (offset/ncodeunits mirror the SubString fields) and add one direct, non-inlined _noshift_substring call, which also exercises the @boundscheck-validated construction path explicitly. Assisted-by: Claude (Anthropic)
mathieu17g
marked this pull request as ready for review
August 3, 2026 13:19
mathieu17g
added a commit
that referenced
this pull request
Aug 4, 2026
Seven entries told the story one PR at a time, with per-step percentages that no release reader can compose. Four now: the GC note, the Node build's scratch stacks (#107), one span-native entry whose figures are the v0.4.4-to-now state (lex 37.4 -> 23.4 ms, FlatNode extract 6.6 -> 3.1 ms, the three cross-library headline flips; #109, #111, #113), and one measurement-upkeep entry (protocol + suite C-heap bounds + microsecond rows; #107, #110). Mechanism detail lives in the referenced PRs; released sections untouched. Assisted-by: Claude (Anthropic)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #109.
Tokens now carry byte spans end to end. Emit sites build
Tokens directly from the integer scan positions already in hand, andraw()/tag_name/attr_value/pi_targetrebuild their views straight from the stored(offset, ncodeunits)fields — noprevind/nextindwalks, no checked-constructor length recomputation. The soundness argument is the one from #109: a DFA lexer over UTF-8 stops only on ASCII structural bytes (NAME_BYTE_TABLEclassifies 0x80–0xFF as name bytes), so every span edge is a character boundary by construction, even on malformed input; the invariant is documented at the point of reliance, and a@boundscheckblock keeps full validation active under--check-bounds=yes, soPkg.testverifies every span the suite builds. Public behavior is unchanged:rawand the accessors still returnSubString{String}, andTokenstays isbits.BenchmarkTools
@benchmarkmedians at default parameters, 14 MB XMark corpus, same-session A/B (A =f6e1017); external witness cells (EzXML, LightXML, walks/extracts on already-built trees) within ±2 %:Node)FlatNodeCursorfull stream (tag+value reads)LazyNodefull walkNodeString/SubStringAllocations, retained sizes and node counts are identical on every cell — the removed round-trips were index arithmetic on stack-allocated views, not heap traffic, so the gain is pure CPU. Two cross-library claims flip and the docs now say so:
FlatNodeout-builds libxml2 (34.9 vs 45.8 ms, ~1.3×), and the pure-JuliaCursorstreams ~2× ahead of EzXML'sStreamReader(30.6 vs 66.4 ms).Tests went in first (
8c72293): the tokenizer testset grows to 469 assertions — every accessor pinned against a checked index-walking reconstruction of its span (still meaningful after the switch: it proves the fast path equal to checked semantics), multibyte coverage with 2-/3-/4-byte characters across names, attribute names/values, text, CDATA and PI content, root-relative spans forSubStringinput, empty spans, and thehas_entitiesflag.Pkg.test: 3981/3981, with--check-bounds=yesexercising the validation path on every token the suite builds; W3C not-wf verdicts identical on all 1257 documents; a per-token equivalence probe over the corpus reports 1,850,962 tokens, zero mismatches.Docs re-measured in this PR on the unchanged protocol: PERFORMANCE-v0.4.md Tables 1–4 (the two flipped claims re-worded), the README access-pattern and cross-library tables (sub-millisecond rows now quoted in µs), and CHANGELOG entries under Unreleased/Changed.
One suite fix rides along:
benchmarks/benchmarks.jlnow bounds its own C heap — the medium/read-file EzXML cells free their C tree per sample (untimedfinalizeteardown, mirroring the existing LightXML pattern; cell timings verified unchanged across four same-day runs, EzXML medium stable at 46.1–46.3 ms), and@add_benchmarkcollects between cells so finalizer-held residue never crosses a cell boundary. A full suite run used to page the machine several GB deep; XMLDict is deliberately left without a per-sample teardown so the GC cost its allocation-heavy conversion pays in real use stays in its number.