perf(decode): faster entity decoding#2248
Conversation
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughRefactors trie pointer arithmetic to end-relative addressing across encoder, decoder, and generated flags documentation, rewrites the decode data-map generation script to use base-91 slot-code encoding with delta+RLE atoms and BPE-merged ngrams, adds HTML4 entity data, and optimizes the entity decoder's numeric/named-entity streaming and non-streaming paths with packed numeric results and inlined fast paths, plus expanded regression tests. ChangesEntity Decoder Refactoring
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Input
participant stateNamedEntity
participant flushAndEmitLegacyOrReject
participant DecoderState as Decoder Instance
Input->>stateNamedEntity: write(chunk)
activate stateNamedEntity
stateNamedEntity->>stateNamedEntity: track local treeIndex/consumed/excess
stateNamedEntity->>stateNamedEntity: inline jump-table descent
stateNamedEntity->>stateNamedEntity: match compact-run entities
stateNamedEntity->>stateNamedEntity: retain best legacy match
alt chunk boundary or completion reached
stateNamedEntity->>flushAndEmitLegacyOrReject: finalize state
activate flushAndEmitLegacyOrReject
flushAndEmitLegacyOrReject->>DecoderState: persist treeIndex/consumed/excess
flushAndEmitLegacyOrReject->>DecoderState: emit entity or reject
deactivate flushAndEmitLegacyOrReject
end
stateNamedEntity->>Input: return consumed offset
deactivate stateNamedEntity
sequenceDiagram
participant Script as write-decode-map
participant BPE as bpeOptimize
participant Encoder as tryEncodeWithSplit
participant Decoder as decodeTrieDict
Script->>BPE: tokenize atoms, count pairs
BPE->>BPE: select profitable merges, promote ngrams
Script->>Encoder: partition atoms/ngrams into dict1/dict2
Encoder->>Encoder: deltaRleEncode atom streams
Encoder->>Encoder: emit slot codes for final data
Encoder->>Decoder: encoded stream + atomCount/dict1AtomCount/ngramCount/dictSize
Decoder->>Decoder: readSlotCode, decode dict1/dict2 atoms
Decoder->>Decoder: expand ngram slots
Decoder->>Decoder: expand final Uint16Array output
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/trie/encode-trie.spec.ts`:
- Around line 61-67: The comment describing the shared-child single-branch
layout in the encode-trie spec is stale: update the block that lists indices
[5],[6],[7] to reflect that this case encodes as a 1-slot jump table (so result
length is 7 with max index 6) rather than a 3-slot dictionary; adjust the listed
slot indexes and the "dest" value accordingly and clarify that dict pointers are
relative to (branchIndex + packedKeySlots + branchCount = 1 + 1 + 1 = 3) for
this case; modify the comment near the shared-child single-branch example and
any references to result length/indices named "result" so they match the actual
encoded layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a7d8beed-d5e7-4b6f-8321-6ac738f8a028
⛔ Files ignored due to path filters (2)
src/generated/decode-data-html.tsis excluded by!**/generated/**src/generated/decode-data-xml.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
maps/html4.jsonscripts/trie/decode-trie.tsscripts/trie/encode-trie.spec.tsscripts/trie/encode-trie.tsscripts/write-decode-map.tssrc/decode-stream.spec.tssrc/decode.spec.tssrc/decode.tssrc/internal/bin-trie-flags.tssrc/internal/decode-shared.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@maps/html4.json`:
- Around line 1-254: The HTML4 hot-set array in maps/html4.json is critical to
hot-node selection used by scripts/write-decode-map.ts and must be validated;
add a contract test that (1) loads maps/html4.json and asserts there are no
duplicate entries, and (2) loads maps/entities.json and asserts every string
from html4.json exists as a key in entities.json; implement the test near
existing map tests (or create a new test file) and fail the CI if duplicates or
missing entity names are detected so typos/omissions cannot silently change trie
layout or benchmarks.
In `@scripts/trie/encode-trie.spec.ts`:
- Around line 161-179: Add two unit tests that build a TrieNode with a
contiguous range of children of length 63 and length 64 (e.g., populate a Map
with keys 0..62 and 0..63), call encodeTrie on each, and assert they take
different branch-encoding paths: for the 63-length case assert the encoded
output matches the jump-table shape used elsewhere in tests (the branch
header/jump-table layout and non-zero slot pointers like in the existing
adjacent self-ref test), and for the 64-length case assert it falls back to the
dictionary encoding shape (i.e., does not have the jump-table header/layout).
Use the same encodeTrie entry point and the same structural checks (header byte
and pointer layout) relied on by other tests to lock the guard implemented
around lines 143–147.
In `@scripts/write-decode-map.ts`:
- Around line 653-661: The HTML-root validation must also ensure the header has
no value and is not a compact run: in scripts/write-decode-map.ts update the
existing check that inspects rootJumpOffset/rootBranchCount to also assert that
(data[0] & (BinTrieFlags.VALUE_LENGTH | BinTrieFlags.FLAG13)) === 0; if that
mask is non-zero throw the same Error (mentioning the header 0x...); this keeps
the invariant expected by decodeWithTrie and stateNamedEntity so the inline root
navigation won't be skipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2a98a66-18e8-47a0-9226-6bfe7cece1f7
⛔ Files ignored due to path filters (2)
src/generated/decode-data-html.tsis excluded by!**/generated/**src/generated/decode-data-xml.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
maps/html4.jsonscripts/trie/decode-trie.tsscripts/trie/encode-trie.spec.tsscripts/trie/encode-trie.tsscripts/write-decode-map.tssrc/decode-stream.spec.tssrc/decode.spec.tssrc/decode.tssrc/internal/bin-trie-flags.tssrc/internal/decode-shared.ts
| [ | ||
| "Aacute", | ||
| "aacute", | ||
| "Acirc", | ||
| "acirc", | ||
| "acute", | ||
| "AElig", | ||
| "aelig", | ||
| "Agrave", | ||
| "agrave", | ||
| "alefsym", | ||
| "Alpha", | ||
| "alpha", | ||
| "amp", | ||
| "and", | ||
| "ang", | ||
| "Aring", | ||
| "aring", | ||
| "asymp", | ||
| "Atilde", | ||
| "atilde", | ||
| "Auml", | ||
| "auml", | ||
| "bdquo", | ||
| "Beta", | ||
| "beta", | ||
| "brvbar", | ||
| "bull", | ||
| "cap", | ||
| "Ccedil", | ||
| "ccedil", | ||
| "cedil", | ||
| "cent", | ||
| "Chi", | ||
| "chi", | ||
| "circ", | ||
| "clubs", | ||
| "cong", | ||
| "copy", | ||
| "crarr", | ||
| "cup", | ||
| "curren", | ||
| "dagger", | ||
| "Dagger", | ||
| "darr", | ||
| "dArr", | ||
| "deg", | ||
| "Delta", | ||
| "delta", | ||
| "diams", | ||
| "divide", | ||
| "Eacute", | ||
| "eacute", | ||
| "Ecirc", | ||
| "ecirc", | ||
| "Egrave", | ||
| "egrave", | ||
| "empty", | ||
| "emsp", | ||
| "ensp", | ||
| "Epsilon", | ||
| "epsilon", | ||
| "equiv", | ||
| "Eta", | ||
| "eta", | ||
| "ETH", | ||
| "eth", | ||
| "Euml", | ||
| "euml", | ||
| "euro", | ||
| "exist", | ||
| "fnof", | ||
| "forall", | ||
| "frac12", | ||
| "frac14", | ||
| "frac34", | ||
| "frasl", | ||
| "Gamma", | ||
| "gamma", | ||
| "ge", | ||
| "gt", | ||
| "harr", | ||
| "hArr", | ||
| "hearts", | ||
| "hellip", | ||
| "Iacute", | ||
| "iacute", | ||
| "Icirc", | ||
| "icirc", | ||
| "iexcl", | ||
| "Igrave", | ||
| "igrave", | ||
| "image", | ||
| "infin", | ||
| "int", | ||
| "Iota", | ||
| "iota", | ||
| "iquest", | ||
| "isin", | ||
| "Iuml", | ||
| "iuml", | ||
| "Kappa", | ||
| "kappa", | ||
| "Lambda", | ||
| "lambda", | ||
| "lang", | ||
| "laquo", | ||
| "larr", | ||
| "lArr", | ||
| "lceil", | ||
| "ldquo", | ||
| "le", | ||
| "lfloor", | ||
| "lowast", | ||
| "loz", | ||
| "lrm", | ||
| "lsaquo", | ||
| "lsquo", | ||
| "lt", | ||
| "macr", | ||
| "mdash", | ||
| "micro", | ||
| "middot", | ||
| "minus", | ||
| "Mu", | ||
| "mu", | ||
| "nabla", | ||
| "nbsp", | ||
| "ndash", | ||
| "ne", | ||
| "ni", | ||
| "not", | ||
| "notin", | ||
| "nsub", | ||
| "Ntilde", | ||
| "ntilde", | ||
| "Nu", | ||
| "nu", | ||
| "Oacute", | ||
| "oacute", | ||
| "Ocirc", | ||
| "ocirc", | ||
| "OElig", | ||
| "oelig", | ||
| "Ograve", | ||
| "ograve", | ||
| "oline", | ||
| "Omega", | ||
| "omega", | ||
| "Omicron", | ||
| "omicron", | ||
| "oplus", | ||
| "or", | ||
| "ordf", | ||
| "ordm", | ||
| "Oslash", | ||
| "oslash", | ||
| "Otilde", | ||
| "otilde", | ||
| "otimes", | ||
| "Ouml", | ||
| "ouml", | ||
| "para", | ||
| "part", | ||
| "permil", | ||
| "perp", | ||
| "Phi", | ||
| "phi", | ||
| "Pi", | ||
| "pi", | ||
| "piv", | ||
| "plusmn", | ||
| "pound", | ||
| "prime", | ||
| "Prime", | ||
| "prod", | ||
| "prop", | ||
| "Psi", | ||
| "psi", | ||
| "quot", | ||
| "radic", | ||
| "rang", | ||
| "raquo", | ||
| "rarr", | ||
| "rArr", | ||
| "rceil", | ||
| "rdquo", | ||
| "real", | ||
| "reg", | ||
| "rfloor", | ||
| "Rho", | ||
| "rho", | ||
| "rlm", | ||
| "rsaquo", | ||
| "rsquo", | ||
| "sbquo", | ||
| "Scaron", | ||
| "scaron", | ||
| "sdot", | ||
| "sect", | ||
| "shy", | ||
| "Sigma", | ||
| "sigma", | ||
| "sigmaf", | ||
| "sim", | ||
| "spades", | ||
| "sub", | ||
| "sube", | ||
| "sum", | ||
| "sup", | ||
| "sup1", | ||
| "sup2", | ||
| "sup3", | ||
| "supe", | ||
| "szlig", | ||
| "Tau", | ||
| "tau", | ||
| "there4", | ||
| "Theta", | ||
| "theta", | ||
| "thetasym", | ||
| "thinsp", | ||
| "THORN", | ||
| "thorn", | ||
| "tilde", | ||
| "times", | ||
| "trade", | ||
| "Uacute", | ||
| "uacute", | ||
| "uarr", | ||
| "uArr", | ||
| "Ucirc", | ||
| "ucirc", | ||
| "Ugrave", | ||
| "ugrave", | ||
| "uml", | ||
| "upsih", | ||
| "Upsilon", | ||
| "upsilon", | ||
| "Uuml", | ||
| "uuml", | ||
| "weierp", | ||
| "Xi", | ||
| "xi", | ||
| "Yacute", | ||
| "yacute", | ||
| "yen", | ||
| "yuml", | ||
| "Yuml", | ||
| "Zeta", | ||
| "zeta", | ||
| "zwj", | ||
| "zwnj" | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add a contract test for HTML4 hot-set integrity.
This list directly drives hot-node selection in scripts/write-decode-map.ts; a typo/duplicate/missing entry silently changes trie layout and benchmark behavior. Please add a test that asserts this array is duplicate-free and every name exists in maps/entities.json.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@maps/html4.json` around lines 1 - 254, The HTML4 hot-set array in
maps/html4.json is critical to hot-node selection used by
scripts/write-decode-map.ts and must be validated; add a contract test that (1)
loads maps/html4.json and asserts there are no duplicate entries, and (2) loads
maps/entities.json and asserts every string from html4.json exists as a key in
entities.json; implement the test near existing map tests (or create a new test
file) and fail the CI if duplicates or missing entity names are detected so
typos/omissions cannot silently change trie layout or benchmarks.
Decoder (src/decode.ts): - Non-streaming decodeWithTrie: inline root navigation (the HTML root is a [A-Za-z] jump table, validated at build time), an inline descent loop that walks all value-less jump-table levels (incl. the single-branch encoding) without calling determineBranch, inline compact-run matching, and inline value extraction for single-char leaves at the semicolon emit site. - Streaming EntityDecoder: the same jump-table descent inline in stateNamedEntity, resumable walk state kept in locals (flushed at chunk boundaries) instead of per-character field writes, and a single idempotent legacy-match record at the loop top + chunk-end epilogue. Numeric states use local accumulators the same way. - determineBranch (shared by both paths) uses a linear scan for packed dictionaries (>90% of dict nodes have <= 4 branches) and handles end-relative pointers. - A single-character check at the resume offset skips the indexOf call between adjacent entities. - Numeric entities: split decimal/hex scan loops, packed (consumed << 21 | codePoint) return with an overflow clamp past U+10FFFF, and a String.fromCharCode fast path for plain BMP code points that replaceCodePoint passes through unchanged. - EntityDecoder reports the correct consumed count for legacy entities ending in a compact run (previously one extra character, making streaming consumers swallow the character after entities like Á when not semicolon-terminated). Trie encoding (scripts/, src/generated/, src/internal/): - Jump-table eligibility is hot/cold: nodes on HTML4 entity paths (maps/html4.json) or with entity traffic >= 16 allow 4x jump-table overhead; the long tail of rare HTML5 names uses the compact dictionary encoding. - Branch pointers are stored relative to the end of the branch data, making the common "child follows the branch array" case a small constant — better for compression, same arithmetic in the decoder, and the stored=0 sentinel collision becomes structurally impossible. - The trie data ships as a dict+BPE-coded base-91 string (25 merges, brotli-aware rank overrides) decoded once at module load by src/internal/decode-shared.ts. src/internal/bin-trie-flags.ts is the authoritative format reference. vs the previous encoding/decoder (html-entity-benchmarks texts, darwin arm64) — non-streaming decodeNormal: short-high-named 7.39M -> 9.70M ops/s (+31%) medium-high-named 299.6K -> 429.2K (+43%) long-high-named 27.5K -> 40.5K (+47%) geo mean 1.13M -> 1.30M (+15%) Streaming EntityDecoder (htmlparser2-style driver, ns/op): short-high-named 101 -> 76 (-25%) medium-high-named 3,525 -> 2,714 (-23%) long-high-named 38.6µs -> 29.7µs (-23%) numeric variants flat (already at parity with sync) Bundle (esbuild --minify on src/decode.ts): 16,289 gzip / 15,357 brotli — within ~25 B gzip of the previous encoding. Tests: every entity in the WHATWG/XML/legacy maps decodes through the sync and streaming implementations; streaming consumed counts for compact-run legacy entities; numeric overflow clamping; inputs that hit misindexed legacy lookups in entities <= 7.0.1 stay literal.
The seven cold reject sites in stateNamedEntity flushed the locally-tracked walk state inline before calling emitLegacyOrReject; fold the flush into a flushAndEmitLegacyOrReject helper. Behavior and benchmarks unchanged.
Apply the same two wins decodeWithTrie already has to the hand-coded XML matcher: check the single character at the resume position before calling indexOf (adjacent entities), and use String.fromCharCode for numeric code points in [1..0x7F] and [0xA0..0xD7FF] that pass replaceCodePoint unchanged. -16% on entity-dense XML text.
Pin the streaming decoder's boundary behavior: legacy matches reached mid-descent or via a compact run split across chunks are recorded at the chunk end (so end() emits them with the right consumed count), strict-only matches are not, and attribute terminator rules apply to the first character of the following chunk.
Rebasing onto main pulled in @feedic/eslint-config 0.5.0 (#2261), which adds several unicorn rules the pre-rebase branch predated: - Restore isStrict/isAttribute parameter names in decodeWithTrie (matches main; the rewrite had reverted them to strict/attribute). - Declare EntityDecoder walk-state fields before the constructor and keep the existing inline disable on stateNumericStart, matching main. - Inline-disable unicorn/no-break-in-nested-loop on the hand-optimized hot-loop breaks in decodeWithTrie/stateNamedEntity (same convention main already uses) and on the two build-script scan loops. - Rewrite two ternaries into the minimal form; disable the rule on the variable-length int decode whose branches read a different number of side-effecting bytes. - Keep Number.NEGATIVE_INFINITY (biome's useNumberNamespace enforces it) with the same inline eslint disable used elsewhere in the repo. - Assorted script-only style fixes (push/length, Map iteration, ternary). No behavior change: 350 tests pass, trie regeneration is byte-identical.
Two latent invariants the current ASCII entity data never exercises but that would silently corrupt a regenerated trie: - encode-trie: the jump-table first char is OR'd into the 7-bit JUMP_TABLE header field, but the guard asserted <= 16 bits (copied from the pointer check) instead of <= 7 like the single-branch path. A branch node whose smallest child char is >= 128 would overflow into BRANCH_LENGTH undetected. Assert the actual field width. - write-decode-map: the HTML-root check required a multi-branch jump table but not that the root is value-less and not a compact run, which the decoder's inline root navigation also assumes. A root with a value or FLAG13 set would skip the descent loop and reject every entity. Extend the guard (per PR review). Validation-only: trie regeneration remains byte-identical, 350 tests pass.
… boundary Guard three latent build-time invariants the current entity data does not exercise, so a future/custom map fails loudly instead of shipping a corrupt trie: - encode-trie: assert the encoded trie fits uint16 index space. Branch pointers are stored end-relative mod 2^16 and the decoder reconstructs them with `& 0xffff`, so a trie > 65536 words would alias a forward pointer onto the wrong node with no per-pointer check catching it. - write-decode-map: throw when dict1AtomCount exceeds the atom count. The dict1 slice clamps to atomCount but the header still reports the larger count, which would make the decoder's decodeDelta over-read the following streams. - encode-trie.spec: pin the 6-bit BRANCH_LENGTH boundary — 63 contiguous children use a jump table, a span > 63 with <=63 branches falls back to the dictionary, and 64 branches is unencodable (throws). Note 64 does not "fall back to dictionary": the dictionary branch-count field is 6-bit too. Validation/tests only: trie regeneration stays byte-identical, 353 tests pass.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR significantly optimizes HTML/XML named-entity decoding performance by revising the binary trie format (end-relative pointers, compact runs) and introducing a denser, dictionary/BPE-based encoding for shipped trie data, alongside streaming correctness fixes at chunk boundaries.
Changes:
- Reworked named-entity traversal for both sync and streaming decoders (inline jump-table descent, compact-run handling, reduced per-character field stores).
- Switched shipped HTML trie data to a dictionary + BPE-coded base-91 string format, and updated the generator/encoder accordingly.
- Added broad regression coverage (full WHATWG/XML/legacy maps + streaming chunk-boundary/compact-run consumed-count invariants).
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/internal/decode-shared.ts | Replaces the old dict-header decoder with a slot-based atom/ngram/data stream decoder for the new base-91+BPE trie string format. |
| src/internal/bin-trie-flags.ts | Expands documentation to precisely describe trie node/branch layouts and end-relative pointer semantics. |
| src/generated/decode-data-xml.ts | Updates the generated XML trie array to match the new encoding/pointer scheme. |
| src/generated/decode-data-html.ts | Updates the generated HTML trie payload and call signature to the new decodeTrieDict format parameters. |
| src/decode.ts | Major performance refactor: inline jump-table descent, improved compact-run streaming behavior, faster numeric entity handling, and adjacent-entity scanning optimizations. |
| src/decode.spec.ts | Adds full-map regression tests for WHATWG entities, XML entities, and legacy entities (no semicolon), plus literal-prefix non-entity guards. |
| src/decode-stream.spec.ts | Adds focused streaming tests for legacy consumed-count correctness around compact runs and chunk-boundary legacy match recording. |
| scripts/write-decode-map.ts | Overhauls trie generation: hot/cold node heuristics, BPE merge optimization, base-91 slot coding, and updated file generation strategy. |
| scripts/trie/encode-trie.ts | Changes trie pointer encoding to end-relative modulo-uint16, adds per-node overhead control, and enforces 6-bit branch-length constraints. |
| scripts/trie/encode-trie.spec.ts | Updates/expands tests to validate end-relative pointers and 6-bit BRANCH_LENGTH behavior (including boundary cases). |
| scripts/trie/decode-trie.ts | Updates debug decoding to interpret end-relative pointers for both dictionary and jump-table layouts. |
| maps/html4.json | Adds the HTML4 entity-name set used to bias encoding decisions toward real-world “hot” entities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/internal/bin-trie-flags.ts`:
- Around line 29-30: The compact-run documentation in BinTrieFlags is incorrect:
the encoder only uses compact runs when the chain length is greater than 2, so
update the spec comment in bin-trie-flags to describe compact runs as 3..63
rather than 2..63. Keep the wording aligned with the behavior in encode-trie.ts
and the compact-run/branch encoding description so the documented format matches
what the trie encoder actually emits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a8c82b2-b506-424a-9e1c-7f18b77e3b39
⛔ Files ignored due to path filters (2)
src/generated/decode-data-html.tsis excluded by!**/generated/**src/generated/decode-data-xml.tsis excluded by!**/generated/**
📒 Files selected for processing (10)
maps/html4.jsonscripts/trie/decode-trie.tsscripts/trie/encode-trie.spec.tsscripts/trie/encode-trie.tsscripts/write-decode-map.tssrc/decode-stream.spec.tssrc/decode.spec.tssrc/decode.tssrc/internal/bin-trie-flags.tssrc/internal/decode-shared.ts
The html4.json name list drives the hot/cold trie-encoding split, but nothing verified its integrity: a duplicated name would waste hot budget and a name missing from entities.json would silently be skipped when marking hot paths. Pin both invariants in a spec. Also correct the compact-run documentation: the encoder only emits runs when the length exceeds 2 (scripts/trie/encode-trie.ts), so the BRANCH_LENGTH range is 3..63, not 2..63.
decodeTrieDict runs once per import and was written for bundle size, not cold-VM speed: per-char alphabet arithmetic in an `at()` closure, `[...a, ...b]` spreads per ngram, and for...of iterators in the hot data loop cost ~1ms and ~945KB of transient garbage per fresh process. Rework the hot paths for a cold VM, keeping the format unchanged: - Precompute a Uint8Array base-91 inverse table at module load instead of re-deriving the alphabet arithmetic per character. - Store slot contents in flat typed arrays: atoms directly in a `single` value table, ngram expansions in one shared Uint16Array pool addressed by start/length — no per-slot number[] allocations. - Indexed loops everywhere; ngram refs are collected in one pass to size the pool exactly. - Warm the function with a minimal decode at module load so V8 baseline-compiles it before the ~18k-char real call (~15% on its own). Fresh-process benchmarks (min/median of 20 runs, Apple Silicon): decodeTrieDict cold 1013/1062us -> 531/584us (1.9x/1.8x) full library import 3394/3630us -> 2642/2787us transient garbage 945KB -> ~50KB; retained: no increase The decoded trie is byte-identical to the previous decoder's output on the generated HTML data string.
parseNumericEntity packs (consumed << 21) | codePoint into one integer.
The 11-bit consumed field wraps for entities of 2048+ characters, so
decodeHTML('&#' + '1'.repeat(2048) + ';X') mis-consumed the entity and
leaked digits into the output (a regression vs entities@7.0.1, which
consumed the full entity and emitted U+FFFD). The old comment claiming
the wrap could "never [emit] an incorrect character" was wrong.
Pin the consumed field at 0x7ff for entities of >= 2047 characters and
spill the true length into a module-level side channel that callers
read back — the packed fast path is untouched for every realistic
input. Verified against entities@7.0.1 across decimal/hex bodies of
1..4096 digits, +/- semicolon, all four decode flavors (1536 cases).
Also:
- Clamp the streaming accumulators in stateNumericDecimal/-Hex to
0x110000 per digit, matching the sync parser, so the errors callbacks
see a finite code point instead of Infinity for absurdly long bodies.
- Cross-reference the sync and streaming numeric parsers, which must
stay in sync.
- Turn NumericPacking into plain consts: with isolatedModules, const
enum member reads compile to runtime property loads, which showed up
in numeric-heavy benchmarks once the new overflow checks were added.
With plain consts the fix benchmarks neutral (within 1% on an
entity-dense corpus).
- Boundary tests: 2045/2046/2047/2048/4096-digit bodies, decimal + hex,
+/- semicolon, all four decode flavors, sync + streaming, plus
EntityDecoder consumed counts and errors-callback clamping.
The BMP fast-path expression for turning a numeric entity's code point into a string was duplicated verbatim in decodeWithTrie and decodeXML. Move it next to replaceCodePoint in decode-codepoint.ts (keeping the 0xd760 range comment there) and call it from both sites. No measurable change on a numeric-entity-heavy benchmark — V8 inlines the helper.
The dict/BPE string encoder (writer) and decodeTrieDict (reader) are hand-synced implementations of one format, and the streaming trie descent was the only reader without a full-map test. Pin all of it: - Extract the base-91/BPE encoder from write-decode-map.ts into scripts/trie/encode-dict.ts (unchanged logic; the script wrote files at import time, so the encoder wasn't importable from a spec). Generated files are byte-identical. - New encode-dict.spec.ts round-trips the writer's encoding through the runtime decodeTrieDict: both real map tries, BPE-heavy patterns, delta escape/double-escape and RLE-chunking edge values, and the whole dictSize search grid. - New exhaustive streaming spec: every entity from entities.json and xml.json through EntityDecoder, whole and char-by-char, asserting output and consumed match decodeHTML/decodeXML. - Staleness guard in write-decode-map.ts: BPE_RANK_OVERRIDES were tuned against the exact current trie contents, so assert the encoded HTML trie length against a recorded constant and tell future editors to re-tune when it fires.
|
Pushed a round of follow-up commits (merge of
Fresh-process startup (min/median of 20 runs, Node 26, Apple Silicon):
Transient decode garbage dropped from ~945 KB to ~50 KB per process; retained memory unchanged. The import gap vs main shrank from +870 µs to +190 µs. Differential validation vs |
Faster named-entity decoding and a denser trie encoding:
html-entity-benchmarks (one process per variant, best of 5): decodeNormal geo mean 1.37M → 1.57M ops/s (+15%), decodeStrict +14% — entity-dense named texts +43% to +72% — decodeXML flat (+1%). Details in the trie commit message.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes