Studio: serialize non-streaming responses once and pool the proxy client#6393
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces performance optimizations for non-streaming inference requests. It implements a shared, pooled httpx.AsyncClient for non-streaming proxy calls to the local llama-server, replacing per-request client creation and ensuring proper cleanup during application shutdown. Additionally, it introduces a helper function _model_json_response to serialize Pydantic models directly to JSON, avoiding the overhead of dictionary round-trips and double serialization in Starlette's JSONResponse. Unit tests are also added to verify the serialization behavior. I have no feedback to provide as there are no review comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Two safe latency wins on the OpenAI/Anthropic-compatible endpoints that leave the streaming generation paths untouched (they keep Connection: close and max_keepalive_connections=0 so a client disconnect still stops GPU decode). 1. Non-streaming responses used JSONResponse(content=model.model_dump()), which builds a dict and then re-runs json.dumps. Serialize once with model.model_dump_json() via a small _model_json_response helper. The body is byte-identical (nulls preserved), about 3x faster to encode in a microbench. 2. The non-streaming completions and embeddings proxies built a fresh httpx.AsyncClient per request. Route them through one pooled client (core/inference/llama_http) closed on shutdown; streaming generation keeps its own per-request close-only client. About 5x faster per call to the local llama-server in a microbench. The existing API-monitor tests for the non-streaming completions, embeddings and passthrough paths now patch nonstreaming_client instead of httpx.AsyncClient to match the pooled client, so they stay deterministic.
danielhanchen
force-pushed
the
studio-api-perf-serialization-pooling
branch
from
53f3a1b to
c0158f4
Compare
for more information, see https://pre-commit.ci
Review follow-up on the shared httpx client. It was a single module-global instance, which has two lifecycle problems the per-request client did not: 1. After aclose() in lifespan shutdown, nonstreaming_client() kept handing back the closed client, so a second lifespan in the same process (repeated TestClient, embedded restart) failed with "client has been closed". 2. An httpx client binds its transport to the loop it first runs on, so reuse from another loop could raise "Event loop is closed". Hold one client per running loop in a WeakKeyDictionary, recreate when missing or closed, and close all on shutdown. Single-loop production is unchanged.
for more information, see https://pre-commit.ci
|
Thanks for the review. Pushed a follow-up in 7aa8995. The shared client is now held per running event loop in a |
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request introduces performance optimizations for non-streaming inference requests. It implements a pooled, event-loop-scoped httpx.AsyncClient utility to reuse connections for non-streaming calls to the local llama-server, replacing per-request client instantiation. Additionally, it introduces a _model_json_response helper to serialize Pydantic responses directly using model_dump_json(), avoiding the overhead of double serialization in Starlette's JSONResponse. Corresponding unit tests and test mocks have been updated to support these changes. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Two safe latency wins on the OpenAI/Anthropic-compatible endpoints. Both leave the streaming generation paths untouched, which keep
Connection: closeandmax_keepalive_connections=0so a client disconnect still tears down the upstream socket and stops GPU decode (#5749).Changes
Non-streaming responses used
JSONResponse(content=model.model_dump()), which builds a dict and then re-runsjson.dumps. They now serialize once withmodel.model_dump_json()via a small_model_json_responsehelper. The body is byte-identical (nulls preserved).The non-streaming completions and embeddings proxies built a fresh
httpx.AsyncClientper request. They now use one pooled client (core/inference/llama_http) closed on shutdown. Streaming generation keeps its own per-request close-only client.Benchmark (local, indicative)
Testing
studio/backend/tests/test_api_perf_serialization.py:_model_json_responsebody decodes identically to the oldJSONResponse(model_dump())(nulls kept), correct media type and status./v1/completionsreturn valid responses through the new serialization and the pooled client.