Mika Fischer <mika.fischer@xxxxxxxxxx> writes:
> Previously, when nothing could be read from the connections curl had
> open, git would just sleep unconditionally for 50ms. This patch changes
> this behavior and instead obtains the recommended timeout and the actual
> file descriptors from curl. This should eliminate time spent sleeping when
> data could actually be read/written on the socket.
>
> Signed-off-by: Mika Fischer <mika.fischer@xxxxxxxxxx>
> ---
Thanks. I added Daniel back to Cc: list as I know he is the area expert
when it comes to the use of libcurl, and also his input helped to polish
this patch during the initial round of discussion.
> http.c | 21 ++++++++++++++++-----
> 1 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/http.c b/http.c
> index a4bc770..12180f3 100644
> --- a/http.c
> +++ b/http.c
> @@ -649,6 +649,7 @@ void run_active_slot(struct active_request_slot *slot)
> fd_set excfds;
> int max_fd;
> struct timeval select_timeout;
> + long int curl_timeout;
Just a style nit, but we usually spell this "long" not "long int" in our
codebase.
> @@ -664,14 +665,24 @@ void run_active_slot(struct active_request_slot *slot)
> }
>
> if (slot->in_use && !data_received) {
> - max_fd = 0;
> + curl_multi_timeout(curlm, &curl_timeout);
According to http://curl.haxx.se/libcurl/c/curl_multi_timeout.html
this was added in 7.15.4 which may be much newer than some of the versions
the existing code checks LIBCURL_VERSION_NUM against (grep for it in http.c).
Shouldn't you make this conditional?
> + if (curl_timeout == 0) {
> + continue;
> + } else if (curl_timeout == -1) {
> + select_timeout.tv_sec = 0;
> + select_timeout.tv_usec = 50000;
> + } else {
> + select_timeout.tv_sec = curl_timeout / 1000;
> + select_timeout.tv_usec = (curl_timeout % 1000) * 1000;
> + }
> +
> + max_fd = -1;
> FD_ZERO(&readfds);
> FD_ZERO(&writefds);
> FD_ZERO(&excfds);
> - select_timeout.tv_sec = 0;
> - select_timeout.tv_usec = 50000;
> - select(max_fd, &readfds, &writefds,
> - &excfds, &select_timeout);
> + curl_multi_fdset(curlm, &readfds, &writefds, &excfds, &max_fd);
I couldn't find in http://curl.haxx.se/libcurl/c/curl_multi_fdset.html
what the version requirement for using this function is, but the same
comment as above applies here.
By the way, I think I saw Daniel posting a link to a nicely formatted
table that lists each and every functions and CURLOPT_* symbol with
ranges of version it is usable, but I seem to be unable to find it.
> +
> + select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
> }
> }
> #else
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html