Studio backend performance: five superlinear paths in the routes and data layer - #8499
Studio backend performance: five superlinear paths in the routes and data layer#8499danielhanchen wants to merge 9 commits into
Conversation
_gguf_read_array_value read, UTF-8 decoded and retained every entry of tokenizer.ggml.tokens before handing the list to delimiter_shaped_tokens, which keeps only "<...>" and "[...]" shapes. A vocabulary is 128k to 256k entries and a few hundred survive. Read the length and the first byte, then seek past the body when that byte is neither "<" nor "[". UTF-8 is self-synchronising, so no multi-byte character begins with either, and the shape filter provably rejects everything else. A 256k-token header parses in 44.3 ms instead of 82.4 ms.
_resolve_missing_tool_call_ids walked the history backwards from every tool result and rescanned each assistant's tool_calls, so one assistant with n calls followed by n results was O(n^2). At 500 results that was 13.9 ms of request validation, at 2000 it was 216 ms. The backward walk only ever looked inside the current user-delimited segment, so one forward pass with an index answers the same question. Explicit ids resolve through a per-segment map of the newest assistant call per id. Missing ids draw from a stack of the assistants that still have an unclaimed call, each holding its remaining call indexes in order plus the same indexes bucketed by function name, so the name-match-else-first-remaining rule keeps its exact order. Requests where every tool result already carries its id now return before either pass, since the first pass only ever fed the second. 500 results: 13.9 ms to 1.1 ms with explicit ids, 12.9 ms to 1.6 ms without.
_truncate_middle_messages copied the middle groups and dropped them with pop(0), which shifts every remaining element, so evicting k of n groups cost O(n*k). A cursor over the existing slice is enough and needs no copy. 32k messages truncate in 89.6 ms instead of 1.157 s.
Diffusion dataset upload validated each incoming filename by scanning every filename already accepted in the batch: once for an exact duplicate, once for a sidecar-stem collision, and twice more for a case variant. That is O(N^2) over a batch, and Starlette accepts up to 1000 files in one multipart request. Three indexes rebuilt per request answer all of it with hash lookups. The per-casefold and per-stem entries keep insertion order, so each error message still names the same earlier file the linear scans named. A 1000-file batch validates in 39.0 ms instead of 1.185 s.
check_url_access re-normalises the whole policy for every URL it checks, and a restricted web search checks up to 20 results against lists of up to 100 domains each, lowercasing, IP-parsing and IDNA-encoding every one of them each time. normalize_domain is a pure function of its argument, so an all-str domain list memoises safely on an immutable tuple. Every public call still builds fresh dicts and lists, and a list that is not exactly all str stays on the original path so its error message keeps the original repr. 20 URLs against a 100 plus 100 domain policy: 0.69 ms instead of 28.6 ms.
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75ea099df1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| return ascii_domain | ||
|
|
||
|
|
||
| @lru_cache(maxsize = 256) |
There was a problem hiding this comment.
Bound raw domain strings before caching
An authenticated caller to /api/chat/research-runs can supply valid domains with arbitrarily large raw representations: Python's IDNA codec removes characters such as U+00AD, so "a" + "\u00ad" * N + ".com" normalizes to a.com and passes the 253-byte normalized-length check. This cache retains the original tuple as its key, allowing up to 256 large request-derived strings to remain resident and making repeated otherwise-valid policies capable of exhausting the backend's memory; reject/cache-bypass oversized raw inputs or avoid retaining the raw tuple.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in fc77b0b. "a" + "\u00ad" * 100000 + ".com" does normalise to a.com here, so the raw string was becoming an lru_cache key and staying resident for the life of the process.
The cache is now skipped for any raw entry longer than the maximum normalised domain (253), so those normalise on the uncached path. Rejecting them would have been a behaviour change, and this keeps the output byte-identical: the differential corpus for this file still reports RESULTS IDENTICAL against the pre-branch tree, including the padded cases and their ValueError messages, and the 100x100 policy benchmark is unchanged at 0.75 ms versus 26.8 ms before the branch. Regression test in test_oversized_raw_domains_normalize_without_entering_the_cache.
Nameprep deletes characters such as U+00AD, so an arbitrarily long raw string can normalise to a short valid domain and pass the 253-byte check. The memo key is the caller's raw tuple, so a request-sized string would stay resident for the life of the process. Entries longer than the maximum normalised domain now take the uncached path, which normalises identically.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
|
Ran this on a staging copy rather than adding to the org queue. Green: Unsloth API CI, Windows API, GGUF (Linux/Mac/Windows), Windows UI, Startup profile, Local Agent Guides. Two reds, neither this branch: Core. Mac Studio UI + API + Update CI. A Playwright The differential harnesses are the real check here, and all five report identical results and identical log text against a pristine base tree. |
One of a series of Studio backend performance PRs. This one is the routes and
data layer: five paths that were superlinear in the size of their input, plus
one that decoded a whole vocabulary in order to throw it away.
Every change is a pure algorithmic rewrite of an existing rule. No behaviour,
no ordering, no error type, no error text and no log line changes.
Before
core/inference/llama_cpp.pytool_calls, so n calls followed by n results was O(n^2)models/inference.pypop(0), shifting every remaining element per droproutes/inference.pyroutes/training.pycheck_url_accessre-normalised the entire website policy for every URL, IDNA-encoding up to 200 domains each timecore/inference/web_access_policy.pyAfter
Each one keeps the identical rule and changes only how it is looked up.
that byte is neither
<nor[. The shape filter accepts nothing else, andUTF-8 is self-synchronising so no multi-byte character starts with either byte.
user-delimited segment, so a single forward pass with an index answers the same
question. Explicit ids use a per-segment map of the newest assistant call per
id. Missing ids draw from a stack of assistants that still have an unclaimed
call, each holding its remaining indexes in order plus the same indexes
bucketed by function name, preserving the name-match-else-first-remaining
order exactly. Requests whose tool results all carry ids now return before
either pass, since the first pass only ever fed the second.
each error message still names the same earlier file the linear scans named.
normalize_domainis a pure function of its argument, so anall-
strdomain list memoises on an immutable tuple. Every public call stillreturns fresh dicts and lists.
Measured on this machine, minimum of repeated timings, CPU only:
The upload figures stop at 999 files because Starlette caps one multipart
request at
max_files=1000, so that is the largest batch this route can receive.Controls, so the small common case is not paying for the large one. A request
validation costs 3 to 37 microseconds here:
The one case that gets slower is a short history whose tool results are missing
their ids: building the index costs about 1.5 us more than the naive scan does at
n=4. That is the only regression I measured, and it is the same path that saves
205 ms at 2000 results.
How output equivalence was verified
Not by inspection. For each change I ran a differential harness that drives the
real code over a wide corpus, once against a pristine copy of
origin/mainandonce against this branch, and compares the recorded results byte for byte. The
emitted log text is compared the same way, with timestamps stripped, so a
changed or missing log line fails the check too. Every corpus includes empty,
unicode, malformed, oversized and adversarial inputs, and every harness applies
each operation twice to confirm idempotency is unchanged.
Test suite,
python3 -m pytest tests/ -q -p no:randomlyinstudio/backend:14c6ce31d: 343 failed, 21585 passed, 85 errorsstrict subset of the base one.
The 161 that differ are cross-file interference, not a fix: they are GPU, VRAM
and tensor-parallel tests unrelated to these five files, and
test_metal_paravirtual_guard.py,test_mtp_vram_budget.pyandtest_tensor_parallel.pyall pass 397/397 in isolation on both the base and thisbranch. The remaining failures and all 85 collection errors are environmental on
this host: no GPU, and
fastmcp,pymupdfandsqlite_vecare absent.The 13 test files that cover these paths directly give the identical result on
both trees: 1075 passed, and the same single pre-existing failure
(
test_every_shipped_video_family_resolves_on_this_diffusers, a diffusersversion check).
Considered and dropped
load_inference_configon the/api/inference/statuspoll. Theendpoint reparses YAML on every request, which I measured at 3.575 ms per call,
or roughly 43 ms of CPU per minute per open client. Dropped: the function logs
a line per call, so caching it would silently remove log output, and it reads
from disk on every call, so memoising it asserts an invariance that nothing in
the code guarantees. The saving does not justify either.
hand-rolling a second JSON serializer beside the
ChatCompletionChunkmodel.Byte equality holds today, but nothing keeps the two in step, and no test
patches that seam, so a future field on the model would diverge silently.
identity check between the raw and emitted line to decide which path is safe,
and it changes which branch the monitor takes for every relayed chunk.
json.loadsfallback on lines starting withdata:. Changesan exception-driven control path for a per-line saving.
stream_deltasmode togenerate_chat_completion. Changes thecontract of a public generator and rewires three streaming routes. Too large a
blast radius for this PR.