-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
Add xfer_buf to multi handle #12805
Add xfer_buf to multi handle #12805
Conversation
Shouldn't this also remove the buffer from the easy handle? |
When I rebase on the other merges... |
now with the download buffer removed everywhere. Curious if this survives the torture tests, because conncache shutdown might now try to allocate the xfer_buf, if it had not been used before. |
Yes that sounds possible. Like if you only do But;: this is a transfer buffer now. Surely the connection shutdown no longer needs the buffer? |
True, good point. |
First: I want to wait with merging this until after 8.6.0. For safety. Then: I was thinking: before this PR, the download buffer is only allocated while in use. It is freed again from the easy handle after the transfer is complete (in the DONE state), which makes keeping N easy handles around not use much memory, even if those handles are used with the easy interface. This PR puts the freeing of the buffer back to when the easy handle is cleaned up, making easy handles use much more memory post a completed transfer. Do you think it would make sense to have the multi handle free the download buffer once there is no transfer in progress anymore? It would risk having to re-alloc it again immediately when new transfers are added, but it would make the easy interface use much more like before and might even make sense for the multi interface use cases too: simply use less memory when no transfers are running. |
Agree.
We could, when entering |
Added |
This comment was marked as outdated.
This comment was marked as outdated.
Tweaked: diff --git a/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md b/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md
index 1faebeef54..b4759ea653 100644
--- a/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md
+++ b/docs/libcurl/opts/CURLOPT_BUFFERSIZE.md
@@ -40,10 +40,15 @@ buffer size allowed to be set is 1024.
DO NOT set this option on a handle that is currently used for an active
transfer as that may lead to unintended consequences.
The maximum size was 512kB until 7.88.0.
+Starting in libcurl 8.7.0, there is just a single transfer buffer allocated
+per multi handle. This buffer is used by all easy handles added to a multi
+handle no matter how many parallel transfers there are. The buffer remains
+allocated as long as there are active transfers.
+
# DEFAULT
CURL_MAX_WRITE_SIZE (16kB)
# PROTOCOLS |
- can be borrowed by transfer during recv-write operation - needs to be released before borrowing again - adjustis size to `data->set.buffer_size` - used in transfer.c readwrite_data()
Added this, thanks! Also rebased without problems. |
data->set.buffer_size