Repository navigation
grammar: clone finds each stack element's rule by binary search - #336
Open
professorpalmer wants to merge 1 commit into
Open
professorpalmer wants to merge 1 commit into
professorpalmer wants to merge 1 commit into
Conversation
llama_grammar_clone_impl redirected each stack element to the copied rules with a search over every element of every rule, O(stack elements x rule elements) for each clone. The server clones the sampler, and with it the grammar, on each speculative step, so with a large tool-call grammar this cost is in every decode step. Now each element's rule is found by a binary search on the rule start addresses; the copy itself does not change. Server tool-call grammar for 107 tools (4704 rules, 43790 elements, 323 stack elements): one clone 8.4 ms -> 0.40 ms (CPU). test-grammar-integration: clone at each position of an input, check that the clone's stacks point into its own rules and that the clone matches the rest of the input. Reported-by: Milor123 (professorpalmer/bonsai-ada-surgery#7)
Author
|
Confirmed by the user who reported it (professorpalmer/bonsai-ada-surgery#7), RTX 4070, Pi agent with 107 MCP tools, same chat at 64-71k: 45.9 -> 63.3 tok/s with this change; time per draft step 44.8 -> 35.7 ms. |
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.
Overview
llama_grammar_clone_implpoints each stack element of the copy at the copied rules. It found each element with a search over every element of every rule: O(stack elements x rule elements) for each clone. The server clones the sampler, and with it the grammar, on each speculative step (common_sampler_clonebeforecommon_sampler_sample_and_accept_ninserver-context.cpp). With a tool list, the tool-call grammar is large, so this cost is in every decode step.This change finds each element's rule with a binary search on the rule start addresses (
std::lesson the pointers, then the offset inside the rule). The copy itself does not change.Additional information
test-grammar-integrationgets a clone check. It clones at each position of a JSON-schema input, frees the original, checks that every stack element of the clone points into the clone's own rules, and checks that the clone matches the rest of the input. Negative control: with the redirect removed, the test fails at the pointer check.test-grammar-parser,test-llama-grammarandtest-grammar-integrationpass (static CPU build, Windows, MSVC).src/llama-grammar.cppand the same per-step clone in the server.Requirements