Drop the mock protocol from the multipart benchmarks - #13515
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13515 +/- ##
=======================================
Coverage 99.02% 99.02%
=======================================
Files 133 133
Lines 49924 49927 +3
Branches 2594 2594
=======================================
+ Hits 49439 49442 +3
Misses 366 366
Partials 119 119
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Confidence Score: 5/5The PR appears safe to merge because the benchmark protocol stub supports all flow-control interactions reached by these pre-buffered streams. The change is confined to benchmark setup, and the replacement protocol provides the connected state and pause/resume methods used by StreamReader without altering user-facing multipart behavior. Reviews (1): Last reviewed commit: "Drop the mock protocol from the multipar..." | Re-trigger Greptile |
Merging this PR will improve performance by 18.63%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_read_base64_part |
18.5 ms | 15.5 ms | +19.23% |
| ⚡ | test_read_chunk_base64_realignment |
7.5 ms | 6.4 ms | +18.04% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing multipart-bench-protocol (8b209ee) with master (2d9c5c1)
Footnotes
-
83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Backport to 3.15: 💚 backport PR created✅ Backport PR branch: Backported as #13518 🤖 @patchback |
Backport to 3.14: 💚 backport PR created✅ Backport PR branch: Backported as #13519 🤖 @patchback |
What do these changes do?
The multipart benchmarks built their
StreamReaderon amock.Mockprotocol, so every chunk read went through a mockresume_reading()call; that was about a third of the measured time and it is sensitive to the runner environment, which showed up as a spurious 10% regression on #13514. Use aBaseProtocolsubclass with no-op flow control instead, as the websocket benchmarks already do.Are there changes in behavior for the user?
No, benchmarks only.
Is it a substantial burden for the maintainers to support this?
No.
Related issue number
Follow up to #13509, noise seen on #13514.
Checklist
CONTRIBUTORS.txtN/ACHANGES/folder N/Aname it
<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature: A new behavior, public APIs. That sort of stuff..deprecation: A declaration of future API removals and breakingchanges in behavior.
.breaking: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc: Notable updates to the documentation structure or buildprocess.
.packaging: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.
Drafted with Claude Fable 5; reviewed by @bdraco.