perf: request-path hot-path optimizations — Tier 3–4 (#149)

[amondnet]

Summary

Tier 3–4 of the request-path hot-path performance audit (#149). Four sub-issues, one commit each:

• perf: avoid holding the global accounts Mutex across the O(N) assessment (accounts.rs) #156 (Tier 3) — AccountPool::select_order_inner no longer holds the global entries Mutex across the O(N) quota assessment. The lock now covers only entry insertion, stale-quota expiry, and a minimal per-account state clone; assess_quota / governing_weekly_reset (pure computation) run after the lock is released. Multi-account failover ordering is unchanged.
• perf: reduce per-selection allocations in select_order / is_fable_model (accounts.rs) #159 (Tier 4) — is_fable_model is computed once per selection instead of ~2N times (removing per-account model lowercasing), threaded into the quota helpers as a precomputed bool. Provider-string construction is hoisted out of the per-account loop.
• perf: skip the per-account refresh lock when the token is still valid (adapters/responses/pool.rs) #158 (Tier 3) — resolve_or_cooldown no longer takes the per-account refresh lock for still-valid tokens. CodexAuthStore::get_valid_chatgpt already single-flights refreshes at the auth layer (perf: add single-flight to CodexAuthStore refresh to dedupe concurrent refreshes (auth/codex/auth.rs) #157 / perf(codex): single-flight CodexAuthStore refresh #168), so valid-token resolution now proceeds concurrently, while the expired-token refresh + atomic writeback stays serialized by the auth-layer path-keyed guard. The force-refresh (401 failover) path keeps its per-account lock (conservative scope).
• perf: reuse per-frame timeout timers and keepalive boundary rotation (codex_ws, keepalive) #160 (Tier 4) — run_turn reuses one pinned tokio::time::Sleep for the idle timeout instead of registering a fresh timer per frame; BoundaryTracker::push inspects at most the trailing 4 bytes of a chunk instead of rotating per byte.

Notes

• The codex_ws select! polls the frame source before the idle timer (biased) to preserve the inner-future-first semantics of the tokio::time::timeout it replaces — a frame arriving as the deadline elapses is delivered, not dropped in favor of the timeout.

Testing

• cargo fmt --all --check — clean
• cargo clippy --all-targets --all-features -- -D warnings — clean
• cargo test --all-features --workspace — green (645 lib tests + all integration suites)
• Regression tests added: valid-token resolution is not serialized by the pool refresh lock (perf: skip the per-account refresh lock when the token is still valid (adapters/responses/pool.rs) #158); long-chunk boundary tracking ending in \r\n\r\n (perf: reuse per-frame timeout timers and keepalive boundary rotation (codex_ws, keepalive) #160).

Docs

Internal performance refactors only — no change to observable behavior, config keys, endpoints, CLI, or provider/model semantics, so no README.md / docs/ / site/ update is required (per AGENTS.md).

Closes #156
Closes #158
Closes #159
Closes #160
Part of #149

---

Summary by cubic

Speeds up the request path by reducing pool lock scope and per-account work, allowing concurrent credential resolution for valid ChatGPT tokens, and trimming Codex WS idle-timer overhead. No behavior changes — just faster hot paths.

• Refactors

  • Account selection: hold the entries mutex only for insertion/expiry/snapshot; run quota assessment and weekly reset after unlock. Compute is_fable_model once per selection; provider string hoisted. QuotaState is Clone; assess_quota/governing_weekly_reset take a precomputed bool.
  • Auth: resolve_or_cooldown skips the per-account refresh lock for valid tokens; expired-token refresh remains single-flighted in CodexAuthStore::get_valid_chatgpt. Tests ensure valid-token resolution doesn’t wait.
  • WS: run_turn reuses a pinned tokio::time::Sleep and uses biased select so frames at the deadline are delivered. BoundaryTracker::push bulk-copies only the trailing 4 bytes. Tests cover idle-timeout error surfacing and long-chunk/empty/3-byte boundary cases.

• Bug Fixes

  • Removed a flaky idle-timer reset test; kept the reliable timeout-after-silence coverage.

^{Written for commit 3c8cb57. Summary will update on new commits.}

[gemini-code-assist]

Code Review

This pull request optimizes account credential resolution by removing the per-account refresh lock, relying instead on the auth store's internal single-flighting. It also refactors quota assessment to pass QuotaState directly, optimizes boundary tracking in keepalive streams, and improves websocket idle timeout handling using a pinned sleep timer with a biased select. The reviewer suggested a medium-severity optimization in src/accounts.rs to avoid redundant quota evaluations and heap allocations for duplicate account aliases by querying only unique representatives.

[codecov]

Codecov Report

❌ Patch coverage is 98.89503% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/adapters/responses/codex_ws.rs	96.36%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[codspeed-hq]

Merging this PR will not alter performance

✅ 11 untouched benchmarks

---

_{Comparing amondnet/perf-2 (3c8cb57) with main (2039c27)}

[greptile-apps]

Greptile Summary

This PR delivers Tier 3–4 hot-path optimizations to the request path: locking scope in AccountPool::select_order_inner is narrowed so quota assessment runs after the entries mutex is released (on a cloned snapshot), is_fable_model and the provider string are computed once per selection, resolve_or_cooldown drops its per-account pool lock for valid tokens, and the Codex WS run_turn reuses a single pinned Sleep via a biased select! while BoundaryTracker::push is rewritten to bulk-copy only the trailing 4 bytes.

• Mutex scope reduction (perf: avoid holding the global accounts Mutex across the O(N) assessment (accounts.rs) #156, perf: reduce per-selection allocations in select_order / is_fable_model (accounts.rs) #159): QuotaState derives Clone; expire_stale_quota still runs inside the lock, but assess_quota and governing_weekly_reset operate on the clone after the lock is dropped. is_fable_model is hoisted out of the per-account loop, and provider.to_string() is computed once before the round-robin counter lookup.
• Lock-free valid-token path (perf: skip the per-account refresh lock when the token is still valid (adapters/responses/pool.rs) #158): resolve_or_cooldown removes the _guard = refresh_lock.lock().await call; CodexAuthStore::get_valid_chatgpt continues to single-flight expired-token refreshes internally via a path-keyed guard, and the force-refresh (401) path still holds its per-account lock.
• Timer and boundary optimizations (perf: reuse per-frame timeout timers and keepalive boundary rotation (codex_ws, keepalive) #160): A single tokio::pin!(idle) replaces a fresh tokio::time::timeout per frame; biased polling preserves frame-over-timeout delivery semantics. BoundaryTracker::push is reduced from O(chunk_len) byte-by-byte rotations to two O(1) bulk copies of at most 4 bytes.

Confidence Score: 5/5

All four hot-path changes are safe to merge: the quota-assessment refactor operates on a well-formed clone taken after expiry, the lock removal in resolve_or_cooldown is backed by auth-layer single-flighting and a direct regression test, the pinned idle timer preserves frame-over-deadline delivery via biased polling, and BoundaryTracker's bulk-copy path is verified correct for every sub-tail length (0, 1, 2, 3, ≥4) by the new unit tests.

The quota assessment now runs on a cloned QuotaState after the entries mutex is released — expire_stale_quota still mutates under the lock, so the clone captures post-expiry state and the pure computation outside the lock is safe. The lock removal from resolve_or_cooldown is correct because CodexAuthStore::get_valid_chatgpt serializes refreshes internally per credential path; the new test validates that a held pool lock doesn't block valid-token resolution. The biased select! + pinned Sleep is a drop-in replacement for tokio::time::timeout with stronger frame-wins-at-deadline semantics. BoundaryTracker::push was verified manually and by tests for all retained lengths including the overlap case handled correctly by copy_within's memmove semantics.

No files require special attention.

Important Files Changed

Filename	Overview
src/accounts.rs	Quota assessment lifted out of the entries mutex via QuotaState clone; is_fable_model and provider string hoisted once per selection. Logic is equivalent and correctness is maintained by the clone-then-assess pattern.
src/adapters/responses/pool.rs	Per-account pool refresh lock removed from resolve_or_cooldown; valid-token path is now lock-free and a regression test verifies it doesn't block on the held pool lock. force_refresh_or_cooldown retains its per-account lock.
src/adapters/responses/codex_ws.rs	Idle timer changed from a per-frame tokio::time::timeout to a single pinned Sleep with biased select!; reset happens unconditionally before each frame wait. New test covers the silence-to-timeout path with paused time and real loopback I/O.
src/keepalive.rs	BoundaryTracker::push rewritten from O(N) rotate-per-byte to O(1) bulk copy of trailing 4 bytes. Empty-chunk early return prevents a no-op copy_within. New tests cover long chunk, empty push, and 3-byte split edge cases.
src/adapters/responses/inbound.rs	Comment-only update to forward_codex_passthrough reflecting the lock-removal in resolve_or_cooldown.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant Pool as AccountPool
    participant Lock as entries Mutex
    participant Quota as assess_quota (pure)

    Note over Caller,Quota: Before (all under lock)
    Caller->>Lock: acquire
    Lock->>Pool: expire_stale_quota
    Lock->>Quota: assess_quota (holding lock)
    Lock->>Quota: governing_weekly_reset (holding lock)
    Lock-->>Caller: release

    Note over Caller,Quota: After (quota assessment outside lock)
    Caller->>Lock: acquire
    Lock->>Pool: expire_stale_quota
    Lock->>Pool: quota.clone()
    Lock-->>Caller: release (lock dropped earlier)
    Caller->>Quota: assess_quota(cloned_quota)
    Caller->>Quota: governing_weekly_reset(cloned_quota)

    Note over Caller,Quota: resolve_or_cooldown valid token path (Before)
    Caller->>Pool: refresh_lock.lock().await
    Pool->>Pool: resolve_chatgpt_account
    Pool-->>Caller: Credential (lock released)

    Note over Caller,Quota: resolve_or_cooldown valid token path (After)
    Caller->>Pool: resolve_chatgpt_account (no pool lock)
    Pool-->>Caller: Credential (concurrent safe)

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant Pool as AccountPool
    participant Lock as entries Mutex
    participant Quota as assess_quota (pure)

    Note over Caller,Quota: Before (all under lock)
    Caller->>Lock: acquire
    Lock->>Pool: expire_stale_quota
    Lock->>Quota: assess_quota (holding lock)
    Lock->>Quota: governing_weekly_reset (holding lock)
    Lock-->>Caller: release

    Note over Caller,Quota: After (quota assessment outside lock)
    Caller->>Lock: acquire
    Lock->>Pool: expire_stale_quota
    Lock->>Pool: quota.clone()
    Lock-->>Caller: release (lock dropped earlier)
    Caller->>Quota: assess_quota(cloned_quota)
    Caller->>Quota: governing_weekly_reset(cloned_quota)

    Note over Caller,Quota: resolve_or_cooldown valid token path (Before)
    Caller->>Pool: refresh_lock.lock().await
    Pool->>Pool: resolve_chatgpt_account
    Pool-->>Caller: Credential (lock released)

    Note over Caller,Quota: resolve_or_cooldown valid token path (After)
    Caller->>Pool: resolve_chatgpt_account (no pool lock)
    Pool-->>Caller: Credential (concurrent safe)

Loading

_{Reviews (4): Last reviewed commit: "test(perf): remove flaky idle-timer rese..." | Re-trigger Greptile}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud