Repository navigation
perf(onnx): pad each batch to its own longest text, not to 512 - #124
Merged
Merged
Conversation
The dense build spent 2 210.8 s on 7 307 chunks (3.3 ch/s). The cost did not depend on text length: a batch of 32 short texts took 7.9 s, a batch of 32 long texts 6.2 s. Root cause: the ONNX tokenizer was configured with enable_padding(length=512) and both inference paths padded every input to max_length before session.run, so a 31-token chunk paid for 512 tokens. Direct session, batch=32 (this machine): seq= 32 -> 0.17 s (190.7 ch/s) seq=128 -> 0.86 s ( 37.1 ch/s) seq=256 -> 2.30 s ( 13.9 ch/s) seq=512 -> 6.33 s ( 5.1 ch/s) Fix: pad to the longest text in the current batch; keep truncation at 512. The indexer already sorts chunks by length (index_project_runner), so batch maxima are close to the real lengths. After the fix, server timings: 32 x short 0.06 s (464 ch/s) 32 x medium 0.63 s ( 50 ch/s) 32 x long 6.16 s (5.2 ch/s, unchanged) Full rebuild on the same corpus: 7 419 chunks in 1 014.7 s vs 2 210.8 s before (the "before" run shared CPU with pytest, so the clean effect is the short-batch measurement: 0.06 s vs ~6.4 s; corpus estimate 2.13x). Compatibility: per-batch padding changes the tensor width, and quantised inference is not bit-identical (cosine p50 0.998, p5 0.996, min 0.863 over 7 059 identical chunks; same-index top-1 query match 14/15). ONNX_FIXED_PADDING=512 restores the legacy width for deployments that must match an old index exactly. No quality panel was run yet - that is the gate before making this the only mode. Docs: ONNX_FIXED_PADDING in CONFIGURATION en/ru/zh; investigation in experiments/onnx_dense_2026_11/REPORT.md; diary entry with numbers. Gates: pytest -m "not integration and not benchmark" 2608 passed / 10 skipped (was 2607/9), ruff OK, dense smoke_e2e PASSED (4 checks, step 4 = vector, provider=onnx, dim=384, results=3), stale_detector and docs=config guards OK.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
Before deciding on #124, run the documented 48-query panel on two dense indexes of this repository: control = fixed-512 padding (index + query), treatment = per-batch padding. Same hybrid `quality` path, same text index, only the vector table and query padding differ. control hit@1=0.562 hit@5=0.688 MRR=0.601 p50=274ms treatment hit@1=0.583 hit@5=0.688 MRR=0.617 p50=258ms paired: hit@1 +1/-0, hit@5 0/0 (33/48 both), MRR +2/-0 No query became worse on the panel; the panel is small (n=48), so the claim is "non-inferior on the frozen panel", not a general quality statement. Adds the reproducible harness (quality_panel.py), both result JSONs and the report section. Gates: pytest 3 dynamic-padding tests passed, ruff OK.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
perf(onnx): pad each batch to its own longest text, not to 512
The dense build spent 2 210.8 s on 7 307 chunks (3.3 ch/s). The cost did not
depend on text length: a batch of 32 short texts took 7.9 s, a batch of 32 long
texts 6.2 s. Root cause: the ONNX tokenizer was configured with
enable_padding(length=512) and both inference paths padded every input to
max_length before session.run, so a 31-token chunk paid for 512 tokens.
Direct session, batch=32 (this machine):
seq= 32 -> 0.17 s (190.7 ch/s)
seq=128 -> 0.86 s ( 37.1 ch/s)
seq=256 -> 2.30 s ( 13.9 ch/s)
seq=512 -> 6.33 s ( 5.1 ch/s)
Fix: pad to the longest text in the current batch; keep truncation at 512. The
indexer already sorts chunks by length (index_project_runner), so batch maxima
are close to the real lengths. After the fix, server timings:
32 x short 0.06 s (464 ch/s)
32 x medium 0.63 s ( 50 ch/s)
32 x long 6.16 s (5.2 ch/s, unchanged)
Full rebuild on the same corpus: 7 419 chunks in 1 014.7 s vs 2 210.8 s before
(the "before" run shared CPU with pytest, so the clean effect is the short-batch
measurement: 0.06 s vs ~6.4 s; corpus estimate 2.13x).
Compatibility: per-batch padding changes the tensor width, and quantised
inference is not bit-identical (cosine p50 0.998, p5 0.996, min 0.863 over 7 059
identical chunks; same-index top-1 query match 14/15). ONNX_FIXED_PADDING=512
restores the legacy width for deployments that must match an old index exactly.
No quality panel was run yet - that is the gate before making this the only mode.
Docs: ONNX_FIXED_PADDING in CONFIGURATION en/ru/zh; investigation in
experiments/onnx_dense_2026_11/REPORT.md; diary entry with numbers.
Gates: pytest -m "not integration and not benchmark" 2608 passed / 10 skipped
(was 2607/9), ruff OK, dense smoke_e2e PASSED (4 checks, step 4 = vector,
provider=onnx, dim=384, results=3), stale_detector and docs=config guards OK.