https://git.ruby-lang.org/ruby.git/commit/?id=7e14762159

From 7e14762159643b4415e094f9d2a90afaf7994588 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@g...>
Date: Wed, 2 Jun 2021 23:55:23 -0700
Subject: Do not doubly hold an MJIT lock

This is a follow-up of 86c262541ad07528842d76dab4b9b34bd888d5f4.
CRITICAL_SECTION_START/FINISH are not needed when it's called from an
MJIT worker.

Also, ZALLOC needs to be calloc because ZALLOC may trigger GC, which an
MJIT worker must not do.
---
 mjit.c        | 23 ++++++++++++++---------
 mjit_worker.c |  4 ++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/mjit.c b/mjit.c
index d9c83c4..3379b34 100644
--- a/mjit.c
+++ b/mjit.c
@@ -230,13 +230,13 @@ finish_conts(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L230
     }
 }
 
-// Create unit for `iseq`.
+// Create unit for `iseq`. This function may be called from an MJIT worker.
 static void
 create_unit(const rb_iseq_t *iseq)
 {
     struct rb_mjit_unit *unit;
 
-    unit = ZALLOC(struct rb_mjit_unit);
+    unit = calloc(1, sizeof(struct rb_mjit_unit));
     if (unit == NULL)
         return;
 
@@ -253,8 +253,9 @@ mjit_target_iseq_p(struct rb_iseq_constant_body *body) https://github.com/ruby/ruby/blob/trunk/mjit.c#L253
         && !body->builtin_inline_p;
 }
 
+// This is called from an MJIT worker when worker_p is true.
 static void
-mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_info *compile_info)
+mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_info *compile_info, bool worker_p)
 {
     if (!mjit_enabled || pch_status == PCH_FAILED)
         return;
@@ -273,14 +274,18 @@ mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_inf https://github.com/ruby/ruby/blob/trunk/mjit.c#L274
     if (compile_info != NULL)
         iseq->body->jit_unit->compile_info = *compile_info;
 
-    CRITICAL_SECTION_START(3, "in add_iseq_to_process");
+    if (!worker_p) {
+        CRITICAL_SECTION_START(3, "in add_iseq_to_process");
+    }
     add_to_list(iseq->body->jit_unit, &unit_queue);
     if (active_units.length >= mjit_opts.max_cache_size) {
         unload_requests++;
     }
-    verbose(3, "Sending wakeup signal to workers in mjit_add_iseq_to_process");
-    rb_native_cond_broadcast(&mjit_worker_wakeup);
-    CRITICAL_SECTION_FINISH(3, "in add_iseq_to_process");
+    if (!worker_p) {
+        verbose(3, "Sending wakeup signal to workers in mjit_add_iseq_to_process");
+        rb_native_cond_broadcast(&mjit_worker_wakeup);
+        CRITICAL_SECTION_FINISH(3, "in add_iseq_to_process");
+    }
 }
 
 // Add ISEQ to be JITed in parallel with the current thread.
@@ -288,7 +293,7 @@ mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_inf https://github.com/ruby/ruby/blob/trunk/mjit.c#L293
 void
 rb_mjit_add_iseq_to_process(const rb_iseq_t *iseq)
 {
-    mjit_add_iseq_to_process(iseq, NULL);
+    mjit_add_iseq_to_process(iseq, NULL, false);
 }
 
 // For this timeout seconds, --jit-wait will wait for JIT compilation finish.
@@ -353,7 +358,7 @@ mjit_recompile(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/mjit.c#L358
     if (UNLIKELY(mjit_opts.wait)) {
         remove_from_list(iseq->body->jit_unit, &active_units);
         add_to_list(iseq->body->jit_unit, &stale_units);
-        mjit_add_iseq_to_process(iseq, &iseq->body->jit_unit->compile_info);
+        mjit_add_iseq_to_process(iseq, &iseq->body->jit_unit->compile_info, false);
         mjit_wait(iseq->body);
     }
     else {
diff --git a/mjit_worker.c b/mjit_worker.c
index 50f1b07..2e87b1c 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -1396,7 +1396,7 @@ unload_units(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1396
     }
 }
 
-static void mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_info *compile_info);
+static void mjit_add_iseq_to_process(const rb_iseq_t *iseq, const struct rb_mjit_compile_info *compile_info, bool worker_p);
 
 // The function implementing a worker. It is executed in a separate
 // thread by rb_thread_create_mjit_thread. It compiles precompiled header
@@ -1448,7 +1448,7 @@ mjit_worker(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L1448
                         remove_from_list(unit, &active_units);
                         add_to_list(unit, &stale_units);
                         // Lazily put it to unit_queue as well to avoid race conditions on jit_unit with mjit_compile.
-                        mjit_add_iseq_to_process(unit->iseq, &unit->iseq->body->jit_unit->compile_info);
+                        mjit_add_iseq_to_process(unit->iseq, &unit->iseq->body->jit_unit->compile_info, true);
                     }
                 }
             }
-- 
cgit v1.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/