test(benchmarks): run shared logging executor inline to make CodSpeed measurements deterministic - #32435
Conversation
… measurements deterministic
|
|
Greptile SummaryThis PR fixes flaky CodSpeed benchmark measurements by ensuring that work submitted to litellm's shared
Confidence Score: 5/5Safe to merge — changes are confined to the test benchmarks directory and do not touch any production code. The implementation is correct: _submit_inline faithfully mirrors ThreadPoolExecutor._WorkItem.run() (same except BaseException pattern, same future state transitions), the Python 3.10+ type annotations (ParamSpec, collections.abc.Callable) match the project's requires-python >= 3.10 floor, the del executor.submit teardown correctly restores the class method via normal attribute lookup, and the guard test reliably detects fixture removal by asserting both immediate completion and calling-thread identity. No files require special attention.
|
| Filename | Overview |
|---|---|
| tests/benchmarks/conftest.py | New session-scoped autouse fixture that replaces ThreadPoolExecutor.submit with an inline implementation to make CodSpeed callgrind measurements hermetic; implementation is correct and well-documented. |
| tests/benchmarks/test_benchmarks.py | Adds a guard benchmark (test_logging_executor_runs_inline) that verifies the patched executor runs inline and on the calling thread; the assertion is reliable and will catch fixture removal. |
Reviews (1): Last reviewed commit: "test(benchmarks): run shared logging exe..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Merging this PR will not alter performance
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes |
60729f7
into
litellm_internal_staging
… measurements deterministic (BerriAI#32435)
3 participants
Relevant issues
Linear ticket
Resolves LIT-4260
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Before (unfixed, observed on production CI over a 12h window ending 2026-07-08 06:20 UTC): the CodSpeed Performance Analysis check flagged false regressions on 10 of ~40 PR heads, always in the three completion benchmarks of
tests/benchmarks/test_inference_benchmarks.pyand never in the other 27 benchmarks. The measurements are bimodal, with the same ~1.0-1.6 ms cost roaming between benchmarks run to run. For example, at PR head661db9a(compared against staging6df5e1b) the report readtest_completion_multi_turn3.1 ms -> 4.2 ms (-25.9%),test_completion_simple_message3.8 ms -> 4.7 ms (-17.96%),test_completion_with_tools4.2 ms -> 3.2 ms (+30.98%); at PR headdb24027the same benchmarks flipped the other way. BASE values for unchanged staging code also flap (test_completion_simple_messagewas variously 3.2, 3.8, 4.1, 4.6 and 4.8 ms across the window), proving the instability is benchmark-internal rather than caused by any PR's diffAfter (this PR, head
8da870f): the CodSpeed workflow was run twice on the same commit (run 28923307313 and its rerun) and the reports agree on every benchmark.test_completion_multi_turnmeasured 3.8 ms in both runs andtest_completion_simple_message3.9 ms in both, with the reported efficiency deltas differing by only 0.06 percentage points between runs (-17.65% vs -17.71%, +16.75% vs +16.77%); before the fix, two runs of the same code flipped by 25 to 47 points. The new values also confirm the mechanism: they equal the old fast mode (3.1/3.2 ms) plus exactly one success handler, meaning each measurement now contains precisely its own workNote for reviewers: the CodSpeed check on this PR reads as a small regression by design, for two reasons that both resolve on merge. The BASE it compares against is staging's still-flapping measurement, and the handler cost that was previously attributed to a random benchmark is now deterministically included, shifting the completion benchmarks up by ~0.7 ms once. The shift should be acknowledged on the CodSpeed dashboard; the property that matters, reruns of the same commit agreeing, is demonstrated above
Type
✅ Test
🚄 Infrastructure
Changes
The three completion benchmarks call
litellm.completion(..., mock_response=...), and each such call submits its success handler (cost calculation, response copying) to the sharedThreadPoolExecutorinlitellm/litellm_core_utils/thread_pool_executor.py(submitted from the sync wrapper inlitellm/utils.py). pytest-codspeed's simulation mode runs one warmup call and one measured call, toggling callgrind instrumentation around the measured call only. Callgrind counts instructions from every thread while the window is open, and valgrind serializes all threads onto one virtual CPU, so the queued handler work lands inside whichever benchmark's measured window the valgrind scheduler happens to resume it under. GC andrandomare already neutralized by pytest-codspeed itself (gc.collect(); gc.disable()andrandom.seed(0)around every measurement), which is why the flakiness only ever appeared in the benchmarks that enqueue background workThis PR adds
tests/benchmarks/conftest.pywith a session-scoped autouse fixture that replaces the shared executor'ssubmitwith an inline implementation for the benchmark session, so every measurement contains exactly its own work. This is deterministic and also more representative of the true per-request cost. A guard benchmark,test_logging_executor_runs_inline, asserts that submissions through the shared executor complete inline on the calling thread; it fails immediately if the fixture is removed (verified by running it with the conftest moved aside)