fix(lib): memoize catalog pricing/capability lookups to fix cold /v1/models freeze (#8697) - #8987
Conversation
cddc0c0 to
7d5f40b
Compare
…models freeze (diegosouzapw#8697) Root cause: a cold GET /v1/models catalog rebuild froze the entire server 41-54s. node --prof profiling found a systemic missing-memoization pattern — a per-model function rescanning a static or synced data structure with Object.entries()/ Object.keys() (or hitting SQLite) on every call instead of once per rebuild. Fixed 6 instances of the same pattern, found by iteratively re-profiling the full catalog sweep after each fix (plus a whitebox review pass) until no further hotspot of this shape remained: 1. getModelsDevPricing() (modelsDevSync.ts) — re-ran a synchronous SQLite query and re-JSON.parse'd ~180 blobs on every call (up to ~6091x instead of once per request). Memoized via the existing modelCatalogCacheVersion invalidation signal (same pattern as getCachedRawProviderConnections/getCachedProviderNodes in db/readCache.ts). Dominant cost of the original 41-54s freeze. 2. findInsensitive() (modelMetadataRegistry.ts, resolveCatalogPricing) — rebuilt a full Object.entries() scan on every case-insensitive lookup miss, twice per model. Replaced with a lowercase-key index built once per distinct pricing object and cached by identity (WeakMap). Warns once at index-build time on a case-insensitive key collision instead of silently discarding the second value. 3. getSyncedCapability() (modelsDevSync.ts) — ran a per-model SQLite SELECT on cold cache instead of self-warming the whole-table cache; no caller in the /v1/models build path ever primed it, so a cold rebuild ran one SQLite round-trip per model per call site. Now self-warms via the existing bulk getSyncedCapabilities() on first miss. Measured as the dominant remaining cost after fixes 1-2 (~70% of a full catalog sweep). 4. getCanonicalModelSpecId() (shared/constants/modelSpecs.ts) — up to 3 separate linear scans over the static MODEL_SPECS table per call (exact ci, alias ci, prefix). Replaced with a lazy, lowercase-key index built once (MODEL_SPECS never changes at runtime); prefix-match iteration order preserved exactly so resolution outcomes are unchanged. 5. getStaticSpecCanonicalModelId() (modelCapabilities.ts) — duplicated the same exact+alias scan as (4) in a second, separate rescan. Now reuses the shared index via a new exported helper (findModelSpecIdByExactOrAlias) instead of maintaining a second cache over the same static table. reverseModelsDevProviders() (modelCapabilities.ts) — rescanned Object.entries(MODELS_DEV_PROVIDER_MAP) (also static) on every call; memoized by provider key. Result is frozen (readonly) since it is now shared across calls instead of freshly allocated each time. 6. resolveModelAlias() (shared/constants/modelSpecs.ts) — rescanned Object.entries(MODEL_SPECS) unconditionally once per model (verified 1:1 call ratio, no short-circuit). Case-sensitive exact match (Array.includes(), no .toLowerCase()) — uses a dedicated exact-match index, deliberately not the case-insensitive alias index from fix 4/5 (would silently broaden matches). Measured on a 1940-pair real-catalog sample (static PROVIDER_MODELS registry): cold sweep 828ms -> 356ms after fixes 3-5 on top of 1-2, extrapolating to roughly 1s on the real ~6091-model catalog, down from the original 41-54s freeze. Complementary to the stale-serve fix in diegosouzapw#8801 (upstream) — neither alone eliminates the freeze. Tests: call-count regression guards for every fix (DB prepare / Object.entries / Object.keys call counts staying constant instead of scaling with iteration count), plus correctness coverage for case-insensitive/case-sensitive resolution. All pre-existing consumer suites re-verified passing (96 tests total across 19 files).
7d5f40b to
35db149
Compare
Runtime validationBuilt this branch (webpack fallback — Turbopack's native memory isn't bounded by Measured on the same instance, same method, immediately before/after the deploy:
Response validated as a real, complete catalog (6079 models), not a fast error. PROC-0 sanity check green (404/405 on bogus ids), clean boot, no silent exceptions in the application log. The real-world gain (~11x) is smaller than the synthetic sweep extrapolation in the PR description (~37-54x) — expected, since the synthetic benchmark only exercised the static registry lookup path in a tight loop, while the real endpoint also does live provider-connection resolution and full response formatting that this PR doesn't touch. Still a decisive improvement over the original 41-54s freeze. |
|
Two days running with this patch and zero freeze, no side effect that I can tell. |
|
Merged via local merge-train on 192.168.0.113 (32 cores) @ train tip Green:
For reference, |
e50f232
into
diegosouzapw:release/v3.8.50
…models freeze (diegosouzapw#8697) (diegosouzapw#8987) Root cause: a cold GET /v1/models catalog rebuild froze the entire server 41-54s. node --prof profiling found a systemic missing-memoization pattern — a per-model function rescanning a static or synced data structure with Object.entries()/ Object.keys() (or hitting SQLite) on every call instead of once per rebuild. Fixed 6 instances of the same pattern, found by iteratively re-profiling the full catalog sweep after each fix (plus a whitebox review pass) until no further hotspot of this shape remained: 1. getModelsDevPricing() (modelsDevSync.ts) — re-ran a synchronous SQLite query and re-JSON.parse'd ~180 blobs on every call (up to ~6091x instead of once per request). Memoized via the existing modelCatalogCacheVersion invalidation signal (same pattern as getCachedRawProviderConnections/getCachedProviderNodes in db/readCache.ts). Dominant cost of the original 41-54s freeze. 2. findInsensitive() (modelMetadataRegistry.ts, resolveCatalogPricing) — rebuilt a full Object.entries() scan on every case-insensitive lookup miss, twice per model. Replaced with a lowercase-key index built once per distinct pricing object and cached by identity (WeakMap). Warns once at index-build time on a case-insensitive key collision instead of silently discarding the second value. 3. getSyncedCapability() (modelsDevSync.ts) — ran a per-model SQLite SELECT on cold cache instead of self-warming the whole-table cache; no caller in the /v1/models build path ever primed it, so a cold rebuild ran one SQLite round-trip per model per call site. Now self-warms via the existing bulk getSyncedCapabilities() on first miss. Measured as the dominant remaining cost after fixes 1-2 (~70% of a full catalog sweep). 4. getCanonicalModelSpecId() (shared/constants/modelSpecs.ts) — up to 3 separate linear scans over the static MODEL_SPECS table per call (exact ci, alias ci, prefix). Replaced with a lazy, lowercase-key index built once (MODEL_SPECS never changes at runtime); prefix-match iteration order preserved exactly so resolution outcomes are unchanged. 5. getStaticSpecCanonicalModelId() (modelCapabilities.ts) — duplicated the same exact+alias scan as (4) in a second, separate rescan. Now reuses the shared index via a new exported helper (findModelSpecIdByExactOrAlias) instead of maintaining a second cache over the same static table. reverseModelsDevProviders() (modelCapabilities.ts) — rescanned Object.entries(MODELS_DEV_PROVIDER_MAP) (also static) on every call; memoized by provider key. Result is frozen (readonly) since it is now shared across calls instead of freshly allocated each time. 6. resolveModelAlias() (shared/constants/modelSpecs.ts) — rescanned Object.entries(MODEL_SPECS) unconditionally once per model (verified 1:1 call ratio, no short-circuit). Case-sensitive exact match (Array.includes(), no .toLowerCase()) — uses a dedicated exact-match index, deliberately not the case-insensitive alias index from fix 4/5 (would silently broaden matches). Measured on a 1940-pair real-catalog sample (static PROVIDER_MODELS registry): cold sweep 828ms -> 356ms after fixes 3-5 on top of 1-2, extrapolating to roughly 1s on the real ~6091-model catalog, down from the original 41-54s freeze. Complementary to the stale-serve fix in diegosouzapw#8801 (upstream) — neither alone eliminates the freeze. Tests: call-count regression guards for every fix (DB prepare / Object.entries / Object.keys call counts staying constant instead of scaling with iteration count), plus correctness coverage for case-insensitive/case-sensitive resolution. All pre-existing consumer suites re-verified passing (96 tests total across 19 files). Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
This fixes a server-wide freeze, not a slow endpoint. While a cold
/v1/modelscatalog rebuild runs, the whole Node event loop is blocked synchronously — every other in-flight request stalls too (dashboard, other API keys, health checks), for the full duration. Reporter measured ~15s on a 9-API-key Docker deployment (#8697); profiling on a larger local catalog measured 41-54s. On any instance with the models.dev sync enabled (default-on catalog resolution path), the first/v1/modelscall after a cold start or after the ~60s catalog memo TTL expires pays this cost, and it recurs roughly every minute per distinct API key (#8219 capped the memo TTL at 60s; the memo is also keyed per-API-key per #6440, so multi-key deployments pay it multiple times over).Root cause & how we got here
This is a nice example of a cost that's essentially invisible in isolation and only shows up once a project has grown a lot. A handful of small, individually reasonable helpers — case-insensitive lookups, reverse-alias maps — were added over the last few months as the model catalog and its feature set kept expanding (
MODEL_SPECSalone grew from 5 entries to 56, and models.dev itself started at "109 providers, 4,146+ models"). Each one is cheap on its own; none of them were ever slow in a unit test with a handful of fixture models. It's only once the real catalog reached thousands of entries that the combined per-model cost of these lookups became visible as a multi-second stall instead of noise — nothing here is a classic regression (nothing that used to be fast got slower), just a cost curve that only bends upward once you're operating at real scale.This also matches the reporter's own note on #8697 — "Subjectively this was much faster on 3.8.48 — I have not bisected" — there's no single commit to point to; it's a threshold crossed gradually as the project (happily) kept growing.
Summary
GET /v1/modelscatalog rebuild freezing the entire server for 41-54s (issue fix(api): cold GET /v1/models blocks ~15s per API key — memo should serve last snapshot + async refresh #8697) by fixing 6 instances of the same missing-memoization pattern — a per-model function rescanning a static or synced data structure (Object.entries()/Object.keys(), or a raw SQLite query) on every call instead of once per rebuild:getModelsDevPricing()(src/lib/modelsDevSync.ts) — re-ran a synchronous SQLite query and re-JSON.parse'd ~180 blobs on every call (up to ~6091x instead of once per request). Memoized via the existingmodelCatalogCacheVersioninvalidation signal (same pattern asgetCachedRawProviderConnections/getCachedProviderNodesindb/readCache.ts). Dominant cost of the original 41-54s freeze.findInsensitive()(src/lib/modelMetadataRegistry.ts,resolveCatalogPricing) — rebuilt a fullObject.entries()scan on every case-insensitive lookup miss, twice per model. Replaced with a lowercase-key index built once per distinct pricing object and cached by identity (WeakMap); warns once at index-build time if two keys collide case-insensitively instead of silently discarding the second value.getSyncedCapability()(src/lib/modelsDevSync.ts) — ran a per-model SQLiteSELECTon a cold cache instead of self-warming the whole-table cache; no caller in the/v1/modelsbuild path ever primed it, so a cold rebuild ran one SQLite round-trip per model per call site. Now self-warms via the existing bulkgetSyncedCapabilities()on first miss. ~70% of a full catalog sweep's remaining cost after fixes 1-2.getCanonicalModelSpecId()(src/shared/constants/modelSpecs.ts) — up to 3 separate linear scans over the staticMODEL_SPECStable per call (exact case-insensitive, alias case-insensitive, prefix). Replaced with a lazy lowercase-key index built once (MODEL_SPECSnever changes at runtime); prefix-match iteration order preserved exactly so resolution outcomes are unchanged.getStaticSpecCanonicalModelId()(src/lib/modelCapabilities.ts) — duplicated the same exact+alias scan as (4) in a second, independent rescan. Now reuses the shared index via a new exported helper (findModelSpecIdByExactOrAlias) instead of maintaining a second cache over the same static table. Also fixedreverseModelsDevProviders()in the same file — rescannedObject.entries(MODELS_DEV_PROVIDER_MAP)(also static) on every call; memoized by provider key, result frozen (readonly string[]) since it is now shared across calls instead of freshly allocated each time.resolveModelAlias()(src/shared/constants/modelSpecs.ts) — rescannedObject.entries(MODEL_SPECS)unconditionally once per model. Case-sensitive exact match (Array.includes(), no.toLowerCase()) — uses a dedicated exact-match index rather than the case-insensitive alias index from fix 4/5, which would have silently broadened matches and changed behavior.PROVIDER_MODELSregistry): cold sweep 828ms → 356ms after fixes 3-6 on top of 1-2, extrapolating to roughly ~1s on the real ~6091-model catalog — down from the original 41-54s freeze.Related Issues
Validation
node --import tsx/esm --test tests/unit/*8697*.test.ts tests/integration/modelsDevSync.test.tsnpm run lint(clean on touched files)npm run typecheck:core(clean)Tests Added Or Updated
tests/unit/models-dev-pricing-memoization-8697.test.ts(new) —getModelsDevPricing()hits the DB at most once across repeated reads within the same cache version; returns fresh data immediately after a write invalidates the cache.tests/unit/catalog-pricing-lookup-index-8697.test.ts(new) — case-insensitive pricing resolution stays correct;Object.entries()call count stays constant (not scaling with iteration count) across repeated catalog-scale lookups.tests/unit/synced-capability-warmup-8697.test.ts(new) —getSyncedCapability()runs at most 2db.prepare()calls (bulk load + table guard) across 200 distinct model lookups, not one round-trip per lookup.tests/unit/model-spec-lookup-index-8697.test.ts(new) —getCanonicalModelSpecId()stays correct for case-insensitive/unknown ids; 0Object.entries()/Object.keys()calls across 500 repeated lookups once the index is warm.tests/unit/reverse-models-dev-providers-8697.test.ts(new) — indirect correctness viagetResolvedModelCapabilities(); 0 scans ofMODELS_DEV_PROVIDER_MAPspecifically across 300 repeated calls for the same provider.tests/unit/resolve-model-alias-index-8697.test.ts(new) —resolveModelAlias()stays correct (exact alias resolves, case-varied input does NOT resolve — preserving case-sensitive semantics, unknown input passes through unchanged); 0Object.entries()calls across 500 repeated lookups once the index is warm.catalog-pricing-surface-8018,model-capabilities-specialty-8016,effort-thinking-standardization-6241,modelsDevSync-extended,xiaomi-mimo-provider,reasoning-token-buffer-6274,t31-t33-t34-t38-model-specs,combo-thinking-disabled-fable5-3554,minimax-m3-maxtokens,kimi-k2.7-code-registration,openai-gpt56-catalog,model-capabilities-mistral-vision-sync-4073,modelsdev-specialty-keying-8017, plus themodelsDevSyncintegration suite — 96 tests total across 19 files, all passing.Coverage Notes
src/lib/modelMetadataRegistry.ts,src/lib/modelsDevSync.ts,src/lib/modelCapabilities.ts,src/shared/constants/modelSpecs.ts. Every changed function is exercised end-to-end by the new regression tests plus the pre-existing suites listed above; no coverage regression expected — pure memoization/indexing added around already-covered logic, no new branches beyond cache-hit/cache-miss paths the tests exercise.Reviewer Notes
getModelsDevPricing()'s only other caller (src/app/api/settings/models-dev/route.ts) is read-only, so memoizing the returned reference is safe (no mutation risk).findInsensitive()'sWeakMapindex is invalidated implicitly:getModelsDevPricing()returns a brand-new object on real cache misses, so the old object (and its index entry) becomes unreachable and garbage-collected.getSyncedCapability()'s self-warm reusesgetSyncedCapabilities()'s existing write-path invalidation (saveModelsDevCapabilities/clearModelsDevCapabilitiesboth already setcachedCapabilitiesLoadedAll), so no new invalidation logic was needed.getCanonicalModelSpecId()/getStaticSpecCanonicalModelId()/reverseModelsDevProviders()/resolveModelAlias()index static, module-level constants (MODEL_SPECS,MODELS_DEV_PROVIDER_MAP) that are never mutated at runtime, so none of their caches need any invalidation at all.resolveModelAlias()deliberately does NOT reuse the case-insensitivealiasCiindex built for fixes 4/5 — it has always been case-sensitive (Array.includes()), and reusing the ci index would silently broaden which inputs resolve. A dedicated case-sensitivealiasExactmap was added togetModelSpecIndex()instead.reverseModelsDevProviders()'s return type changed fromstring[]toreadonly string[](its single call site only spreads the result into aSet, never mutates it) — the result is nowObject.freeze()d since it is shared across calls via the cache instead of freshly allocated each time.