transfer: DID handling - #22763
Closed
icing wants to merge 3 commits into
Closed
transfer: DID handling#22763icing wants to merge 3 commits into
icing wants to merge 3 commits into
Conversation
When a transfer leaves its DID state and goes to PERFORM, it should not hog the multi processing when there are other transfers alive. Make multistate_did() return CURLM_OK then which triggers the processing of the next easy handle. This gives more fairness among easy handles: - on a fast server, responses may arrive very quickly and processing a response is costly. All this time is then lost to other transfers that have not even sent their request - on a slow server, directly trying to receive the response will not work and is an unnecessary network call. Local scorecard tests with h1 requests showed that the former handling was only using 10 connections for 40 parallel requests after an initial peak of 40 connections. The change makes libcurl use all 40 connections continuously. This also explains the h1-requests memory use development. When we closed connections more aggressively, the 40 initial connections shrank and released their memory. But the parallelism was not good.
There was a problem hiding this comment.
🔵 Needs a closer look
Pull request overview
Adjusts libcurl’s multi state machine so transfers transitioning from MSTATE_DID to MSTATE_PERFORMING don’t monopolize immediate re-processing when multiple transfers are active, improving fairness and sustained parallelism (notably for HTTP/1.x workloads).
Changes:
- Update
multistate_did()to sometimes returnCURLM_OK(instead ofCURLM_CALL_MULTI_PERFORM) after enteringMSTATE_PERFORMING, allowing other easy handles to progress. - Restrict the fairness behavior to HTTP-family protocols and only when more than one transfer is alive.
File summaries
| File | Description |
|---|---|
| lib/multi.c | Changes multistate_did() return behavior to improve scheduling fairness across concurrent transfers. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
When a transfer leaves its DID state and goes to PERFORM, it should not hog the multi processing when there are other transfers alive. Make multistate_did() return CURLM_OK then which triggers the processing of the next easy handle.
This gives more fairness among easy handles:
Local scorecard tests with h1 requests showed that the former handling was only using 10 connections for 40 parallel requests after an initial peak of 40 connections. The change makes libcurl use all 40 connections continuously.
This also explains the h1-requests memory use development. When we closed connections more aggressively, the 40 initial connections shrank and released their memory. But the parallelism was not good.