⚡ Bolt: optimize global pricing lookup performance #302
Conversation
Co-authored-by: Laisky <4532436+Laisky@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis pull request refactors the pricing configuration retrieval mechanism by replacing direct map indexing with dedicated function calls. The changes introduce Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. |
There was a problem hiding this comment.
Pull request overview
Optimizes hot-path pricing resolution in One-API by avoiding full global pricing map cloning on per-request lookups, switching to targeted single-model retrieval via GetGlobalModelConfig.
Changes:
- Updated three-layer pricing helpers to use
GetGlobalModelConfiginstead of cloning the entire global pricing map. - Updated image relay billing to use
GetGlobalModelConfigfor global image pricing resolution. - Added an internal Bolt note documenting the “avoid map cloning in hot path” learning.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| relay/pricing/global.go | Replaces full-map global pricing retrieval with single-model config lookups in three-layer pricing helpers. |
| relay/controller/image.go | Switches global image pricing fallback to use GetGlobalModelConfig during image quota computation. |
| .jules/bolt.md | Documents performance guidance about avoiding large map clones in request hot paths. |
| imagePriceUsd := 0.0 | ||
| if resolvedConfig.Image != nil { | ||
| imagePriceUsd = resolvedConfig.Image.PricePerImageUsd | ||
| } | ||
| if imagePriceUsd == 0 { | ||
| if pm, ok := pricing.GetGlobalModelPricing()[imageModel]; ok && pm.Image != nil { | ||
| if pm, ok := pricing.GetGlobalModelConfig(imageModel); ok && pm.Image != nil { | ||
| imagePriceUsd = pm.Image.PricePerImageUsd | ||
| } | ||
| } |
There was a problem hiding this comment.
imagePriceUsd fallback only checks global pricing when resolvedConfig.Image is nil/zero, but resolvedConfig comes from ResolveModelConfig, which returns the channel model config as soon as it exists (even if it has no Image metadata). This means provider default per-image pricing can be skipped entirely whenever a channel has a model entry without Image pricing, leading to incorrect billing (per-token path instead of per-image). Consider resolving image pricing with the same 3-layer precedence (channel → provider defaults → global) and using that resolved image pricing consistently for defaults/validation and imagePriceUsd.
| imagePriceUsd := 0.0 | ||
| if resolvedConfig.Image != nil { | ||
| imagePriceUsd = resolvedConfig.Image.PricePerImageUsd | ||
| } | ||
| if imagePriceUsd == 0 { | ||
| if pm, ok := pricing.GetGlobalModelPricing()[imageModel]; ok && pm.Image != nil { | ||
| if pm, ok := pricing.GetGlobalModelConfig(imageModel); ok && pm.Image != nil { | ||
| imagePriceUsd = pm.Image.PricePerImageUsd | ||
| } | ||
| } |
There was a problem hiding this comment.
There’s no regression test covering the pricing-precedence edge case here (channel has a ModelConfig entry without Image metadata, provider defaults define Image.PricePerImageUsd). Adding a unit test in the existing image billing/controller test suite for this scenario would help prevent future under/over-charging regressions.
Laisky
deleted the
bolt-optimize-global-pricing-lookup-5915013610921063642
branch
1 participant
💡 What:
relay/pricing/global.goto useGetGlobalModelConfiginGetModelRatioWithThreeLayers,GetCompletionRatioWithThreeLayers, andGetVideoPricingWithThreeLayers.relay/controller/image.goto useGetGlobalModelConfigfor image pricing resolution.🎯 Why:
GetGlobalModelPricing()clones the entire global pricing map and all its entries (potentially thousands of models) on every call. In a high-throughput gateway, this causes massive CPU and memory allocation overhead for every request, as it was being called twice or more per request.📊 Impact:
🔬 Measurement:
Verified via
go testandgo vet. Performance improvement is architectural and avoids a known O(N) bottleneck in a hot path.PR created automatically by Jules for task 5915013610921063642 started by @Laisky
Summary by CodeRabbit
Release Notes
Performance Improvements
Documentation