research: measured SentencePiece-BPE efficiency study - #13
Conversation
Autoresearch on encoder/sentencepiece.rs, the slowest encoder in the
library (2-3x vs HF on Gemma vs 25-154x elsewhere).
cargo is unusable in this environment (static.crates.io blocked by
egress policy, empty registry), so the study runs as a dependency-free
rustc harness that ports SentencePieceBPE verbatim as the baseline and
layers one isolated change per variant. Models are SP-BPE tokenizers
trained locally with HF tokenizers at 32K and 128K vocab.
Correctness is established rather than assumed: the baseline matches HF
tokenizers on every id (1,983,739 at 32K; 1,482,102 at 128K), and all
ten variants are asserted token-identical to it.
Headline, on document-sized inputs (the contract encode_batch uses):
32K vocab 128K vocab
A 4.98 MB/s 3.88 MB/s <- today
best 50.25 (10.1x cold) 44.60 (11.5x cold)
179.85 (36.1x warm) 194.43 (50.1x warm)
The win grows with vocabulary size, which matters because every model
worth applying it to is larger than what was measured.
Largest contributors, in order: per-metaspace-unit splitting behind the
interior-metaspace guard; unit memoization; an incremental-rank merge
that probes the pair table once per pair instead of per iteration; a
flat open-addressed pair table; an ASCII fast path in symbol init; and
SWAR metaspace scanning.
Also records what does NOT work - inline-token cache slots, the linear
merge without the flat table, front-table resizing, and a bigram-span
cache - and establishes a hard lookup-bound ceiling of ~192-199 MB/s
for the warm path, which the best variant already reaches.
No patch to the production encoder is included: it could not be
compiled or tested here, and the mapping from harness to encoder is
documented instead.
📝 WalkthroughWalkthroughAdds standalone Rust SentencePiece-BPE research harnesses, a Hugging Face tokenizer training/export script, reproducible benchmark commands, correctness validation, optimization variants, and measured implementation recommendations. ChangesSentencePiece-BPE research
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TrainHF
participant TokenizerJSON
participant RustHarness
participant HFReference
TrainHF->>TokenizerJSON: train and save Metaspace BPE model
TrainHF->>RustHarness: export vocabulary and merge TSV files
HFReference->>RustHarness: provide reference token IDs
RustHarness->>RustHarness: run encoder variants and benchmark results
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
Pull request overview
Adds a dependency-free research harness (plus training/export tooling and write-up) to measure and attribute performance improvements for tokie’s SentencePiece-BPE encoder, without requiring cargo/crates.io access.
Changes:
- Introduces Rust
rustc-buildable harnesses (sp_harness.rs,sp_harness2.rs) implementing multiple optimization variants with token-identity checks. - Adds a small HF
tokenizerstrainer/export script to produce flat vocab + merges artifacts for the harness. - Documents the methodology and measured results in a dedicated research note and a local README.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| research/sp-bpe/train_hf.py | Trains an SP-BPE-shaped tokenizer via HF tokenizers and exports vocab/merges artifacts for the harness. |
| research/sp-bpe/sp_harness2.rs | Full optimization ladder harness with multiple variants, correctness checks, and benchmarking drivers. |
| research/sp-bpe/sp_harness.rs | Smaller/intro harness covering baseline + early variants with basic benchmarking. |
| research/sp-bpe/README.md | Local instructions for training artifacts and running the harness. |
| docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md | Detailed write-up of experimental setup, results, and integration mapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data = json.load(open(f"{OUT}.json")) | ||
| model = data["model"] | ||
| vocab, merges = model["vocab"], model["merges"] |
| self.front_keys[i] = k; self.front_meta[i] = (o, l); | ||
| if (l as usize) <= INLINE_MAX { | ||
| let src = &self.arena[o as usize..o as usize + l as usize]; | ||
| let mut tmp = [0u32; INLINE_MAX]; | ||
| tmp[..l as usize].copy_from_slice(src); | ||
| self.front_inline[i] = tmp; | ||
| } | ||
| out.extend_from_slice(&self.arena[o as usize..o as usize + l as usize]); | ||
| true |
| fn hex_decode(s: &str) -> Vec<u8> { | ||
| (0..s.len() / 2).map(|i| u8::from_str_radix(&s[i * 2..i * 2 + 2], 16).unwrap_or(0)).collect() | ||
| } | ||
| fn load_vocab(p: &str) -> Vec<(u32, Vec<u8>)> { | ||
| std::fs::read_to_string(p).expect("vocab").lines().filter_map(|l| { | ||
| let mut it = l.split('\t'); | ||
| Some((it.next()?.parse().ok()?, hex_decode(it.next()?))) | ||
| }).collect() | ||
| } | ||
| fn load_merges(p: &str) -> Vec<(TokenId, TokenId)> { | ||
| std::fs::read_to_string(p).expect("merges").lines().filter_map(|l| { | ||
| let mut it = l.split('\t'); | ||
| Some((it.next()?.parse().ok()?, it.next()?.parse().ok()?)) | ||
| }).collect() | ||
| } |
| fn hex_decode(s: &str) -> Vec<u8> { | ||
| (0..s.len() / 2) | ||
| .map(|i| u8::from_str_radix(&s[i * 2..i * 2 + 2], 16).unwrap_or(0)) | ||
| .collect() | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md (1)
204-208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTag the fence with a language (
sh) — markdownlint MD040.🤖 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 `@docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md` around lines 204 - 208, Tag the fenced command block containing the rustc, python3, and sp_harness2 commands with the sh language identifier to satisfy markdownlint MD040.Source: Linters/SAST tools
research/sp-bpe/sp_harness.rs (2)
534-623: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueArena offsets are
u32with no bound check.
UnitCache::insertcastsarena.len()tou32unconditionally; past 4 Gi token ids the offset silently wraps andlookupreturns wrong tokens (or panics on slice range). Harness inputs are ~10 MB so it can't trigger today, but adebug_assert!(self.arena.len() <= u32::MAX as usize)documents the invariant cheaply.🤖 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 `@research/sp-bpe/sp_harness.rs` around lines 534 - 623, Add a debug assertion in UnitCache::insert before casting arena.len() to u32, verifying the arena length does not exceed u32::MAX as usize. Keep the existing offset and lookup representation unchanged while documenting this required invariant.
223-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
EncodeState::resultis never read.
encode_a/encode_b/encode_call write into the caller-providedout;resultis only allocated and cleared. Dropping it removes a misleading field from the ported state struct.🤖 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 `@research/sp-bpe/sp_harness.rs` around lines 223 - 238, Remove the unused result: Vec<TokenId> field from EncodeState, stop initializing it in EncodeState::new, and remove its clear call from EncodeState::clear. Leave symbols, heap, and the caller-provided output handling in encode_a, encode_b, and encode_c unchanged.research/sp-bpe/sp_harness2.rs (4)
582-585: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment overstates
encode_h_bits.It is a pure alias for
encode_h; the front-table size comes entirely from the caller-suppliedUnitCache. Either drop the wrapper or reword the comment so the sweep at Line 1097 isn't read as exercising a distinct code path.🤖 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 `@research/sp-bpe/sp_harness2.rs` around lines 582 - 585, Update encode_h_bits and its documentation: either remove the redundant wrapper and use encode_h directly, or reword the doc comment to describe it as an alias that relies on the caller-provided UnitCache. Ensure the sweep around the referenced call is not presented as testing a distinct front-table-size code path.
590-601: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer index-tracking over pointer arithmetic for unit bounds.
u.as_ptr() as usize - baserelies onFastUnitsalways yielding subslices oftext, and silently underflows if that ever stops holding. Yielding(start, end)from the iterator (or exposingpos) gives the same bounds without the raw-pointer dependency.🤖 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 `@research/sp-bpe/sp_harness2.rs` around lines 590 - 601, Update encode_j’s FastUnits iteration to obtain each unit’s start and end indices directly, using the iterator’s existing index/position API or changing it to yield those bounds, instead of deriving offsets with as_ptr and base. Preserve the current bounds contents and downstream encoding behavior.
287-312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSymbol/linear init is copy-pasted four times.
init_symbols_into,init_symbols_fast,encode_linear, andencode_linear_inceach re-implement the same UTF-8 walk +token_cache/byte_lutresolution. A shared#[inline(always)] fn next_token(&self, text, pos) -> (TokenId, usize)would keep the variants attributable while removing three copies that must stay in sync for the identity checks to mean anything.Also applies to: 396-418, 506-527
🤖 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 `@research/sp-bpe/sp_harness2.rs` around lines 287 - 312, Extract the duplicated UTF-8 traversal and token resolution from init_symbols_into, init_symbols_fast, encode_linear, and encode_linear_inc into a shared #[inline(always)] next_token method returning the resolved TokenId and consumed length. Update each caller to use this helper while preserving its existing symbol construction, output, and incremental behavior.
838-857: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
front_index(fixedFRONT_BITS) andfront_index_bitscoexist on the same type.
UnitCacheitself only uses thebitsvariant;front_indexexists solely forInlineCache, whose table is sized1 << FRONT_BITS. That's currently consistent, but the two indexers on one type invite a future mismatch between table size and index width. Consider givingInlineCacheits own private indexer.Also note
front_index_bitsshifts by64 - bits, which panics forbits == 0; the sweep only passes 12–20, so it's latent only.🤖 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 `@research/sp-bpe/sp_harness2.rs` around lines 838 - 857, Move the fixed-width front_index helper out of UnitCache and make it a private indexer owned by InlineCache, keeping its index width tied to InlineCache’s FRONT_BITS-sized table. Retain front_index_bits for UnitCache, and guard its shift calculation against bits == 0 without changing the existing 12–20-bit behavior.research/sp-bpe/train_hf.py (2)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnclosed handle and locale-dependent decoding.
-data = json.load(open(f"{OUT}.json")) +with open(f"{OUT}.json", encoding="utf-8") as fh: + data = json.load(fh)Same for the two
open(..., "w")calls — passencoding="utf-8"so hex/TSV output doesn't depend on the ambient locale.🤖 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 `@research/sp-bpe/train_hf.py` at line 23, Update the JSON loading and both output-file open calls in the training script to use context managers and explicitly specify encoding="utf-8". Ensure all file handles are closed deterministically and JSON, hex, and TSV processing is independent of the ambient locale.
20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorpus path is hardcoded while vocab size and output prefix are arguments.
sys.argv[3]with acorpus/clean.txtdefault keeps the script reproducible outside the exact layout inresearch/sp-bpe/README.md.🤖 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 `@research/sp-bpe/train_hf.py` at line 20, Update the argument handling around the training invocation to accept the corpus path as a configurable command-line argument, while preserving the existing default of corpus/clean.txt for reproducibility. Replace the hardcoded path in tok.train with the parsed corpus argument, keeping the existing vocab-size and output-prefix arguments unchanged.research/sp-bpe/README.md (1)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the
sp_harness.rsbuild line.The file's own header comment omits
--edition 2021while the ladder harness needs it; readers following this section will hit a mismatch. One extrarustcline here makes the earlier harness runnable too.🤖 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 `@research/sp-bpe/README.md` around lines 42 - 43, Add a rustc build command for sp_harness.rs in this README section, including the --edition 2021 flag required by the harness. Keep the existing explanation and ladder harness instructions unchanged.
🤖 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 `@docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md`:
- Around line 152-163: The “~79% of warm time is the cache lookup” claim is
inconsistent with the lookup-only and warm measurements in this section.
Recompute the percentage from the cited data or identify the measurement that
produces 79%, and reconcile the differing H warm/cold values across the Line
143, ladder, and doc-size measurements by explicitly labeling them as separate
runs if applicable.
- Around line 26-28: The sanity-check paragraph contradicts itself by calling
1.37× faster “squarely” within the README’s 2–3× range. Update the paragraph
around the baseline comparison to describe 1.37× as below but in the same
general order of magnitude, or explicitly explain the corpus, machine, or
vocabulary difference that accounts for the gap.
In `@research/sp-bpe/README.md`:
- Around line 20-22: Update the corpus-building command in the README to use a
find-based file selection that recursively reads regular files under
/usr/share/doc, avoiding shell globstar requirements and directory errors.
Preserve the existing corpus/clean.txt output and subsequent 8 MiB
benchmark-file creation.
In `@research/sp-bpe/sp_harness2.rs`:
- Around line 1056-1095: Update the document-splitting logic in the docsz
benchmark loop so each end boundary is advanced beyond any partial `▁` unit, not
merely to a UTF-8 character boundary. Use the existing unit-start/metaspace
representation and encoder context to identify safe boundaries, preserving
complete unit streams across docs; alternatively add a clear note that
per-document token identity is not guaranteed if boundary snapping cannot be
implemented.
In `@research/sp-bpe/train_hf.py`:
- Line 9: Update the vocabulary export logic in train_hf.py to recognize
byte-fallback token strings matching <0xXX> and convert each to its single raw
byte before writing the export. Preserve normal encoding for non-byte-fallback
tokens, ensuring load_vocab can populate byte_lut and token_cache for fallback
bytes; only remove byte_fallback if this decoding cannot be implemented.
---
Nitpick comments:
In `@docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md`:
- Around line 204-208: Tag the fenced command block containing the rustc,
python3, and sp_harness2 commands with the sh language identifier to satisfy
markdownlint MD040.
In `@research/sp-bpe/README.md`:
- Around line 42-43: Add a rustc build command for sp_harness.rs in this README
section, including the --edition 2021 flag required by the harness. Keep the
existing explanation and ladder harness instructions unchanged.
In `@research/sp-bpe/sp_harness.rs`:
- Around line 534-623: Add a debug assertion in UnitCache::insert before casting
arena.len() to u32, verifying the arena length does not exceed u32::MAX as
usize. Keep the existing offset and lookup representation unchanged while
documenting this required invariant.
- Around line 223-238: Remove the unused result: Vec<TokenId> field from
EncodeState, stop initializing it in EncodeState::new, and remove its clear call
from EncodeState::clear. Leave symbols, heap, and the caller-provided output
handling in encode_a, encode_b, and encode_c unchanged.
In `@research/sp-bpe/sp_harness2.rs`:
- Around line 582-585: Update encode_h_bits and its documentation: either remove
the redundant wrapper and use encode_h directly, or reword the doc comment to
describe it as an alias that relies on the caller-provided UnitCache. Ensure the
sweep around the referenced call is not presented as testing a distinct
front-table-size code path.
- Around line 590-601: Update encode_j’s FastUnits iteration to obtain each
unit’s start and end indices directly, using the iterator’s existing
index/position API or changing it to yield those bounds, instead of deriving
offsets with as_ptr and base. Preserve the current bounds contents and
downstream encoding behavior.
- Around line 287-312: Extract the duplicated UTF-8 traversal and token
resolution from init_symbols_into, init_symbols_fast, encode_linear, and
encode_linear_inc into a shared #[inline(always)] next_token method returning
the resolved TokenId and consumed length. Update each caller to use this helper
while preserving its existing symbol construction, output, and incremental
behavior.
- Around line 838-857: Move the fixed-width front_index helper out of UnitCache
and make it a private indexer owned by InlineCache, keeping its index width tied
to InlineCache’s FRONT_BITS-sized table. Retain front_index_bits for UnitCache,
and guard its shift calculation against bits == 0 without changing the existing
12–20-bit behavior.
In `@research/sp-bpe/train_hf.py`:
- Line 23: Update the JSON loading and both output-file open calls in the
training script to use context managers and explicitly specify encoding="utf-8".
Ensure all file handles are closed deterministically and JSON, hex, and TSV
processing is independent of the ambient locale.
- Line 20: Update the argument handling around the training invocation to accept
the corpus path as a configurable command-line argument, while preserving the
existing default of corpus/clean.txt for reproducibility. Replace the hardcoded
path in tok.train with the parsed corpus argument, keeping the existing
vocab-size and output-prefix arguments unchanged.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 1a424b2f-8829-4424-8e7b-f04208fdd35e
📒 Files selected for processing (5)
docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.mdresearch/sp-bpe/README.mdresearch/sp-bpe/sp_harness.rsresearch/sp-bpe/sp_harness2.rsresearch/sp-bpe/train_hf.py
| Sanity check on the regime: the baseline measures **1.37× faster than HF** on | ||
| this corpus, which lands squarely in the README's reported 2–3× band for | ||
| Gemma. The harness reproduces the problem tokie actually has. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"1.37× faster than HF … lands squarely in the README's reported 2–3× band" is self-contradictory.
1.37× is below the 2–3× band quoted at Line 4, so the sanity check as written argues the opposite of its conclusion. Either restate it as "somewhat below the README band, same order of magnitude" or explain the gap (different corpus/machine/vocab).
🤖 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 `@docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md` around
lines 26 - 28, The sanity-check paragraph contradicts itself by calling 1.37×
faster “squarely” within the README’s 2–3× range. Update the paragraph around
the baseline comparison to describe 1.37× as below but in the same general order
of magnitude, or explicitly explain the corpus, machine, or vocabulary
difference that accounts for the gap.
| | | 32K | 128K | | ||
| |---|---:|---:| | ||
| | unit split only, naive 3-byte scan | 571 MB/s | 558 MB/s | | ||
| | unit split only, SWAR scan | 892 MB/s | 897 MB/s | | ||
| | **split + cache lookup, no encoding** | **192 MB/s** | **199 MB/s** | | ||
| | measured warm, best variant | 190 MB/s | 193 MB/s | | ||
|
|
||
| **The warm path sits on the floor.** The BPE work is fully amortized away and | ||
| ~79% of warm time is the cache lookup itself. No further work *inside the | ||
| encoder* moves it — warm C/D/E/F/H/I all land in a 172–193 MB/s band that is | ||
| mostly run-to-run noise, and the only change that reliably moved warm was G | ||
| (SWAR splitting), because splitting is the other half of the floor. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The "~79% of warm time is the cache lookup" figure doesn't follow from the table above it.
With the lookup-only floor at 192 MB/s and warm at 190 MB/s, lookup accounts for ~99% of warm time, not 79% — which is in fact the stronger claim and matches the sentence right before it ("sits on the floor"). Please recompute or state which measurement 79% comes from.
Also worth reconciling: this section cites H warm 190.9 / cold 47.6 (Line 143) while the ladder table gives 49.05 and the doc-size table gives 179.85–205.85. If those come from separate runs, say so.
🤖 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 `@docs/superpowers/research/2026-07-26-sentencepiece-bpe-efficiency.md` around
lines 152 - 163, The “~79% of warm time is the cache lookup” claim is
inconsistent with the lookup-only and warm measurements in this section.
Recompute the percentage from the cited data or identify the measurement that
produces 79%, and reconcile the differing H warm/cold values across the Line
143, ladder, and doc-size measurements by explicitly labeling them as separate
runs if applicable.
| # 1. build a corpus (any UTF-8 text; the findings used system documentation) | ||
| mkdir -p corpus && cat /usr/share/doc/**/* > corpus/clean.txt | ||
| head -c 8388608 corpus/clean.txt > corpus/bench.txt |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
/usr/share/doc/**/* needs shopt -s globstar.
In a plain sh/default bash shell ** degrades to *, so the corpus is one level deep and much smaller than the 37 MB the findings doc assumes; cat also errors on the subdirectories it picks up. A find-based line reproduces the documented corpus:
📝 Suggested command
-mkdir -p corpus && cat /usr/share/doc/**/* > corpus/clean.txt
+mkdir -p corpus && find /usr/share/doc -type f -name '*.txt' -o -type f -name 'changelog*' \
+ | xargs cat > corpus/clean.txt 2>/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 1. build a corpus (any UTF-8 text; the findings used system documentation) | |
| mkdir -p corpus && cat /usr/share/doc/**/* > corpus/clean.txt | |
| head -c 8388608 corpus/clean.txt > corpus/bench.txt | |
| # 1. build a corpus (any UTF-8 text; the findings used system documentation) | |
| mkdir -p corpus && find /usr/share/doc -type f -name '*.txt' -o -type f -name 'changelog*' \ | |
| | xargs cat > corpus/clean.txt 2>/dev/null | |
| head -c 8388608 corpus/clean.txt > corpus/bench.txt |
🤖 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 `@research/sp-bpe/README.md` around lines 20 - 22, Update the corpus-building
command in the README to use a find-based file selection that recursively reads
regular files under /usr/share/doc, avoiding shell globstar requirements and
directory errors. Preserve the existing corpus/clean.txt output and subsequent 8
MiB benchmark-file creation.
| println!("\ndocument-sized inputs (corpus split into N-byte docs):"); | ||
| for docsz in [1024usize, 4096, 16384, 65536] { | ||
| let mut docs: Vec<&[u8]> = Vec::new(); | ||
| let mut i = 0; | ||
| while i < text.len() { | ||
| let mut e = (i + docsz).min(text.len()); | ||
| // keep doc boundaries on a UTF-8 char boundary | ||
| while e < text.len() && (text[e] & 0xC0) == 0x80 { e += 1; } | ||
| docs.push(&text[i..e]); | ||
| i = e; | ||
| } | ||
| let mut ba = f64::MAX; | ||
| for _ in 0..reps { | ||
| let t = Instant::now(); | ||
| let mut o = Vec::with_capacity(nb / 3); | ||
| for d in &docs { enc.encode_a(d, &mut st, &mut o); } | ||
| let e = t.elapsed().as_secs_f64(); if e < ba { ba = e; } | ||
| } | ||
| let mut ch = UnitCache::with_bits(16); | ||
| { let mut o = Vec::new(); for d in &docs { enc.encode_h(d, &mut st, &mut ch, &mut o); } } | ||
| let mut bh = f64::MAX; | ||
| for _ in 0..reps { | ||
| let t = Instant::now(); | ||
| let mut o = Vec::with_capacity(nb / 3); | ||
| for d in &docs { enc.encode_h(d, &mut st, &mut ch, &mut o); } | ||
| let e = t.elapsed().as_secs_f64(); if e < bh { bh = e; } | ||
| } | ||
| // cold-cache H (fresh cache per rep) = first-touch of a new corpus | ||
| let mut bc = f64::MAX; | ||
| for _ in 0..reps { | ||
| let t = Instant::now(); | ||
| let mut c = UnitCache::with_bits(16); | ||
| let mut o = Vec::with_capacity(nb / 3); | ||
| for d in &docs { enc.encode_h(d, &mut st, &mut c, &mut o); } | ||
| let e = t.elapsed().as_secs_f64(); if e < bc { bc = e; } | ||
| } | ||
| let mb = nb as f64 / 1048576.0; | ||
| println!(" doc={:>6}B n={:<6} A {:>7.2} H-cold {:>7.2} H-warm {:>7.2} MB/s (H-warm/A = {:.1}x)", | ||
| docsz, docs.len(), mb/ba, mb/bc, mb/bh, ba/bh); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document splitting can cut mid-unit, so per-doc token streams aren't identity-checked.
Boundaries are only snapped to UTF-8 char boundaries, not to ▁ unit starts, so encode_a and encode_h on the doc list can legitimately produce different tokens from each other and from the whole-text reference. Throughput comparison is still meaningful, but adding a note (or snapping e forward to the next metaspace) would prevent the table being read as a correctness-preserving measurement.
🔧 Snap doc boundaries to unit starts
let mut e = (i + docsz).min(text.len());
// keep doc boundaries on a UTF-8 char boundary
while e < text.len() && (text[e] & 0xC0) == 0x80 { e += 1; }
+ // keep doc boundaries on a metaspace-unit start so per-doc token
+ // streams concatenate to the whole-text reference
+ if let Some(p) = find_ms_fast(&text, e) { e = p; } else { e = text.len(); }🤖 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 `@research/sp-bpe/sp_harness2.rs` around lines 1056 - 1095, Update the
document-splitting logic in the docsz benchmark loop so each end boundary is
advanced beyond any partial `▁` unit, not merely to a UTF-8 character boundary.
Use the existing unit-start/metaspace representation and encoder context to
identify safe boundaries, preserving complete unit streams across docs;
alternatively add a clear note that per-document token identity is not
guaranteed if boundary snapping cannot be implemented.
| VOCAB = int(sys.argv[1]) if len(sys.argv) > 1 else 32000 | ||
| OUT = sys.argv[2] if len(sys.argv) > 2 else "spbpe_hf32k" | ||
|
|
||
| tok = Tokenizer(models.BPE(unk_token="<unk>", fuse_unk=True, byte_fallback=True)) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the export shape assumption against the harness loader and byte_lut use.
fd -t f 'sp_harness2.rs' --exec rg -n -C3 'byte_lut|fn load_vocab' {}Repository: feyninc/tokie
Length of output: 4500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -t f '^train_hf\.py$|sp_harness2\.rs$' .
echo "== train_hf.py =="
sed -n '1,80p' research/sp-bpe/train_hf.py
echo "== relevant harness sections =="
sed -n '220,250p' research/sp-bpe/sp_harness2.rs
sed -n '890,920p' research/sp-bpe/sp_harness2.rs
echo "== byte fallback references in repo =="
rg -n "byte_fallback|<0x|byte_lut|load_vocab|train_hf" .Repository: feyninc/tokie
Length of output: 16521
🌐 Web query:
sentencepiece BPE byte fallback token format <0xXX>
💡 Result:
In SentencePiece, when byte_fallback is enabled (via the byte_fallback option), characters that are not present in the model's vocabulary are decomposed into a sequence of raw UTF-8 byte tokens [1][2][3]. These tokens follow the specific format <0xXX>, where XX represents the hexadecimal value of the byte [4][5]. Key details about this format and behavior include: Format: Each byte token is a string consisting of 6 characters: the literal characters <0x, followed by a two-digit hexadecimal representation of the byte value (00-FF), and ending with a > character (e.g., <0xE3>) [4][5]. Model Implementation: - When enabled, the model reserves 256 special symbols to represent every possible byte value (0-255) [5][6]. - These symbols are assigned the type ModelProto::SentencePiece::Type::BYTE [3][7]. - During inference, if an unknown character is encountered, SentencePiece decomposes it into its corresponding UTF-8 byte sequence and maps each byte to its associated <0xXX> token [1][8]. Handling Offsets: - To maintain consistent text slicing, SentencePiece assigns specific spans to these fallback tokens [1][7]. - Intermediate byte tokens (all except the last one) are assigned a zero-width span (start, start), meaning they do not contribute to the character-level offset [1][7]. - The final byte token in the sequence is assigned the span of the entire original character (start, start + length), which ensures that slicing operations can correctly reconstruct the character [1][7]. This mechanism is highly recommended for modern Large Language Models (LLMs) to ensure that the tokenizer can process any input text without encountering unknown token errors [2].
Citations:
- 1: https://github.com/google/sentencepiece/blob/master/python/README.md
- 2: https://github.com/google/sentencepiece/blob/master/doc/options.md
- 3: https://github.com/google/sentencepiece/blob/273449044caa593c2fd7eb7550cb3ab2cff93f1a/src/sentencepiece_model.proto
- 4: https://docs.rs/shimmytok/latest/src/shimmytok/sentencepiece.rs.html
- 5: https://github.com/google/sentencepiece/blob/master/src/model_interface.cc
- 6: [Question] Why include characters that are in the set of fallback bytes? google/sentencepiece#840
- 7: https://github.com/google/sentencepiece/tree/master/python
- 8: https://github.com/google/sentencepiece/blob/master/src/sentencepiece_processor.cc
Decode <0xXX> byte-fallback tokens on vocab export.
When byte_fallback=True, HF writes byte tokens as literal strings like <0x0A>. The export writes every literal character’s bytes, while research/sp-bpe/sp_harness2.rs::load_vocab hex-decodes those tokens back to raw bytes. As a result, byte-fallback entries do not populate byte_lut/token_cache, so fallback bytes cannot be represented through the BPE path. Convert <0xXX> pieces back to the single byte on export, otherwise drop byte_fallback.
Applies to research/sp-bpe/train_hf.py:31-33.
🧰 Tools
🪛 Ruff (0.15.21)
[error] 9-9: Possible hardcoded password assigned to argument: "unk_token"
(S106)
🤖 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 `@research/sp-bpe/train_hf.py` at line 9, Update the vocabulary export logic in
train_hf.py to recognize byte-fallback token strings matching <0xXX> and convert
each to its single raw byte before writing the export. Preserve normal encoding
for non-byte-fallback tokens, ensuring load_vocab can populate byte_lut and
token_cache for fallback bytes; only remove byte_fallback if this decoding
cannot be implemented.
Autoresearch on encoder/sentencepiece.rs, the slowest encoder in the
library (2-3x vs HF on Gemma vs 25-154x elsewhere).
cargo is unusable in this environment (static.crates.io blocked by
egress policy, empty registry), so the study runs as a dependency-free
rustc harness that ports SentencePieceBPE verbatim as the baseline and
layers one isolated change per variant. Models are SP-BPE tokenizers
trained locally with HF tokenizers at 32K and 128K vocab.
Correctness is established rather than assumed: the baseline matches HF
tokenizers on every id (1,983,739 at 32K; 1,482,102 at 128K), and all
ten variants are asserted token-identical to it.
Headline, on document-sized inputs (the contract encode_batch uses):
A 4.98 MB/s 3.88 MB/s <- today
best 50.25 (10.1x cold) 44.60 (11.5x cold)
179.85 (36.1x warm) 194.43 (50.1x warm)
The win grows with vocabulary size, which matters because every model
worth applying it to is larger than what was measured.
Largest contributors, in order: per-metaspace-unit splitting behind the
interior-metaspace guard; unit memoization; an incremental-rank merge
that probes the pair table once per pair instead of per iteration; a
flat open-addressed pair table; an ASCII fast path in symbol init; and
SWAR metaspace scanning.
Also records what does NOT work - inline-token cache slots, the linear
merge without the flat table, front-table resizing, and a bigram-span
cache - and establishes a hard lookup-bound ceiling of ~192-199 MB/s
for the warm path, which the best variant already reaches.
No patch to the production encoder is included: it could not be
compiled or tested here, and the mapping from harness to encoder is
documented instead.
Summary by CodeRabbit
New Features
Documentation