Repository navigation
perf(api-server): stop keeping finished sequences, keep prompt rendering and tokenizing off the event loop #2912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
77212d9
bd367f7
c4ce7e5
3036898
a7b1b7c
b7387b9
6ca4ec8
80aea67
991517c
bf3f38a
25cb433
beefb96
9786106
5cfc14e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,7 @@ def add_cli_args(cls, parser): | |
| def from_cli_args(cls, args): | ||
| return cls() | ||
|
|
||
| def create_engine(self, tokenizer=None): | ||
| def create_engine(self, tokenizer=None, **kwargs): | ||
| return None | ||
|
|
||
| _try_import_else_stub( | ||
|
|
@@ -876,6 +876,85 @@ async def drive(): | |
| assert events == ["initialize", "serving", "shutdown"] | ||
|
|
||
|
|
||
| class _FakeSeq: | ||
| def __init__(self, seq_id, token_ids): | ||
| self.id = seq_id | ||
| self.prompt_token_ids = list(token_ids) | ||
| self.num_prompt_tokens = len(token_ids) | ||
|
|
||
|
|
||
| def test_prompt_is_checked_and_truncated_in_the_preprocess_call(monkeypatch): | ||
| # /v1/messages and /v1/responses: no encode on the event loop. The length | ||
| # check reads the engine's own token count from the one preprocess call, | ||
| # and only an over-long prompt is built again, from its first ids. | ||
| requests = {} | ||
| monkeypatch.setattr( | ||
| api_server, | ||
| "engine", | ||
| SimpleNamespace(io_processor=SimpleNamespace(requests=requests)), | ||
| ) | ||
| monkeypatch.setattr( | ||
| api_server, | ||
| "tokenizer", | ||
| SimpleNamespace(decode=lambda ids, **_kw: f"<{len(ids)} ids>"), | ||
| ) | ||
| built = [] | ||
|
|
||
| def preprocess(prompt): | ||
| tokens = list(range(10)) if isinstance(prompt, str) else prompt | ||
| seq = _FakeSeq(len(built), tokens) | ||
| built.append(prompt) | ||
| requests[seq.id] = seq | ||
| return seq | ||
|
|
||
| cap = api_server._PromptCap(16) | ||
| seq = api_server._preprocess_prompt(preprocess, "short", cap) | ||
| assert built == ["short"] and seq.num_prompt_tokens == 10 | ||
| assert cap.cut_text is None | ||
| seq = api_server._preprocess_prompt(preprocess, "short", None) | ||
| assert seq.num_prompt_tokens == 10 | ||
|
|
||
| built.clear() | ||
| requests.clear() | ||
| cap = api_server._PromptCap(4) | ||
| seq = api_server._preprocess_prompt(preprocess, "long prompt", cap) | ||
| assert built == ["long prompt", [0, 1, 2, 3]] | ||
| assert seq.prompt_token_ids == [0, 1, 2, 3] | ||
| # The text the engine runs, for the reasoning channel's seed. | ||
| assert cap.cut_text == "<4 ids>" | ||
| # The discarded first build is not left registered. | ||
| assert list(requests) == [seq.id] | ||
|
|
||
|
|
||
| def test_responses_channel_follows_the_text_the_engine_runs(monkeypatch): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this doesn't drive responses_create, so deleting the call stays green.. can we add a structured case over the cap?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added @valar-cop re-review please. |
||
| # Structured: cut before the engine call (off the loop), so the channel | ||
| # the sampling params are built from reads the kept text. Unstructured: | ||
| # re-seeded from `cut_text` after the preprocess call cut it. | ||
| monkeypatch.setattr( | ||
| api_server, | ||
| "tokenizer", | ||
| SimpleNamespace( | ||
| encode=lambda text, **_kw: list(range(len(text))), | ||
| decode=lambda ids, **_kw: "x" * len(ids), | ||
| ), | ||
| ) | ||
| cap = api_server._PromptCap(4) | ||
| assert api_server._cut_prompt("abc", cap) == ("abc", "abc") | ||
| assert api_server._cut_prompt("abcdefgh", cap) == ([0, 1, 2, 3], "xxxx") | ||
|
|
||
| seeds = [] | ||
| monkeypatch.setattr( | ||
| api_server, "prompt_starts_in_reasoning", lambda text: seeds.append(text) | ||
| ) | ||
| monkeypatch.setattr( | ||
| api_server, "reasoning_channel", lambda start, **_kw: ("reseeded", start) | ||
| ) | ||
| assert api_server._reseed_after_cut("orig", cap, {}) == "orig" | ||
| cap.cut_text = "kept" | ||
| assert api_server._reseed_after_cut("orig", cap, {})[0] == "reseeded" | ||
| assert seeds == ["kept"] | ||
|
|
||
|
|
||
| class TestDeploymentDefaultOutputCap: | ||
| @pytest.mark.parametrize("explicit", [False, True]) | ||
| def test_small_deployment_distinguishes_default_from_explicit( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pr description still describes the old design, please update it to match the diff..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated after this review: Summary/Changes now describe
Config.keep_finished_outputsviacreate_engine,asyncio.to_thread, and the check/truncation in the preprocess call. Validation lists the current unit test run.