-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ws: Prep work for trailers support #4130
Conversation
This puts the caller in charge of triggering the workspace rollback before moving pipelined bytes around. This will open the door to pipelining for req and beresp trailers. The workspace emulator version of WS_ReqPipeline() had a systematic rollback instead of a conditional one. The reason why this didn't pose a problem was that fetch tasks never need pipelining and always take the shortcut where the rollback was properly optional. This is a good occasion to simplify the emulated variant and document why it is so different from the original. Co-authored-by: Walid Boudebouda <[email protected]>
The reset pointer is effectively the beginning of the reservation, so in order to better generalize HEADERS frames processing between headers and trailers, we reference the workspace directly.
The second (hpack) commit seems to make sense to me, but I do not understand the first: What are your plans with the pipeline logic? #4125 does not seem to make use of the conditional rollback and I do not understand what the meaning of a "pipeline at the end of the workspace" would be? |
The new condition for the rollback ended up in this pull request, in
I can't find a reference to the text you quoted. |
I forgot to answer this specific question. Fetching trailers is done with the same building blocks that fetch headers. And they feed from pipelined bytes before fetching more. Your observation is on point, the pull request currently doesn't leave any room for pipelining, because chunked framing is still fetched one byte at a time. Moving away from this suboptimal (euphemism) mode of operations is definitely a goal. I opted for status quo, because that reduced the amount of changes for the first step. |
bugwash: I still do not understand how the first patch is going to be used later, because #4125 does not currently use it, IIUC. But I do not want to hold things up. |
It prevents bugwash: merge and rebase #4125 onto trunk. |
Originally from #4125, submitted independently as per bugwash to reduce the size of #4125.