[Rd] Big speedup in install.packages() by re-using connections

[Ivan Krylov]

On Thu, 25 Apr 2024 14:45:04 +0200
Jeroen Ooms <jeroenooms using gmail.com> wrote:

> Thoughts?

How verboten would it be to create an empty external pointer object,
add it to the preserved list, and set an on-exit finalizer to clean up
the curl multi-handle? As far as I can tell, the internet module is not
supposed to be unloaded, so this would not introduce an opportunity to
jump to an unmapped address. This makes it possible to avoid adding a
CurlCleanup() function to the internet module:

Index: src/modules/internet/libcurl.c
===================================================================
--- src/modules/internet/libcurl.c	(revision 86484)
+++ src/modules/internet/libcurl.c	(working copy)
@@ -55,6 +55,47 @@
 
 static int current_timeout = 0;
 
+// The multi-handle is shared between downloads for reusing connections
+static CURLM *shared_mhnd = NULL;
+static SEXP mhnd_sentinel = NULL;
+
+static void cleanup_mhnd(SEXP ignored)
+{
+    if(shared_mhnd){
+        curl_multi_cleanup(shared_mhnd);
+        shared_mhnd = NULL;
+    }
+    curl_global_cleanup();
+}
+static void rollback_mhnd_sentinel(void* sentinel) {
+    // Failed to allocate memory while registering a finalizer,
+    // therefore must release the object
+    R_ReleaseObject((SEXP)sentinel);
+}
+static CURLM *get_mhnd(void)
+{
+    if (!mhnd_sentinel) {
+      SEXP sentinel = PROTECT(R_MakeExternalPtr(NULL, R_NilValue, R_NilValue));
+      R_PreserveObject(sentinel);
+      UNPROTECT(1);
+      // Avoid leaking the sentinel before setting the finalizer
+      RCNTXT cntxt;
+      begincontext(&cntxt, CTXT_CCODE, R_NilValue, R_BaseEnv, R_BaseEnv,
+                   R_NilValue, R_NilValue);
+      cntxt.cend = &rollback_mhnd_sentinel;
+      cntxt.cenddata = sentinel;
+      R_RegisterCFinalizerEx(sentinel, cleanup_mhnd, TRUE);
+      // Succeeded, no need to clean up if endcontext() fails allocation
+      mhnd_sentinel = sentinel;
+      cntxt.cend = NULL;
+      endcontext(&cntxt);
+    }
+    if(!shared_mhnd) {
+      shared_mhnd = curl_multi_init();
+    }
+    return shared_mhnd;
+}
+
 # if LIBCURL_VERSION_MAJOR < 7 || (LIBCURL_VERSION_MAJOR == 7 && LIBCURL_VERSION_MINOR < 28)
 
 // curl/curl.h includes <sys/select.h> and headers it requires.
@@ -565,8 +606,6 @@
 	if (c->hnd && c->hnd[i])
 	    curl_easy_cleanup(c->hnd[i]);
     }
-    if (c->mhnd)
-	curl_multi_cleanup(c->mhnd);
     if (c->headers)
 	curl_slist_free_all(c->headers);
 
@@ -668,7 +707,7 @@
 	c.headers = headers = tmp;
     }
 
-    CURLM *mhnd = curl_multi_init();
+    CURLM *mhnd = get_mhnd();
     if (!mhnd)
 	error(_("could not create curl handle"));
     c.mhnd = mhnd;


-- 
Best regards,
Ivan