Remove boxed timeout future allocation#815
Open
simbiont666 wants to merge 3 commits intocloudflare:mainfrom
Open
Remove boxed timeout future allocation#815simbiont666 wants to merge 3 commits intocloudflare:mainfrom
simbiont666 wants to merge 3 commits intocloudflare:mainfrom
Conversation
Use concrete future types in ToTimeout/Timeout instead of Box<dyn Future>
Collaborator
|
Without having looked into too much detail yet, we were wondering if you've ran some basic benchmarks to show improvements if any. |
Author
|
The original timeout benchmarks hit the immediately-ready path, so they did not capture the case where the wrapped future is polled as Pending at least once. I added one to cover that path so.. timeout-noalloc-future
pingora timeout pending-once 20.444917ms total, 204ns avg per iteration
pingora timeout pending-once 9.817333ms total, 98ns avg per iteration
pingora timeout pending-once 9.63175ms total, 96ns avg per iteration
pingora timeout pending-once 9.457833ms total, 94ns avg per iteration
pingora timeout pending-once 9.575208ms total, 95ns avg per iteration
tokio timeout pending-once 9.119375ms total, 91ns avg per iteration
tokio timeout pending-once 6.196458ms total, 61ns avg per iteration
tokio timeout pending-once 5.382125ms total, 53ns avg per iteration
tokio timeout pending-once 5.568625ms total, 55ns avg per iteration
tokio timeout pending-once 5.797833ms total, 57ns avg per iteration
BASELINE (9ac75d0)
pingora timeout pending-once 22.12925ms total, 221ns avg per iteration
pingora timeout pending-once 10.712041ms total, 107ns avg per iteration
pingora timeout pending-once 10.881959ms total, 108ns avg per iteration
pingora timeout pending-once 10.841833ms total, 108ns avg per iteration
pingora timeout pending-once 10.953334ms total, 109ns avg per iteration
tokio timeout pending-once 8.404417ms total, 84ns avg per iteration
tokio timeout pending-once 6.20375ms total, 62ns avg per iteration
tokio timeout pending-once 5.321458ms total, 53ns avg per iteration
tokio timeout pending-once 5.06025ms total, 50ns avg per iteration
tokio timeout pending-once 6.092583ms total, 60ns avg per iterationI’m still new to this project, so I can’t confidently estimate how frequent this “at least one pending poll” path is in production traffic yet. But even though the tokio variant is still faster overall, fast timeout became about ~10% faster. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi!
This PR removes heap allocation in pingora-timeout by replacing boxed timeout futures with concrete future types.
Behavior stays the same.