perf(tokenizers): reuse one BPE merge queue per Encode call (#413)

[jamesburton]

Reuses one BPE merge queue per Encode call instead of allocating one per pre-tokenized
segment, per the "possible change" in #413.

Measured with GC.GetAllocatedBytesForCurrentThread over 20 warmed iterations
(ArrayPool at steady state, so this is per-call garbage), Llama-3.1-8B vocabulary
(tokenizer.ggml.pre = llama-bpe):

chars	before	after
1,024	77,032 B (75.2 B/char)	6,040 B (5.9 B/char)	12.8x
8,192	634,040 B (77.4 B/char)	18,072 B (2.2 B/char)	35.1x
32,768	2,530,584 B (77.2 B/char)	59,376 B (1.8 B/char)	42.6x

Those figures include a second change I have since dropped from this PR; the queue
hoist alone accounts for 2,530,584 → 157,680 B (4.81 B/char) at 32,768 chars, 16.0x.
See "what is not here" below.

EnsureCapacity after Clear is load-bearing, not tidiness. Letting the queue reach its
size by doubling allocates the whole chain of intermediate arrays, and a single-segment
input regressed from 32.73 to 64.88 B/char — worse than the per-segment allocation being
removed. Worth keeping if this is ever refactored.

The attribution needs one correction

The acceptance criteria asked for the attribution to be confirmed or refuted before any
change landed. It is conditionally confirmed — and the condition is not currently met
by the benchmark that motivated the issue.

TokenizerAllocationBenchmarks uses SmolLM-135M, whose tokenizer.ggml.pre is smollm.
That value is not in TiktokenPreTokenizer's table, so GetRegex returns null, _preRegex
is null, and Encode takes return EncodeSegment(gpt2Text) — the whole input is one
segment with one queue. There are no per-segment queues in that measurement at all.

I reproduced the issue's numbers exactly and instrumented the segment count:

chars	bytes/call	B/char	segments
1,024	33,608	32.82	1
8,192	268,088	32.73	1
32,768	1,072,471	32.73	1

So the ~32 B/char on that benchmark is one queue sized to the entire text — about 32 bytes
per element at roughly one element per character. That also explains the Gen2 column the
issue noticed: at 32,768 characters the backing array is ~1 MB and goes straight to the LOH.
This change is exactly neutral on that benchmark (33,608 / 268,088 / 1,072,471, unchanged),
which is why the numbers above use a llama-bpe vocabulary instead.

Two follow-ups this implies, both outside this PR:

1. The benchmark would be more representative with a mapped pre-type in its [Params], or a
   second model — otherwise it measures a path most models do not take.
2. smollm being unmapped is a correctness bug, not just a benchmarking artifact: without
   pre-tokenization, merges cross boundaries the model was trained to respect. That is moe(perf+vulkan): CPU quantized-direct + thread pool + Q6_K Vulkan indexed-expert + docs #237 /
   PR fix(tokenizers): pre-tokenize raw text before byte-encoding; add StarCoder/SmolLM pre-type family #417. Once it lands, this benchmark's own numbers change substantially — I measured
   32.73 → 83.13 B/char, because the segmented path then allocates a queue per pre-token.
   Applying this PR on top brings it to 16.76 B/char, below where it stands today.

What is not here

The new List<int>(gpt2Text.Length) over-reserve is real. Sizing it by an estimated token
count saves a further 2.66x on prose — but it costs more on a vocabulary that emits ~1
token per character, where growth by doubling exceeds the over-reserve avoided (I measured
40.0 → 43.0 B/char on such a case). That is a heuristic trade, not a strict improvement, so
it should be its own change with its own measurement rather than riding along here.

result.ToArray() needs the ITokenizer.Encode signature to change, as the issue notes.

Tests

PreTokenizedEncode_IsUnchangedByQueueReuse covers the risk this change introduces — a
reused queue leaking state between segments — via repeat calls, a fresh-instance comparison
and a decode round-trip. Existing tokenizer tests (220) pass unchanged.

I deliberately did not add an allocation-threshold unit test. The cost removed here is
the queue's growth during merging, so it only exists for a vocabulary with a real merge
table; the synthetic 256-byte vocab in the unit tests has none and measures identically with
and without this change (655,512 bytes either way). Such a test would pass whether or not the
fix were present.

Closes #413

[Copilot]

Pull request overview

This PR reduces per-call allocations in the tiktoken/GPT-2 BPE encoder by reusing a single merge PriorityQueue across all pre-tokenized segments within an Encode call, aligning with the optimization proposed in issue #413.

Changes:

• Hoists the BPE merge PriorityQueue to the Encode call scope and reuses it across segments by clearing/reseeding it (FillQueue).
• Refactors segment-encoding helpers to accept a caller-supplied scratch queue, avoiding per-segment queue allocations.
• Adds a unit test intended to guard against state leakage when reusing the queue across segments.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/DotLLM.Tokenizers/Bpe/Gpt2TiktokenEncoding.cs	Reuses one merge queue per Encode call via a new FillQueue helper and updated segment encode helpers.
tests/DotLLM.Tests.Unit/Tokenizers/BpeTokenizerTests.cs	Adds a regression test to detect incorrect behavior from queue reuse across segments/calls.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jamesburton]

Both addressed in 2d12a3b — both self-inflicted.

Stale comment (fixed). Correct: the comment described sizing by an estimated token count, which
is the variant I measured and then dropped from this PR, and I left the prose behind. It now records
the actual capacity and why the estimate was left out — 2.66x less garbage on prose, but worse on a
vocabulary emitting ~1 token per character (40.0 -> 43.0 bytes/char), so a heuristic trade rather
than a strict win.

Test comment vs test (fixed by adding the assertion). You are right that it claimed a comparison
it did not perform. I would rather have the test than a truthful comment about a weaker one, so the
segmentation comparison is now there: each pre-token piece is encoded through its own tokenizer
instance — hence its own never-reused queue — and the concatenation must equal the single call.

That is the actual leak check for sharing one queue across segments, which the previous assertions
(repeat-call stability, fresh-instance parity, decode round-trip) did not cover. It passes, which is
independent evidence that the reuse carries no state between segments.

Tests 39/39 in the touched class.