Re: [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(a)redhat.com&gt; 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]); > +}