-
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 13 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 |
|---|---|---|
| @@ -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[@]}" -c src/parsers/benches/support/xml_c_shim.cpp -o "$SUP/xml_c_shim.o" | ||
| XML_CFG=() | ||
| for d in "${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