Richard W.M. Jones
2023-Jul-27 11:46 UTC
[Libguestfs] [PATCH nbdkit] curl: Try to share as much as possible between handles in the pool
Using the libcurl share interface we can share data between the separate curl easy handles in the pool. For more about this see: https://curl.se/libcurl/c/CURLSHOPT_SHARE.html https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b https://everything.curl.dev/libcurl/sharing --- plugins/curl/pool.c | 65 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/plugins/curl/pool.c b/plugins/curl/pool.c index 10f9011d7..e9f387c38 100644 --- a/plugins/curl/pool.c +++ b/plugins/curl/pool.c @@ -52,6 +52,7 @@ #include <nbdkit-plugin.h> +#include "array-size.h" #include "ascii-ctype.h" #include "ascii-string.h" #include "cleanup.h" @@ -72,6 +73,18 @@ static int get_content_length_accept_range (struct curl_handle *ch); static bool try_fallback_GET_method (struct curl_handle *ch); static size_t header_cb (void *ptr, size_t size, size_t nmemb, void *opaque); static size_t error_cb (char *ptr, size_t size, size_t nmemb, void *opaque); +static void lock_cb (CURL *handle, curl_lock_data data, + curl_lock_access access, void *userptr); +static void unlock_cb (CURL *handle, curl_lock_data data, + void *userptr); + +/* These locks protect access to the curl share data. See: + * https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b + */ +static pthread_rwlock_t sharelocks[CURL_LOCK_DATA_LAST]; + +/* Curl share data. */ +static CURLSH *share; /* This lock protects access to the curl_handles vector below. */ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; @@ -93,6 +106,22 @@ static size_t in_use = 0, waiting = 0; void load_pool (void) { + size_t i; + + for (i = 0; i < ARRAY_SIZE (sharelocks); ++i) + pthread_rwlock_init (&sharelocks[i], NULL); + + share = curl_share_init (); + if (share == NULL) { + nbdkit_error ("curl_share_init: %m"); + exit (EXIT_FAILURE); + } + curl_share_setopt (share, CURLSHOPT_LOCKFUNC, lock_cb); + curl_share_setopt (share, CURLSHOPT_UNLOCKFUNC, unlock_cb); + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE); + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); } /* Close and free all handles in the pool. */ @@ -108,6 +137,12 @@ unload_pool (void) for (i = 0; i < curl_handles.len; ++i) free_handle (curl_handles.ptr[i]); curl_handle_list_reset (&curl_handles); + + /* Free share data. */ + curl_share_cleanup (share); + + for (i = 0; i < ARRAY_SIZE (sharelocks); ++i) + pthread_rwlock_destroy (&sharelocks[i]); } /* Get a handle from the pool. @@ -230,6 +265,9 @@ allocate_handle (void) goto err; } + /* Share settings with other handles. */ + curl_easy_setopt (ch->c, CURLOPT_SHARE, share); + /* Various options we always set. * * NB: Both here and below constants must be explicitly long because @@ -621,3 +659,30 @@ error_cb (char *ptr, size_t size, size_t nmemb, void *opaque) return 0; /* in older curl, any size < requested will also be an error */ #endif } + +static void +lock_cb (CURL *handle, curl_lock_data data, curl_lock_access access, + void *userptr) +{ + assert (data < ARRAY_SIZE (sharelocks)); + + switch (access) { + case CURL_LOCK_ACCESS_SHARED: + pthread_rwlock_rdlock (&sharelocks[data]); + break; + case CURL_LOCK_ACCESS_SINGLE: + pthread_rwlock_wrlock (&sharelocks[data]); + break; + default: + /* CURL_LOCK_ACCESS_NONE is never used in the current libcurl code. */ + abort (); + } +} + +static void +unlock_cb (CURL *handle, curl_lock_data data, void *userptr) +{ + assert (data < ARRAY_SIZE (sharelocks)); + + pthread_rwlock_unlock (&sharelocks[data]); +} -- 2.41.0
Laszlo Ersek
2023-Jul-27 11:59 UTC
[Libguestfs] [PATCH nbdkit] curl: Try to share as much as possible between handles in the pool
On 7/27/23 13:46, Richard W.M. Jones wrote:> Using the libcurl share interface we can share data between the > separate curl easy handles in the pool. For more about this see: > > https://curl.se/libcurl/c/CURLSHOPT_SHARE.html > https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b > https://everything.curl.dev/libcurl/sharing > --- > plugins/curl/pool.c | 65 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 65 insertions(+)Can you document the benefits of this change in the commit message? (Faster downloads / lower CPU demand / lower memory usage -- just guessing.) Acked-by: Laszlo Ersek <lersek at redhat.com> Thanks Laszlo> > diff --git a/plugins/curl/pool.c b/plugins/curl/pool.c > index 10f9011d7..e9f387c38 100644 > --- a/plugins/curl/pool.c > +++ b/plugins/curl/pool.c > @@ -52,6 +52,7 @@ > > #include <nbdkit-plugin.h> > > +#include "array-size.h" > #include "ascii-ctype.h" > #include "ascii-string.h" > #include "cleanup.h" > @@ -72,6 +73,18 @@ static int get_content_length_accept_range (struct curl_handle *ch); > static bool try_fallback_GET_method (struct curl_handle *ch); > static size_t header_cb (void *ptr, size_t size, size_t nmemb, void *opaque); > static size_t error_cb (char *ptr, size_t size, size_t nmemb, void *opaque); > +static void lock_cb (CURL *handle, curl_lock_data data, > + curl_lock_access access, void *userptr); > +static void unlock_cb (CURL *handle, curl_lock_data data, > + void *userptr); > + > +/* These locks protect access to the curl share data. See: > + * https://gist.github.com/bagder/7eccf74f8b6d70b5abefeb7f288dba9b > + */ > +static pthread_rwlock_t sharelocks[CURL_LOCK_DATA_LAST]; > + > +/* Curl share data. */ > +static CURLSH *share; > > /* This lock protects access to the curl_handles vector below. */ > static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; > @@ -93,6 +106,22 @@ static size_t in_use = 0, waiting = 0; > void > load_pool (void) > { > + size_t i; > + > + for (i = 0; i < ARRAY_SIZE (sharelocks); ++i) > + pthread_rwlock_init (&sharelocks[i], NULL); > + > + share = curl_share_init (); > + if (share == NULL) { > + nbdkit_error ("curl_share_init: %m"); > + exit (EXIT_FAILURE); > + } > + curl_share_setopt (share, CURLSHOPT_LOCKFUNC, lock_cb); > + curl_share_setopt (share, CURLSHOPT_UNLOCKFUNC, unlock_cb); > + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_CONNECT); > + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_COOKIE); > + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_DNS); > + curl_share_setopt (share, CURLSHOPT_SHARE, CURL_LOCK_DATA_SSL_SESSION); > } > > /* Close and free all handles in the pool. */ > @@ -108,6 +137,12 @@ unload_pool (void) > for (i = 0; i < curl_handles.len; ++i) > free_handle (curl_handles.ptr[i]); > curl_handle_list_reset (&curl_handles); > + > + /* Free share data. */ > + curl_share_cleanup (share); > + > + for (i = 0; i < ARRAY_SIZE (sharelocks); ++i) > + pthread_rwlock_destroy (&sharelocks[i]); > } > > /* Get a handle from the pool. > @@ -230,6 +265,9 @@ allocate_handle (void) > goto err; > } > > + /* Share settings with other handles. */ > + curl_easy_setopt (ch->c, CURLOPT_SHARE, share); > + > /* Various options we always set. > * > * NB: Both here and below constants must be explicitly long because > @@ -621,3 +659,30 @@ error_cb (char *ptr, size_t size, size_t nmemb, void *opaque) > return 0; /* in older curl, any size < requested will also be an error */ > #endif > } > + > +static void > +lock_cb (CURL *handle, curl_lock_data data, curl_lock_access access, > + void *userptr) > +{ > + assert (data < ARRAY_SIZE (sharelocks)); > + > + switch (access) { > + case CURL_LOCK_ACCESS_SHARED: > + pthread_rwlock_rdlock (&sharelocks[data]); > + break; > + case CURL_LOCK_ACCESS_SINGLE: > + pthread_rwlock_wrlock (&sharelocks[data]); > + break; > + default: > + /* CURL_LOCK_ACCESS_NONE is never used in the current libcurl code. */ > + abort (); > + } > +} > + > +static void > +unlock_cb (CURL *handle, curl_lock_data data, void *userptr) > +{ > + assert (data < ARRAY_SIZE (sharelocks)); > + > + pthread_rwlock_unlock (&sharelocks[data]); > +}