Feature request: Support specifying which domain to join when registering C threads

Ecosystem
1

Hi. We are investigating switching from OCaml 4.14 to OCaml 5.3.

Our setup is a C# process calling many times into OCaml, running fairly small computations. This is done from a mixture of short-lived and long-lived threads.

For each thread, the first time we call into OCaml, we use the caml_c_thread_register function. This function will always register the thread in domain 0. To support parallel execution we have made a small patch (attached) that allows the thread to specify which domain to register to. The C# process is made aware of how many OCaml domains are available and then assigns threads in a round-robin manner (domain 0, then 1, etc. and then wrapping around).

This seems to work fine, but is there maybe reasons not to do this approach?

We want to avoid some kind of dispatcher on the OCaml side, as the overhead of that will likely be quite high compared to the time spent on most of the computations.

Assuming this is a valid approach, I will make a feature request on the OCaml Github site.

Another question relates to how we chose the domain for a new thread: Is there a way to quickly determine which domains are not locked? Instead of doing round-robin we may consider such an approach instead, to have a higher chance of the thread being able to do OCaml execution immediately.

Patch 
diff --git a/otherlibs/systhreads/caml/threads.h b/otherlibs/systhreads/caml/threads.h
index 11da0471707..0cd38a93c06 100644
--- a/otherlibs/systhreads/caml/threads.h
+++ b/otherlibs/systhreads/caml/threads.h
@@ -51,17 +51,20 @@ CAMLextern void caml_leave_blocking_section (void);
 */
 
 CAMLextern int caml_c_thread_register(void);
+CAMLextern int caml_c_thread_register_in_domain(int);
 CAMLextern int caml_c_thread_unregister(void);
 
 /* If a thread is created by C code (instead of by OCaml itself),
    it must be registered with the OCaml runtime system before
    being able to call back into OCaml code or use other runtime system
-   functions.  Just call [caml_c_thread_register] once. The domain lock
+   functions.  Just call [caml_c_thread_register] or
+   [caml_c_thread_register_in_domain] once. The domain lock
    is not held when [caml_c_thread_register] returns.
    Before the thread finishes, it must call [caml_c_thread_unregister]
    (without holding the domain lock).
-   Both functions return 1 on success, 0 on error.
-   Note that threads registered by C code belong to the domain 0.
+   All these functions return 1 on success, 0 on error.
+   Threads registered by C code belong to the domain 0 unless a specific
+   domain is given via [caml_c_thread_register_in_domain].
 */
 
 #ifdef __cplusplus
diff --git a/otherlibs/systhreads/st_stubs.c b/otherlibs/systhreads/st_stubs.c
index fdb432540da..7be4ae59b8a 100644
--- a/otherlibs/systhreads/st_stubs.c
+++ b/otherlibs/systhreads/st_stubs.c
@@ -55,8 +55,8 @@
 #include "../../runtime/sync_posix.h"
 
 /* "caml/threads.h" is *not* included since it contains the _external_
-   declarations for the caml_c_thread_register and caml_c_thread_unregister
-   functions. */
+   declarations for the caml_c_thread_register, caml_c_thread_register_in_domain
+   and caml_c_thread_unregister functions. */
 
 /* Max computation time before rescheduling, in milliseconds */
 #define Thread_timeout 50
@@ -717,10 +717,8 @@ CAMLprim value caml_thread_new(value clos)
 
 /* Register a thread already created from C */
 
-#define Dom_c_threads 0
-
 /* the thread lock is not held when entering */
-CAMLexport int caml_c_thread_register(void)
+CAMLexport int caml_c_thread_register_in_domain(int domain_id)
 {
   /* Systhreads initialized? */
   if (!threads_initialized) return 0;
@@ -731,8 +729,8 @@ CAMLexport int caml_c_thread_register(void)
   CAMLassert(Caml_state_opt == NULL);
 
   /* Acquire lock of domain */
-  caml_init_domain_self(Dom_c_threads);
-  thread_lock_acquire(Dom_c_threads);
+  caml_init_domain_self(domain_id);
+  thread_lock_acquire(domain_id);
 
   /* Create tick thread if not already done */
   st_retcode err = create_tick_thread();
@@ -753,10 +751,15 @@ CAMLexport int caml_c_thread_register(void)
 
 out_err:
   /* Note: we cannot raise an exception here. */
-  thread_lock_release(Dom_c_threads);
+  thread_lock_release(domain_id);
   return 0;
 }
 
+CAMLexport int caml_c_thread_register()
+{
+  return caml_c_thread_register_in_domain(0);
+}
+
 /* Unregister a thread that was created from C and registered with
    the function above */
 
diff --git a/otherlibs/systhreads/threads.h b/otherlibs/systhreads/threads.h
index 97fd1b2746c..e97b2232f0b 100644
--- a/otherlibs/systhreads/threads.h
+++ b/otherlibs/systhreads/threads.h
@@ -51,14 +51,16 @@ CAMLextern void caml_leave_blocking_section (void);
 */
 
 CAMLextern int caml_c_thread_register(void);
+CAMLextern int caml_c_thread_register_in_domain(int);
 CAMLextern int caml_c_thread_unregister(void);
 
 /* If a thread is created by C code (instead of by OCaml itself),
    it must be registered with the OCaml runtime system before
    being able to call back into OCaml code or use other runtime system
-   functions.  Just call [caml_c_thread_register] once.
+   functions. Just call [caml_c_thread_register] or
+   [caml_c_thread_register_in_domain] once.
    Before the thread finishes, it must call [caml_c_thread_unregister].
-   Both functions return 1 on success, 0 on error.
+   All these functions return 1 on success, 0 on error.
 */
 
 #ifdef __cplusplus
1 Like
2

For your first question, there can be issues if this is used to change the domain of a C thread (by unregistering and registering it again in a different domain). System threads have a similar limitation. But if you ensure that the choice of domain is final (by making it clear in the specification and adding a relevant check inside caml_c_thread_register_in_domain) then the patch should be simple indeed.

3

Thank you. I will guard against that.
Do you think this is something that could be accepted upstream?

4

The proposal makes sense, but upstream works in mysterious ways!

5

I had a brief look at this. It seems all thread local storage is reset when the thread is unregistered: I can re-register the thread (even in another domain) and it (seemingly, I did not test it a lot) works fine.

If I am to guard against this I believe I have to introduce a new thread local variable which is initially some very large/negative value and, when the thread initially registers, is set to the initial domain.
With such a variable it would be possible to guard any future re-registers of the same thread from joining another domain than the initial one.

Since this is a slightly bigger change, it would be nice to know why the guard is needed. Does anyone know of a source describing the limitation mentioned above?

Thank you.

6

Having a guard against switching domains sounds like the smaller change: less work to convince oneself that it is correct for both implementers and reviewers!

7

I missed that your original message comes with a patch. Would you be willing to submit it as a PR on https://github.com/ocaml/ocaml ?

8

After looking at the patch:

  • I would expect some sanity check that the domain that you are trying to join is already running, and I think this precondition should be clearly stated. (Otherwise users might expect that registering a thread for a non-running domain would start it, and this sounds substantially more difficult.)
  • Iā€™m not sure what behavior to expect if the spawn function that created the domain terminates while this foreign-initiated thread is still running. Do you know? It may be worth checking that something sensible happens, and documenting it.