fix(openai-compat): reuse cached engine instances in _resolve_engine (#1614)
* fix(openai-compat): reuse cached engine instances in _resolve_engine
The direct engine-ID path in /v1/audio/speech constructed a fresh
backend per request (return cls()). For SubprocessBackend engines that
meant: a new sidecar process, a full torch import and an engine model
reload on EVERY request (measured ~28s floor per pockettts request on
an M3 Pro), plus another atexit hook registration each time — exactly
what get_engine_instance_for()'s docstring warns against.
Route the explicit-ID path through the same cached-singleton seam the
active-engine path already uses. Unknown/unavailable IDs keep their
400s; tts-1/tts-1-hd and the OmniVoiceBackend special case are
unchanged.
* fix(openai-compat): unload the outgoing engine on explicit-ID switches
Review follow-up (Greptile/CodeRabbit on #1614): caching instances without
a switch rule would let each distinct explicit engine ID stay resident,
accumulating sidecars / multi-GB in-process models. Mirror
get_active_tts_backend's MM2-01 switch rule: a different explicit ID
(omnivoice included, which resolves to the active engine) unloads the
outgoing instance first, best-effort.
* fix(openai-compat): evict via the shared single-engine-resident seam, not a router-local cache
The explicit-ID unload cache (13c14e2c) kept its own instance ref keyed by
model id. The shared engine cache is deliberately keyed by CLASS (registry
rebinds, idle sweeps and engine_memory eviction all mutate it), so the
router's id-keyed ref could go stale and keep serving an instance the
lifecycle system no longer tracked — caught by
test_openai_speech_toggle_off_sends_raw_text in full-suite order, and it
also introduced a novel unload path that ignored the
OMNIVOICE_SINGLE_ENGINE_RESIDENT opt-out.
Drop the router-local cache entirely: _resolve_engine returns the shared
cached singleton (get_engine_instance_for), and create_speech calls
evict_other_tts_engines(backend.id) before warming the engine — the exact
seam /generate uses. That covers every transition (explicit id → explicit
id, explicit id → tts-1/omnivoice aliases), honors the policy opt-out, and
leaves no per-router state to drift. Regression pinned at the route level in
test_speech_request_evicts_other_resident_engines.
* chore(changelog): trim the #1614 entry to the one-liner limit
415 chars against the 400 the style test allows — CI would have failed on it.
---------
Co-authored-by: debpalash <4178343+debpalash@users.noreply.github.com>
This commit is contained in:
co-authored by
debpalash
parent
2d37627ab2
commit
3223a20f88
@@ -33,6 +33,7 @@ the frozen-backend fallback mirror it for their toolchains.
|
||||
- The OmniVoice guide now covers combining style attributes with a reference clip (consistent instruct stabilizes cloning; the reference wins conflicts), inline pronunciation control (pinyin / CMU phonemes), and corrects the claim that the default engine can't do voice design — it can, from attributes (#1565)
|
||||
|
||||
### Fixed
|
||||
- The OpenAI-compatible `/v1/audio/speech` route now reuses the shared cached engine for explicit `model` ids instead of constructing a fresh engine — and its sidecar/model load, a ~28s floor per call for subprocess engines — on every request, with the same single-engine-resident discipline `/generate` applies (#1614) — thanks @paoloantinori!
|
||||
- The setup wizard's RAM check no longer blocks 8 GB machines whose OS reports ~7.8 GB usable — the thresholds now tolerate reserved memory, and `OMNIVOICE_RAM_PREFLIGHT=0` turns a genuine block into a warning for those who accept the OOM risk (#1618)
|
||||
- Invisible watermarking now runs eagerly instead of through `torch.compile` — AudioSeal's lazy compile sent the first embed of every session into Inductor's C++ codegen, which failed outright on macOS hosts whose toolchain couldn't serve it and shipped the audio unmarked after a 30-40s wait; first embed drops from 9.70s to 0.26s (#1615) — thanks @paoloantinori!
|
||||
- The macOS Accessibility blocker now rechecks while visible and closes as soon as the grant is enabled instead of keeping a stale permission prompt on screen (#1609)
|
||||
|
||||
@@ -160,7 +160,9 @@ _OPENAI_VOICE_ALIASES = {
|
||||
|
||||
def _resolve_engine(model_id: str):
|
||||
"""Map an OpenAI model name to a VoiceStudio backend."""
|
||||
from services.tts_backend import get_backend_class, get_active_tts_backend
|
||||
from services.tts_backend import (
|
||||
get_backend_class, get_active_tts_backend, get_engine_instance_for,
|
||||
)
|
||||
|
||||
# Accept OpenAI model names as pass-through to the active engine.
|
||||
if model_id in ("tts-1", "tts-1-hd"):
|
||||
@@ -177,8 +179,18 @@ def _resolve_engine(model_id: str):
|
||||
)
|
||||
from services.tts_backend import OmniVoiceBackend
|
||||
if cls is OmniVoiceBackend:
|
||||
# OmniVoice only ever runs as the shared active engine — the
|
||||
# explicit-omnivoice request is the active-engine request.
|
||||
return get_active_tts_backend()
|
||||
return cls()
|
||||
# Cached singleton, not a fresh cls(): SubprocessBackend engines would
|
||||
# spawn a sidecar process and reload their model on EVERY request, and
|
||||
# register a new atexit hook each time (get_engine_instance's contract).
|
||||
# No router-local cache on top of it: the shared cache is keyed by
|
||||
# CLASS precisely so id rebinds/evictions can't serve a stale instance,
|
||||
# and cross-engine memory discipline is create_speech's
|
||||
# evict_other_tts_engines call (the same seam /generate uses) — not a
|
||||
# bespoke unload here.
|
||||
return get_engine_instance_for(model_id)
|
||||
except ValueError:
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
@@ -388,6 +400,15 @@ async def create_speech(req: SpeechRequest):
|
||||
# VRAM eviction runs in get_model()'s warm-return path now, covering every
|
||||
# native TTS generate (this route, WS TTS, dub, batch, audiobook).
|
||||
|
||||
# Single-active-engine memory discipline (MM2-01), the same call /generate
|
||||
# makes before its load: hand back every OTHER resident TTS engine's model
|
||||
# before this one warms up, so switching `model` ids across requests —
|
||||
# explicit id → explicit id, or explicit id → the tts-1/omnivoice aliases —
|
||||
# can't stack multi-GB engines/sidecars. No-op when nothing else is
|
||||
# resident; opt out with OMNIVOICE_SINGLE_ENGINE_RESIDENT=0.
|
||||
from services.engine_memory import evict_other_tts_engines
|
||||
await evict_other_tts_engines(backend.id)
|
||||
|
||||
# ── #1033/#1037/#1014: warm the engine under the LOAD budget before the
|
||||
# generate clock starts. The T4 verification (#1014) measured a fresh
|
||||
# install's first /v1/audio/speech burning its whole 300s generate budget
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
"""_resolve_engine must reuse engine instances across /v1/audio/speech requests.
|
||||
|
||||
Regression: the direct engine-ID path did ``return cls()`` per request, so every
|
||||
``model: "pockettts"`` (or any SubprocessBackend engine) spawned a fresh sidecar
|
||||
process, re-imported torch and reloaded the engine's model — a ~28s floor per
|
||||
request on real hardware — and registered another atexit hook each time. The
|
||||
cached-singleton seam (``get_engine_instance_for``) exists precisely for this;
|
||||
the route just wasn't using it for explicit engine IDs (only tts-1/tts-1-hd got
|
||||
the shared active-engine instance).
|
||||
|
||||
The flip side of caching is accumulation: cached explicit engines must not
|
||||
stack multi-GB residents when requests switch ``model`` ids. That is NOT a
|
||||
router-local unload cache (an id-keyed instance ref goes stale against the
|
||||
class-keyed shared cache — registry rebinds, idle sweeps) — the route calls
|
||||
``evict_other_tts_engines`` before warming the engine, the exact seam
|
||||
/generate uses (single-engine-resident policy, MM2-01), pinned here at the
|
||||
route level.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
os.environ.setdefault("OMNIVOICE_MODEL", "test")
|
||||
os.environ.setdefault("OMNIVOICE_DISABLE_FILE_LOG", "1")
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
def _tts_mod():
|
||||
import importlib
|
||||
|
||||
return importlib.import_module("services.tts_backend")
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def oc():
|
||||
import importlib
|
||||
|
||||
return importlib.import_module("api.routers.openai_compat")
|
||||
|
||||
|
||||
def test_explicit_engine_id_resolves_to_the_cached_singleton(oc, monkeypatch):
|
||||
svc = _tts_mod()
|
||||
|
||||
class _FakeBackend:
|
||||
instances = 0
|
||||
|
||||
def __init__(self):
|
||||
_FakeBackend.instances += 1
|
||||
|
||||
@staticmethod
|
||||
def is_available():
|
||||
return True, "ok"
|
||||
|
||||
monkeypatch.setattr(svc, "get_backend_class", lambda _id: _FakeBackend)
|
||||
|
||||
first = oc._resolve_engine("pockettts")
|
||||
second = oc._resolve_engine("pockettts")
|
||||
assert first is second
|
||||
# Exactly one construction across both resolves: the cached singleton did
|
||||
# the work, not a fresh cls() per request.
|
||||
assert _FakeBackend.instances == 1
|
||||
|
||||
|
||||
def test_unknown_engine_id_still_400s(oc, monkeypatch):
|
||||
from fastapi import HTTPException
|
||||
|
||||
svc = _tts_mod()
|
||||
|
||||
def _unknown(_id):
|
||||
raise ValueError("no such engine")
|
||||
|
||||
monkeypatch.setattr(svc, "get_backend_class", _unknown)
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
oc._resolve_engine("not-an-engine")
|
||||
assert exc.value.status_code == 400
|
||||
assert "Unknown model" in exc.value.detail
|
||||
|
||||
|
||||
# ── Cross-request memory discipline ─────────────────────────────────────────
|
||||
|
||||
|
||||
def _make_engine(tb, eid: str):
|
||||
"""A registry-real fake engine (same harness shape as
|
||||
tests/test_text_normalization_routes.py) that counts its unloads."""
|
||||
import torch
|
||||
|
||||
class _E(tb.TTSBackend):
|
||||
id = eid
|
||||
display_name = f"{eid} (test)"
|
||||
supports_cloning = True
|
||||
gpu_compat = ("cpu",)
|
||||
unloads = 0
|
||||
|
||||
@property
|
||||
def sample_rate(self) -> int:
|
||||
return 24000
|
||||
|
||||
@property
|
||||
def supported_languages(self) -> list[str]:
|
||||
return ["multi"]
|
||||
|
||||
@classmethod
|
||||
def is_available(cls):
|
||||
return True, "ready"
|
||||
|
||||
def generate(self, text, **kw) -> torch.Tensor:
|
||||
return torch.zeros(1, 24000)
|
||||
|
||||
def unload(self):
|
||||
type(self).unloads += 1
|
||||
|
||||
return _E
|
||||
|
||||
|
||||
def test_speech_request_evicts_other_resident_engines(monkeypatch):
|
||||
"""Switching explicit `model` ids across requests must hand back the
|
||||
outgoing engine's model (single-engine-resident policy) — the cached
|
||||
singletons cannot accumulate residents."""
|
||||
svc = _tts_mod()
|
||||
a = _make_engine(svc, "fake-cache-a")
|
||||
b = _make_engine(svc, "fake-cache-b")
|
||||
monkeypatch.setitem(svc._REGISTRY, "fake-cache-a", a)
|
||||
monkeypatch.setitem(svc._REGISTRY, "fake-cache-b", b)
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
from main import app
|
||||
|
||||
client = TestClient(app, client=("127.0.0.1", 50000))
|
||||
|
||||
r1 = client.post("/v1/audio/speech", json={
|
||||
"model": "fake-cache-a", "input": "hi", "response_format": "wav",
|
||||
})
|
||||
assert r1.status_code == 200, r1.text
|
||||
assert a.unloads == 0 # the engine that just ran stays warm
|
||||
|
||||
r2 = client.post("/v1/audio/speech", json={
|
||||
"model": "fake-cache-b", "input": "hi", "response_format": "wav",
|
||||
})
|
||||
assert r2.status_code == 200, r2.text
|
||||
assert a.unloads == 1 # outgoing engine handed its model back
|
||||
assert b.unloads == 0 # incoming engine untouched
|
||||
Reference in New Issue
Block a user