perf(routing): look up models by selector and memoize resolved-model strings - #829
perf(routing): look up models by selector and memoize resolved-model strings#829SantiagoDePolonia wants to merge 1 commit into
Conversation
…strings The hot path built the qualified "provider/model" string ~12 times per request only to have the registry split it apart again. The registry now answers Supports/GetProvider/GetProviderType/GetProviderName for an already-parsed selector directly (an optional fast path the router asserts once at construction), and the six duplicated string-lookup bodies collapse into one shared helper. RequestModelResolution memoizes its two derived selector strings, computed once at resolution time instead of on every audit-enrich, rate-limit and cache read. Also: QualifyModelWithProvider no longer allocates for its prefix check, and the snapshot middleware canonicalizes the brand-cased user-path header key once instead of on every Header.Get. Production-shape hot path: 157 -> 145 allocs/op, routed 99 -> 90; ceilings lowered to match.
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Confidence Score: 5/5Safe to merge based on the focused routing and registry behavior checks. There are no final findings. The focused validation passed for selector lookup parity and the router's selector-capable and string-only fallback behaviors. Files Needing Attention: None. The exercised changes in internal/providers/registry.go and internal/providers/router.go preserved expected resolution behavior. Reviews (1): Last reviewed commit: "perf(routing): look up models by selecto..." | Re-trigger Greptile |
2 participants
Third round of hot-path allocation work (follows #792/#793). Profiling showed
ModelSelector.QualifiedModel()as the single largest remaining allocation site: ~12 heap allocations per request spent building the qualifiedprovider/modelstring, which the registry then immediately split apart again.Changes
GetProvider,GetModel,LookupModel,Supports,GetProviderType,GetProviderName) collapse into one sharedmodelInfoLockedhelper, and new*ForSelectorvariants answer the same queries for an already-parsed selector without materializing the qualified string (it is only built on the rare second-chance paths, e.g. slash-shaped model IDs). A parity test pins both paths to identical answers across selector shapes.qualifiedSelectorResolver) and uses it everywhere it previously calledr.lookup.X(selector.QualifiedModel()). Lookups that only speak qualified strings (test fakes) keep working through the fallback.resolvedModelForAuditLog, nowResolvedRouteModel()on the resolution) — computed once when the gateway builds the resolution instead of on every audit-enrich pass, rate-limit read and cache read. Getters fall back to computing when the cache wasn't filled, so literal-constructed resolutions behave as before.QualifyModelWithProviderno longer allocates for its prefix check; the snapshot middleware canonicalizes the brand-casedX-GoModel-User-Pathkey once at construction instead of on everyHeader.Get(wire format and error messages unchanged).Impact
No user-visible behavior change; request/response shapes, audit log contents and the public string-based lookup APIs are unchanged.
CI alloc ceilings lowered to match.