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

From e8a78d7df899291aa025e41207436a450235a4d7 Mon Sep 17 00:00:00 2001
From: Takashi Kokubun <takashikkbn@g...>
Date: Thu, 30 Apr 2020 21:37:55 -0700
Subject: Do not stop the world during JIT compaction

Running C compiler for JIT compaction inside a critical section may lock
main thread for a long time when it triggers GC. As I'm planning to
increase this duration a bit, I'd like to make sure this doesn't stop
the world.

For now, I chose to give up unloading units when it's during JIT
compaction, assuming other calls may unload them later.

diff --git a/mjit.c b/mjit.c
index c394cd5..59e58f3 100644
--- a/mjit.c
+++ b/mjit.c
@@ -362,11 +362,7 @@ unload_units(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L362
         free_unit(worst);
     }
 
-    if (units_num == active_units.length && mjit_opts.wait) {
-        mjit_opts.max_cache_size++; // avoid infinite loop on `rb_mjit_wait_call`. Note that --jit-wait is just for testing.
-        verbose(1, "No units can be unloaded -- incremented max-cache-size to %d for --jit-wait", mjit_opts.max_cache_size);
-    }
-    else {
+    if (units_num > active_units.length) {
         verbose(1, "Too many JIT code -- %d units unloaded", units_num - active_units.length);
     }
 }
@@ -389,8 +385,16 @@ 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#L385
     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) {
-        RB_DEBUG_COUNTER_INC(mjit_unload_units);
-        unload_units();
+        if (in_compact) {
+            verbose(1, "Too many JIT code, but skipped unloading units for JIT compaction");
+        } else {
+            RB_DEBUG_COUNTER_INC(mjit_unload_units);
+            unload_units();
+        }
+        if (active_units.length == mjit_opts.max_cache_size && mjit_opts.wait) { // Sometimes all methods may be in use
+            mjit_opts.max_cache_size++; // avoid infinite loop on `rb_mjit_wait_call`. Note that --jit-wait is just for testing.
+            verbose(1, "No units can be unloaded -- incremented max-cache-size to %d for --jit-wait", mjit_opts.max_cache_size);
+        }
     }
     verbose(3, "Sending wakeup signal to workers in mjit_add_iseq_to_process");
     rb_native_cond_broadcast(&mjit_worker_wakeup);
diff --git a/mjit_worker.c b/mjit_worker.c
index a2f6244..04eac1f 100644
--- a/mjit_worker.c
+++ b/mjit_worker.c
@@ -221,9 +221,11 @@ static rb_nativethread_cond_t mjit_worker_wakeup; https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L221
 // A thread conditional to wake up workers if at the end of GC.
 static rb_nativethread_cond_t mjit_gc_wakeup;
 // True when GC is working.
-static bool in_gc;
+static bool in_gc = false;
 // True when JIT is working.
-static bool in_jit;
+static bool in_jit = false;
+// True when JIT compaction is running.
+static bool in_compact = false;
 // Set to true to stop worker.
 static bool stop_worker_p;
 // Set to true if worker is stopped.
@@ -913,21 +915,21 @@ compact_all_jit_code(void) https://github.com/ruby/ruby/blob/trunk/mjit_worker.c#L915
     // NULL-ending for form_args
     o_files = alloca(sizeof(char *) * (active_units.length + 1));
     o_files[active_units.length] = NULL;
-    CRITICAL_SECTION_START(3, "in compact_all_jit_code to keep .o files");
+    CRITICAL_SECTION_START(3, "in compact_all_jit_code to guard .o files from unload_units");
     list_for_each(&active_units.head, cur, unode) {
         o_files[i] = cur->o_file;
         i++;
     }
+    in_compact = true;
+    CRITICAL_SECTION_FINISH(3, "in compact_all_jit_code to guard .o files from unload_units");
 
     start_time = real_ms_time();
     bool success = link_o_to_so(o_files, so_file);
     end_time = real_ms_time();
 
-    // TODO: Shrink this big critical section. For now, this is needed to prevent failure by missing .o files.
-    // This assumes that o -> so link doesn't take long time because the bottleneck, which is compiler optimization,
-    // is already done. But actually it takes about 500ms for 5,000 methods on my Linux machine, so it's better to
-    // finish this critical section before link_o_to_so by disabling unload_units.
-    CRITICAL_SECTION_FINISH(3, "in compact_all_jit_code to keep .o files");
+    CRITICAL_SECTION_START(3, "in compact_all_jit_code to release .o files");
+    in_compact = false;
+    CRITICAL_SECTION_FINISH(3, "in compact_all_jit_code to release .o files");
 
     if (success) {
         void *handle = dlopen(so_file, RTLD_NOW);
diff --git a/test/ruby/test_jit.rb b/test/ruby/test_jit.rb
index 9e4367b..1d0c86d 100644
--- a/test/ruby/test_jit.rb
+++ b/test/ruby/test_jit.rb
@@ -14,6 +14,7 @@ class TestJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_jit.rb#L14
   ]
   MAX_CACHE_PATTERNS = [
     /\AJIT compaction \([^)]+\): .+\n\z/,
+    /\AToo many JIT code, but skipped unloading units for JIT compaction\n\z/,
     /\ANo units can be unloaded -- .+\n\z/,
   ]
 
@@ -700,8 +701,9 @@ class TestJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_jit.rb#L701
       10.times do |i|
         assert_match(/\A#{JIT_SUCCESS_PREFIX}: mjit#{i}@\(eval\):/, errs[i], debug_info)
       end
-      assert_equal("Too many JIT code -- 1 units unloaded\n", errs[10], debug_info)
-      assert_match(/\A#{JIT_SUCCESS_PREFIX}: mjit10@\(eval\):/, errs[11], debug_info)
+      assert_equal("Too many JIT code, but skipped unloading units for JIT compaction\n", errs[10], debug_info)
+      assert_equal("No units can be unloaded -- incremented max-cache-size to 11 for --jit-wait\n", errs[11], debug_info)
+      assert_match(/\A#{JIT_SUCCESS_PREFIX}: mjit10@\(eval\):/, errs[12], debug_info)
 
       # On --jit-wait, when the number of JIT-ed code reaches --jit-max-cache,
       # it should trigger compaction.
-- 
cgit v0.10.2


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