-
Notifications
You must be signed in to change notification settings - Fork 5k
XML parser: SIMD structural index + tape rows, one-call rows→JS #37146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5c4abf6
bbfdc48
42f4898
a077a54
cf1c4f4
228880f
072d131
4260651
9493755
704d974
19dc05c
224949f
20dc228
2b2858a
bbb13a8
2eff1ba
38b13f4
8a6e7b1
d7b33b8
f40f8db
8e0b47e
30762af
b2382c7
3767a01
5c36d2f
d50527c
ee1d723
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,34 @@ It is run against the [W3C XML Conformance Test Suite](https://www.w3.org/XML/Te | |||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Performance | ||||||
|
|
||||||
| The parser works in two stages, like Bun's JSON parser: a SIMD pass (runtime-dispatched AVX2/AVX-512/NEON/SVE kernels) records the position of every byte that can change the parse — `<`, `>`, `&`, line ends, quotes and `=` inside tags — and the parser then hops from one of those positions to the next, so character data, attribute values, comments and CDATA sections are never scanned a byte at a time. The result is written as flat rows and turned into JavaScript objects in a single pass that reuses JavaScriptCore's atom-string cache for element and attribute names, the same way `JSON.parse` does. A JS string is parsed in place in whatever representation the engine holds it in (Latin-1 or UTF-16) and the strings in the result share that representation, so nothing is transcoded on the way in or out; `Buffer` and `Blob` input is parsed as UTF-8. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win Qualify the byte-scan claim for entity processing. Line 29 says that character data and attribute values are never scanned a byte at a time. DTD and entity replacement text still use byte-level scanning. Entity replacement can occur in character data and attribute values. Limit this statement to structural scanning and document the exception. Proposed wording-... so character data, attribute values, comments and CDATA sections are never scanned a byte at a time.
+... so structural scanning of character data, attribute values, comments and CDATA sections avoids byte-at-a-time scans; DTD and entity replacement text still use byte-level scanning.📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| `bench/xml/xml.mjs` in the Bun repository compares `Bun.XML.parse` with popular npm parsers on the same documents (lower is better; Linux x64, one core): | ||||||
|
|
||||||
| | Document | `Bun.XML.parse` | txml | fast-xml-parser | @xmldom/xmldom | xml2js | | ||||||
| | ----------------------------------- | --------------: | -----: | --------------: | -------------: | -----: | | ||||||
| | S3 `ListObjectsV2` response, 231 KB | **1.1 ms** | 4.0 ms | 23 ms | 31 ms | 19 ms | | ||||||
| | Atom feed, 193 KB | **1.1 ms** | 3.7 ms | 19 ms | 23 ms | 16 ms | | ||||||
| | libphonenumber metadata, 960 KB | **5.3 ms** | 9.6 ms | 56 ms | 53 ms | — | | ||||||
| | Chromium `enums.xml`, 1.4 MB | **16 ms** | 41 ms | 150 ms | 103 ms | — | | ||||||
| | freedesktop MIME database, 2.2 MB | **27 ms** | 56 ms | 299 ms | 280 ms | — | | ||||||
|
|
||||||
| Roughly half of `Bun.XML.parse`'s time on these documents is creating the JavaScript objects rather than parsing. Measured at the native level (`scripts/bench-json-rust.sh --xml`, parse to the in-memory tree, MiB/s, higher is better), against widely used C, C++ and Rust parsers: | ||||||
|
|
||||||
| | Document | Bun | pugixml | quick-xml | expat | roxmltree | libxml2 | | ||||||
| | --------------------------------- | ----: | ------: | --------: | ----: | --------: | ------: | | ||||||
| | SVG drawing (path data), 1.2 MB | 3,600 | 2,200 | 960 | 170 | 290 | 630 | | ||||||
| | Vulkan `vk.xml`, 3.2 MB | 340 | 630 | 240 | 130 | 110 | 41 | | ||||||
| | Chromium `enums.xml`, 1.4 MB | 320 | 700 | 275 | 114 | 106 | 35 | | ||||||
| | libphonenumber metadata, 960 KB | 440 | 840 | 580 | 170 | 175 | 88 | | ||||||
| | freedesktop MIME database, 2.4 MB | 210 | 585 | 240 | 120 | 91 | 30 | | ||||||
|
|
||||||
| Unlike the fastest C++ parsers, Bun's parser checks everything XML requires of a well-formed document (valid UTF-8, legal characters, unique attributes, entity expansion limits) and expands entities declared in the DTD; attribute- and text-heavy documents are where the SIMD stage pays off most. | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||
|
|
||||||
| --- | ||||||
|
|
||||||
| ## Runtime API | ||||||
|
|
||||||
| ### `Bun.XML.parse()` | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| #!/usr/bin/env bash | ||
| # Build + run the JSON parser criterion bench (src/parsers/benches/json_parse.rs): compiles the | ||
| # native pieces the parser reaches into one archive and points RUSTFLAGS at it. Needs `bun bd` once. | ||
| # Build + run the JSON parser criterion bench (src/parsers/benches/json_parse.rs), or with `--xml` | ||
| # the XML one (benches/xml_parse.rs): compiles the native pieces the parsers reach into one archive | ||
| # and points RUSTFLAGS at it. Needs `bun bd` once. `--test` runs the crate's unit tests instead. | ||
| set -euo pipefail | ||
| cd "$(dirname "$0")/.." | ||
|
|
||
|
|
@@ -41,9 +42,30 @@ build "$SUP/simdutf_shim.o" $CXX -O3 -fPIC -std=c++20 -I"$SUP" -c src/parsers/be | |
| for f in abort targets per_target print timer nanobenchmark aligned_allocator; do | ||
| build "$SUP/hwy_$f.o" $CXX -O3 -fPIC -std=c++17 -Ivendor/highway -c "vendor/highway/hwy/$f.cc" | ||
| done | ||
| if [ -f src/jsc/bindings/highway_json.cpp ]; then | ||
| $CXX -O3 -fPIC -std=c++17 -Ivendor/highway -Isrc/jsc/bindings -I"$BUN_CODEGEN_DIR" -c src/jsc/bindings/highway_json.cpp -o "$SUP/highway_json.o" | ||
| for k in json xml; do | ||
| if [ -f src/jsc/bindings/highway_$k.cpp ]; then | ||
| $CXX -O3 -fPIC -std=c++17 -Ivendor/highway -Isrc/jsc/bindings -I"$BUN_CODEGEN_DIR" -c src/jsc/bindings/highway_$k.cpp -o "$SUP/highway_$k.o" | ||
| fi | ||
| done | ||
| # C/C++ XML parsers for benches/xml_parse.rs to compare against (optional). | ||
| XML_C_DEFS=() | ||
| XML_C_LIBS=() | ||
| PUGI_VERSION=1.14 | ||
| if [ ! -f "$SUP/pugixml-$PUGI_VERSION/src/pugixml.cpp" ]; then | ||
| curl -fsSL "https://github.com/zeux/pugixml/releases/download/v$PUGI_VERSION/pugixml-$PUGI_VERSION.tar.gz" | tar -xz -C "$SUP" || true | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Report the skipped pugixml comparison.
As per coding guidelines, never swallow failures or signal success after failure. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| fi | ||
| if [ -f "$SUP/pugixml-$PUGI_VERSION/src/pugixml.cpp" ]; then | ||
| build "$SUP/pugixml.o" $CXX -O3 -fPIC -std=c++17 -DNDEBUG -c "$SUP/pugixml-$PUGI_VERSION/src/pugixml.cpp" | ||
| XML_C_DEFS+=(-DHAVE_PUGIXML "-I$SUP/pugixml-$PUGI_VERSION/src") | ||
| fi | ||
| if [ -f /usr/include/expat.h ]; then XML_C_DEFS+=(-DHAVE_EXPAT); XML_C_LIBS+=(-Clink-arg=-lexpat); fi | ||
| if [ -d /usr/include/libxml2 ]; then XML_C_DEFS+=(-DHAVE_LIBXML2 -I/usr/include/libxml2); XML_C_LIBS+=(-Clink-arg=-lxml2); fi | ||
| $CXX -O3 -fPIC -std=c++17 ${XML_C_DEFS[@]+"${XML_C_DEFS[@]}"} -c src/parsers/benches/support/xml_c_shim.cpp -o "$SUP/xml_c_shim.o" | ||
| XML_CFG=() | ||
| for d in ${XML_C_DEFS[@]+"${XML_C_DEFS[@]}"}; do | ||
| case "$d" in -DHAVE_*) XML_CFG+=("--cfg" "$(echo "${d#-DHAVE_}" | tr A-Z a-z)") ;; esac | ||
| done | ||
|
|
||
|
claude[bot] marked this conversation as resolved.
|
||
| rm -f "$SUP/libbun_bench_cdeps.a" | ||
| ar rcs "$SUP/libbun_bench_cdeps.a" "$SUP"/*.o | ||
| ranlib "$SUP/libbun_bench_cdeps.a" | ||
|
|
@@ -52,10 +74,16 @@ export MIMALLOC_PURGE_DELAY=${MIMALLOC_PURGE_DELAY:-2000} | |
| export BUN_JSON_BENCH_FIXTURES=${BUN_JSON_BENCH_FIXTURES:-$PWD/bench/json-corpus} | ||
| CXXLIB=stdc++ | ||
| [ "$(uname -s)" = Darwin ] && CXXLIB=c++ | ||
| export RUSTFLAGS="${RUSTFLAGS:-} -Clink-arg=$PWD/$SUP/libbun_bench_cdeps.a -Clink-arg=-l$CXXLIB -Clink-arg=-lm -Clink-arg=-ldl -Clink-arg=-lpthread -Clink-arg=-lc" | ||
| export BUN_XML_BENCH_FIXTURES=${BUN_XML_BENCH_FIXTURES:-$PWD/bench/xml-corpus} | ||
| export RUSTFLAGS="${RUSTFLAGS:-} ${XML_CFG[*]-} -Clink-arg=$PWD/$SUP/libbun_bench_cdeps.a ${XML_C_LIBS[*]-} -Clink-arg=-l$CXXLIB -Clink-arg=-lm -Clink-arg=-ldl -Clink-arg=-lpthread -Clink-arg=-lc" | ||
|
|
||
| if [ "${1:-}" = "--test" ]; then | ||
| shift | ||
| exec cargo test -p bun_parsers --lib --release "$@" | ||
| fi | ||
| exec cargo bench -p bun_parsers --bench json_parse "$@" | ||
| BENCH=json_parse | ||
| if [ "${1:-}" = "--xml" ]; then | ||
| shift | ||
| BENCH=xml_parse | ||
| fi | ||
| exec cargo bench -p bun_parsers --bench "$BENCH" "$@" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| // Generates `xml_byte_class.h` (nibble LUTs, Highway SIMD kernel) and `xml_byte_class.rs` (the | ||
| // derived 256-entry table, Rust scalar indexer): both come from this one table so they agree. | ||
|
|
||
|
claude[bot] marked this conversation as resolved.
|
||
| import { mkdirSync } from "node:fs"; | ||
| import { resolve } from "node:path"; | ||
| import type { Config } from "./config.ts"; | ||
| import { writeIfChanged } from "./fs.ts"; | ||
|
|
||
| // Indexed everywhere: `&`, `\r`, and the control characters XML forbids. | ||
| const ALWAYS = 0x07; | ||
| // Indexed only between `<` and the next `>`: `\t`, `\n`, `"`, `'`, `=`. | ||
| const TAG = 0x38; | ||
| const LT = 0x40; | ||
| const GT = 0x80; | ||
|
|
||
| const LUT_LO = [0x03, 0x03, 0x13, 0x03, 0x03, 0x03, 0x07, 0x13, 0x03, 0x0a, 0x0a, 0x03, 0x43, 0x23, 0x83, 0x03]; | ||
| const LUT_HI = [0x09, 0x02, 0x14, 0xe0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00]; | ||
|
|
||
| const classOf = (b: number): number => LUT_LO[b & 0xf] & LUT_HI[b >> 4]; | ||
|
|
||
| function check() { | ||
| const expectedClass = (b: number): number => { | ||
| const ch = String.fromCharCode(b); | ||
| if (ch === "<") return LT; | ||
| if (ch === ">") return GT; | ||
| if (ch === "&" || ch === "\r") return ALWAYS; | ||
| if (b < 0x20 && ch !== "\t" && ch !== "\n") return ALWAYS; | ||
| if ("\t\n\"'=".includes(ch)) return TAG; | ||
| return 0; | ||
| }; | ||
| for (let b = 0; b < 0x100; b++) { | ||
| const got = classOf(b); | ||
| const expected = b < 0x80 ? expectedClass(b) : 0; | ||
| const ok = expected === 0 ? got === 0 : (got & expected) !== 0 && (got & ~expected) === 0; | ||
| if (!ok) { | ||
| throw new Error( | ||
| `xml_byte_class: byte 0x${b.toString(16)} classifies as 0x${got.toString(16)}, expected class 0x${expected.toString(16)}`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export function generateXmlByteClass(cfg: Config): { h: string; rs: string } { | ||
| check(); | ||
|
|
||
| const banner = (comment: string) => [ | ||
| `${comment} Generated by scripts/build/xmlByteClass.ts at configure time. Do not`, | ||
| `${comment} edit. The same definition feeds the Highway XML kernel (these nibble`, | ||
| `${comment} LUTs) and the Rust scalar indexer (the derived 256-entry table).`, | ||
| `${comment}`, | ||
| `${comment} cls = LUT_LO[b & 0xF] & LUT_HI[b >> 4]`, | ||
| `${comment} 0x07 always & \\r control 0x38 tag \\t \\n " ' =`, | ||
| `${comment} 0x40 < 0x80 >`, | ||
| "", | ||
| ]; | ||
| const hex = (v: number) => `0x${v.toString(16).padStart(2, "0")}`; | ||
|
|
||
| const h = [ | ||
| ...banner("//"), | ||
| "#pragma once", | ||
| "#include <stdint.h>", | ||
| "", | ||
| `#define BUN_XML_CLASS_ALWAYS ${hex(ALWAYS)}`, | ||
| `#define BUN_XML_CLASS_TAG ${hex(TAG)}`, | ||
| `#define BUN_XML_CLASS_LT ${hex(LT)}`, | ||
| `#define BUN_XML_CLASS_GT ${hex(GT)}`, | ||
| "", | ||
| `alignas(16) static const uint8_t kBunXmlLutLo[16] = { ${LUT_LO.map(hex).join(", ")} };`, | ||
| `alignas(16) static const uint8_t kBunXmlLutHi[16] = { ${LUT_HI.map(hex).join(", ")} };`, | ||
| "", | ||
| ].join("\n"); | ||
|
|
||
| const table: string[] = []; | ||
| for (let row = 0; row < 256; row += 16) { | ||
| const cells = []; | ||
| for (let b = row; b < row + 16; b++) cells.push(hex(classOf(b))); | ||
| table.push(` ${cells.join(", ")},`); | ||
| } | ||
| const allow = "#[allow(dead_code, unreachable_pub, unused)]"; | ||
| const rs = [ | ||
| ...banner("//"), | ||
| allow, | ||
| `pub const CLASS_ALWAYS: u8 = ${hex(ALWAYS)};`, | ||
| allow, | ||
| `pub const CLASS_TAG: u8 = ${hex(TAG)};`, | ||
| allow, | ||
| `pub const CLASS_LT: u8 = ${hex(LT)};`, | ||
| allow, | ||
| `pub const CLASS_GT: u8 = ${hex(GT)};`, | ||
| "", | ||
| "/// `LUT_LO[b & 0xF] & LUT_HI[b >> 4]` for every byte `b`.", | ||
| allow, | ||
| "#[rustfmt::skip]", | ||
| "pub const XML_BYTE_CLASS: [u8; 256] = [", | ||
| ...table, | ||
| "];", | ||
| "", | ||
| ].join("\n"); | ||
|
|
||
| mkdirSync(cfg.codegenDir, { recursive: true }); | ||
| const hPath = resolve(cfg.codegenDir, "xml_byte_class.h"); | ||
| const rsPath = resolve(cfg.codegenDir, "xml_byte_class.rs"); | ||
| writeIfChanged(hPath, h); | ||
| writeIfChanged(rsPath, rs); | ||
| return { h: hPath, rs: rsPath }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Define one strict
BUN_XML_BENCH_FIXTUREScontract.The JavaScript runner treats an empty value as unset and accepts relative paths. The Rust runner treats an empty value as an unreadable directory, ignores that error, and can panic while reading a selected fixture. The same configuration can therefore benchmark different corpora or silently fall back to synthetic inputs.
bench/xml/xml.mjs#L82-L85: Distinguish unset from empty, reject an empty configured value, and resolve the configured directory to an absolute path before file operations.src/parsers/benches/xml_parse.rs#L23-L43: Apply the same empty and absolute-path policy. Propagate directory enumeration, entry, and file-read errors instead of usingif let Ok,filter_map(|e| e.ok()), andunwrap().As per coding guidelines, use absolute file paths, distinguish empty and unset input, and propagate I/O failures explicitly.
📍 Affects 2 files
bench/xml/xml.mjs#L82-L85(this comment)src/parsers/benches/xml_parse.rs#L23-L43🤖 Prompt for AI Agents
Source: Coding guidelines