From c46bda6f191b01121ebbc8afa88b35683b6417a9 Mon Sep 17 00:00:00 2001
From: Alan Wu <XrXr@users.noreply.github.com>
Date: Mon, 20 Sep 2021 14:55:10 -0400
Subject: [PATCH] Fix excessive invalidation for opt_getinlinecache

YJIT expects the VM to invalidate opt_getinlinecache when updating the
constant cache, and the invalidation used to happen even when YJIT can't
use the cached value.

Once the first invalidation happens, the block for opt_getinlinecache
becomes a stub. When the stub is hit, YJIT fails to compile the
instruction as the cache is not usable. The stub becomes a block that
exits for opt_getinlinecache which can be invalidated again. Some
workloads that bust the interpreter's constant cache can create an
invalidation loop with this behavior.

Check if the cache is usable become doing invalidation to fix this
problem.

In the test harness, evaluate the test script in a lambda instead of a
proc so `return` doesn't return out of the harness.
---
 test/ruby/test_yjit.rb | 27 ++++++++++++++++++++++++++-
 yjit_codegen.c         |  2 +-
 yjit_iface.c           | 10 +++++++++-
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb
index ed83bd681a..e6325a4d1c 100644
--- a/test/ruby/test_yjit.rb
+++ b/test/ruby/test_yjit.rb
@@ -394,6 +394,31 @@ class TestYJIT < Test::Unit::TestCase
     RUBY
   end
 
+  def test_no_excessive_opt_getinlinecache_invalidation
+    assert_compiles(<<~'RUBY', exits: :any, result: :ok)
+      objects = [Object.new, Object.new]
+
+      objects.each do |o|
+        class << o
+          def foo
+            Object
+          end
+        end
+      end
+
+      9000.times {
+        objects[0].foo
+        objects[1].foo
+      }
+
+      stats = YJIT.runtime_stats
+      return :ok unless stats[:all_stats]
+      return :ok if stats[:invalidation_count] < 10
+
+      :fail
+    RUBY
+  end
+
   def assert_no_exits(script)
     assert_compiles(script)
   end
@@ -437,7 +462,7 @@ class TestYJIT < Test::Unit::TestCase
 
     script = <<~RUBY
       #{"# frozen_string_literal: true" if frozen_string_literal}
-      _test_proc = proc {
+      _test_proc = -> {
         #{test_script}
       }
       #{reset_stats}
diff --git a/yjit_codegen.c b/yjit_codegen.c
index 4fec409621..0afe617bb1 100644
--- a/yjit_codegen.c
+++ b/yjit_codegen.c
@@ -4068,7 +4068,7 @@ gen_opt_getinlinecache(jitstate_t *jit, ctx_t *ctx)
     VALUE const_cache_as_value = jit_get_arg(jit, 1);
     IC ic = (IC)const_cache_as_value;
 
-    // See vm_ic_hit_p().
+    // See vm_ic_hit_p(). The same conditions are checked in yjit_constant_ic_update().
     struct iseq_inline_constant_cache_entry *ice = ic->entry;
     if (!ice || // cache not filled
         ice->ic_serial != ruby_vm_global_constant_state || // cache out of date
diff --git a/yjit_iface.c b/yjit_iface.c
index 0f11fa61a5..18f35f576d 100644
--- a/yjit_iface.c
+++ b/yjit_iface.c
@@ -597,10 +597,18 @@ rb_yjit_constant_state_changed(void)
     }
 }
 
-// Callback from the opt_setinlinecache instruction in the interpreter
+// Callback from the opt_setinlinecache instruction in the interpreter.
+// Invalidate the block for the matching opt_getinlinecache so it could regenerate code
+// using the new value in the constant cache.
 void
 yjit_constant_ic_update(const rb_iseq_t *iseq, IC ic)
 {
+    // We can't generate code in these situations, so no need to invalidate.
+    // See gen_opt_getinlinecache.
+    if (ic->entry->ic_cref || rb_multi_ractor_p()) {
+        return;
+    }
+
     RB_VM_LOCK_ENTER();
     rb_vm_barrier(); // Stop other ractors since we are going to patch machine code.
     {