Skip to content

[Spec][Ngram] Followup fixes for MatchState incremental advance#22180

Merged
hnyls2002 merged 3 commits intomainfrom
lsyin/ngram-match-state-followup-fixes
Apr 6, 2026
Merged

[Spec][Ngram] Followup fixes for MatchState incremental advance#22180
hnyls2002 merged 3 commits intomainfrom
lsyin/ngram-match-state-followup-fixes

Conversation

@hnyls2002
Copy link
Copy Markdown
Collaborator

@hnyls2002 hnyls2002 commented Apr 6, 2026

Followup fixes for #21243.

Summary

  • Eliminate per-token heap allocation in advanceMatchState_() — reuse a single vector<NodeRef> across loop iterations
  • Access root_ directly instead of going through resolve(state, rootRef()) which was a tautological check on a freshly-created ref
  • Move test_ngram_corpus.py from spec/utils/ to unit/spec/ and switch from register_cuda_ci to register_cpu_ci (ngram corpus is pure C++, no GPU needed)
  • Add TestNgramCorpusMatchBenchmark — measures incremental advance vs full rebuild per-step latency with max_trie_depth=18
  • Add clarifying comments: TrieNode::version / NodeRef::version invariant, reset() epoch-based invalidation, match state lifecycle in ngram_worker.py
  • Add x/N progress tracking and timing to rerun-test.yml (both CUDA and CPU jobs)

Benchmark (CPU, max_trie_depth=18)

Incremental: 32.4 us/step
Rebuild:     45.3 us/step
Speedup:     1.40x

match() is O(D) incremental vs O(D²) rebuild, but only accounts for a fraction of total batch_get latency (FFI + buildRecency + fillResult dominate). The 1.40x end-to-end speedup is consistent with expectations at D=18.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/rerun-test test_ngram_corpus.py

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

ubuntu-latest (1 test): View workflow run

cd test/ && python3 registered/unit/spec/test_ngram_corpus.py

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/rerun-test test/registered/unit/spec/test_ngram_corpus.py

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

ubuntu-latest (1 test): View workflow run

cd test/ && python3 registered/unit/spec/test_ngram_corpus.py

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/rerun-test test/registered/unit/spec/test_ngram_corpus.py test/registered/unit/sampling/test_penaltylib.py test/registered/unit/sampling/test_sampling_params.py

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

ubuntu-latest (3 tests): View workflow run

cd test/ && python3 registered/unit/spec/test_ngram_corpus.py
cd test/ && python3 registered/unit/sampling/test_penaltylib.py
cd test/ && python3 registered/unit/sampling/test_sampling_params.py

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/rerun-test test/registered/unit/spec/test_ngram_corpus.py

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

ubuntu-latest (1 test): View workflow run

cd test/ && python3 registered/unit/spec/test_ngram_corpus.py

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/rerun-test test/registered/spec/test_ngram_speculative_decoding.py

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

1-gpu-h100 (1 test): View workflow run

cd test/ && python3 registered/spec/test_ngram_speculative_decoding.py

@hnyls2002
Copy link
Copy Markdown
Collaborator Author

/tag-and-rerun-ci

@github-actions github-actions bot added the run-ci label Apr 6, 2026
@hnyls2002 hnyls2002 merged commit 6de2ff2 into main Apr 6, 2026
87 of 110 checks passed
@hnyls2002 hnyls2002 deleted the lsyin/ngram-match-state-followup-fixes branch April 6, 2026 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant