server: resolve the model contract once per model, not per request - #328
Conversation
…F per request build_speech_request called model_accepts_request_option() on every POST /v1/audio/speech. That resolves the model contract through model_contract(family), which reads the embedded spec with gguf_init_from_file() on the full GGUF. The resolution cache is thread_local and serve_http handles each request on a fresh detached thread, so the cache is cold every time and each request re-opens and re-parses the whole model file. With a 298 MB GGUF this adds a fixed ~900 ms to every request, independent of backend, text length and audio duration. Resolve the option once per model instead: - LoadedModel gains accepts_reference_text, defaulting to true (the existing permissive behaviour for models with no contract). - refresh_model_option_flags() sets it in make_model and again in handle_model_load, so reconfiguration stays correct. - build_speech_request reads the cached flag. A contract that fails to resolve is caught at registration, reported on stderr, and falls back to the permissive default rather than throwing per request. Measured on Windows x64 / Vulkan with supertonic-3-f16.gguf, same server.json and request: ttfb 0.956-1.153 s before, 0.045-0.049 s after. Response bytes are unchanged (Content-Length 114090, identical MD5 before and after).
|
@XythQ Thanks, this is a useful improvement! One small issue before merge: the new fallback swallows real model-spec errors. With an invalid spec override, startup logs the real error but continues permissively: Then the request fails later with an unrelated error: A better approach may be to remove the broad catch and let invalid specs fail during registration or load. |
Review feedback: the catch added in the previous commit swallowed real model-spec errors. An invalid spec override logged the real reason, started anyway, and then failed the request with an unrelated "model path does not exist". model_accepts_request_option already returns true for a model with no contract (it swallows only the missing-contract errors and rethrows the rest), so the catch could only ever extend permissiveness to genuine misconfigurations. It is removed rather than narrowed: nothing replaces it. An invalid spec now fails once, at registration, with the real error, instead of 500ing every request as it did before this branch.
|
Removed rather than narrowed — One consequence: a configured-but-absent override file now fails loudly. Intentional, but I didn't build to reproduce your case — the log line traces statically to |
|
@XythQ Thanks! PR merged 🚀 |
Problem
ServerState::build_speech_requestcallsmodel_accepts_request_option()on everyPOST /v1/audio/speech. That resolves the model contract throughengine::model_spec::model_contract(family), which reads the model's embedded spec withgguf_init_from_file()on the full GGUF.The resolution cache is
thread_local(src/framework/model_spec/package.cpp:22-26), andserve_httphandles each request on a fresh detached thread, so the cache is cold every time. Eachrequest re-opens and re-parses the whole model file.
With a 298 MB GGUF this adds a fixed ~900 ms to every request, independent of backend, text
length and audio duration. It is invisible in
X-AudioCPP-Wall-Msbecauserun_modelstarts itsclock after
build_speech_request, and it does not affectaudiocpp_cli, which resolves the speconce on a single thread.
Fix
Resolve the option once per model instead of once per request:
LoadedModelgainsbool accepts_reference_text(defaults totrue, the existing permissivebehaviour for models with no contract).
refresh_model_option_flags()sets it inmake_model, and again inhandle_model_loadwhen amodel is reconfigured, so dynamic model management stays correct.
build_speech_requestreads the cached flag.A contract that fails to resolve is caught at registration, logged to
stderr, and falls back tothe permissive default — same behaviour as before, but reported once instead of throwing per
request.
The one-off resolution cost moves to registration, where the model load already performs a
gguf_init_from_file.Measurements
v0.7.0, Windows x64, Vulkan backend,supertonic-3-f16.gguf(298 MB), sameserver.json, samerequest:
X-AudioCPP-Wall-MsAlso reproduced before the fix on
--backend cpu(ttfb 1.268–1.414 s against 341–362 ms reported),confirming it is not backend-specific.
Response bytes are unchanged: the same request returns
Content-Length114090 with an identicalMD5 (
1514cf1fcf6bfc4e865a1cc24d5b8948) before and after.Notes
reference_textlookup, the only contract option the speech path uses today.Any other per-request contract lookup added later would hit the same cold
thread_localpath, soit may be worth resolving the contract once per model more generally.