[前][次][番号順一覧][スレッド一覧]

ruby-changes:63346

From: Koichi <ko1@a...>
Date: Wed, 14 Oct 2020 16:37:18 +0900 (JST)
Subject: [ruby-changes:63346] fad97f1f96 (master): sync generic_ivtbl

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

From fad97f1f96caf11005a5858a29d32c66203913e8 Mon Sep 17 00:00:00 2001
From: Koichi Sasada <ko1@a...>
Date: Wed, 14 Oct 2020 10:43:13 +0900
Subject: sync generic_ivtbl

generic_ivtbl is a process global table to maintain instance variables
for non T_OBJECT/T_CLASS/... objects. So we need to protect them
for multi-Ractor exection.

Hint: we can make them Ractor local for unshareable objects, but
      now it is premature optimization.

diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb
index 05139e2..eaa265f 100644
--- a/bootstraptest/test_ractor.rb
+++ b/bootstraptest/test_ractor.rb
@@ -780,6 +780,7 @@ assert_equal "#{N}#{N}", %Q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L780
   }.map{|r| r.take}.join
 }
 
+# enc_table
 assert_equal "#{N/10}", %Q{
   Ractor.new do
     loop do
@@ -794,4 +795,20 @@ assert_equal "#{N/10}", %Q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L795
   }
 }
 
+# Generic ivtbl
+n = N/2
+assert_equal "#{n}#{n}", %Q{
+  2.times.map{
+    Ractor.new do
+      #{n}.times do
+        obj = ''
+        obj.instance_variable_set("@a", 1)
+        obj.instance_variable_set("@b", 1)
+        obj.instance_variable_set("@c", 1)
+        obj.instance_variable_defined?("@a")
+      end
+    end
+  }.map{|r| r.take}.join
+}
+
 end # if !ENV['GITHUB_WORKFLOW']
diff --git a/common.mk b/common.mk
index e7b8f53..5c87f95 100644
--- a/common.mk
+++ b/common.mk
@@ -15082,6 +15082,7 @@ variable.$(OBJEXT): {$(VPATH)}variable.h https://github.com/ruby/ruby/blob/trunk/common.mk#L15082
 variable.$(OBJEXT): {$(VPATH)}vm_core.h
 variable.$(OBJEXT): {$(VPATH)}vm_debug.h
 variable.$(OBJEXT): {$(VPATH)}vm_opts.h
+variable.$(OBJEXT): {$(VPATH)}vm_sync.h
 version.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h
 version.$(OBJEXT): $(CCAN_DIR)/container_of/container_of.h
 version.$(OBJEXT): $(CCAN_DIR)/list/list.h
diff --git a/tool/ruby_vm/views/_mjit_compile_ivar.erb b/tool/ruby_vm/views/_mjit_compile_ivar.erb
index 7283d37..eb05f4d 100644
--- a/tool/ruby_vm/views/_mjit_compile_ivar.erb
+++ b/tool/ruby_vm/views/_mjit_compile_ivar.erb
@@ -82,7 +82,7 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_ivar.erb#L82
 % # JIT: cache hit path of vm_getivar, or cancel JIT (recompile it without any ivar optimization)
         fprintf(f, "    struct gen_ivtbl *ivtbl;\n");
         fprintf(f, "    VALUE val;\n");
-        fprintf(f, "    if (LIKELY(FL_TEST_RAW(obj, FL_EXIVAR) && ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass) && st_lookup(rb_ivar_generic_ivtbl(obj), (st_data_t)obj, (st_data_t *)&ivtbl) && index < ivtbl->numiv && (val = ivtbl->ivptr[index]) != Qundef)) {\n");
+        fprintf(f, "    if (LIKELY(FL_TEST_RAW(obj, FL_EXIVAR) && ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass) && rb_ivar_generic_ivtbl_lookup(obj, &ivtbl) && index < ivtbl->numiv && (val = ivtbl->ivptr[index]) != Qundef)) {\n");
         fprintf(f, "        stack[%d] = val;\n", b->stack_size);
         fprintf(f, "    }\n");
         fprintf(f, "    else {\n");
diff --git a/variable.c b/variable.c
index 0689112..bf80df7 100644
--- a/variable.c
+++ b/variable.c
@@ -37,6 +37,7 @@ https://github.com/ruby/ruby/blob/trunk/variable.c#L37
 #include "variable.h"
 #include "vm_core.h"
 #include "ractor_pub.h"
+#include "vm_sync.h"
 
 typedef void rb_gvar_compact_t(void *var);
 
@@ -896,6 +897,8 @@ IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(ID id) https://github.com/ruby/ruby/blob/trunk/variable.c#L897
 static inline struct st_table *
 generic_ivtbl(VALUE obj, ID id, bool force_check_ractor)
 {
+    ASSERT_vm_locking();
+
     if ((force_check_ractor || rb_is_instance_id(id)) && // not internal ID
         UNLIKELY(rb_ractor_shareable_p(obj) && !rb_ractor_main_p())) {
         rb_raise(rb_eRuntimeError, "can not access instance variables of shareable objects from non-main Ractors");
@@ -909,22 +912,28 @@ generic_ivtbl_no_ractor_check(VALUE obj) https://github.com/ruby/ruby/blob/trunk/variable.c#L912
     return generic_ivtbl(obj, 0, false);
 }
 
-MJIT_FUNC_EXPORTED struct st_table *
-rb_ivar_generic_ivtbl(VALUE obj)
-{
-    return generic_ivtbl(obj, 0, true);
-}
-
 static int
 gen_ivtbl_get(VALUE obj, ID id, struct gen_ivtbl **ivtbl)
 {
     st_data_t data;
+    int r = 0;
 
-    if (st_lookup(generic_ivtbl(obj, id, false), (st_data_t)obj, &data)) {
-	*ivtbl = (struct gen_ivtbl *)data;
-	return 1;
+    RB_VM_LOCK_ENTER();
+    {
+        if (st_lookup(generic_ivtbl(obj, id, false), (st_data_t)obj, &data)) {
+            *ivtbl = (struct gen_ivtbl *)data;
+            r = 1;
+        }
     }
-    return 0;
+    RB_VM_LOCK_LEAVE();
+
+    return r;
+}
+
+MJIT_FUNC_EXPORTED int
+rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **ivtbl)
+{
+    return gen_ivtbl_get(obj, 0, ivtbl);
 }
 
 static VALUE
@@ -1275,8 +1284,13 @@ generic_ivar_set(VALUE obj, ID id, VALUE val) https://github.com/ruby/ruby/blob/trunk/variable.c#L1284
     ivup.iv_extended = 0;
     ivup.u.iv_index_tbl = iv_index_tbl_make(obj);
     iv_index_tbl_extend(&ivup, id);
-    st_update(generic_ivtbl(obj, id, false), (st_data_t)obj, generic_ivar_update,
-	      (st_data_t)&ivup);
+
+    RB_VM_LOCK_ENTER();
+    {
+        st_update(generic_ivtbl(obj, id, false), (st_data_t)obj, generic_ivar_update,
+                  (st_data_t)&ivup);
+    }
+    RB_VM_LOCK_LEAVE();
 
     ivup.u.ivtbl->ivptr[ivup.index] = val;
 
@@ -1590,8 +1604,12 @@ rb_copy_generic_ivar(VALUE clone, VALUE obj) https://github.com/ruby/ruby/blob/trunk/variable.c#L1604
 	 * c.ivtbl may change in gen_ivar_copy due to realloc,
 	 * no need to free
 	 */
-        generic_ivtbl_no_ractor_check(clone);
-	st_insert(generic_ivtbl_no_ractor_check(obj), (st_data_t)clone, (st_data_t)c.ivtbl);
+        RB_VM_LOCK_ENTER();
+        {
+            generic_ivtbl_no_ractor_check(clone);
+            st_insert(generic_ivtbl_no_ractor_check(obj), (st_data_t)clone, (st_data_t)c.ivtbl);
+        }
+        RB_VM_LOCK_LEAVE();
     }
     return;
 
diff --git a/variable.h b/variable.h
index 2f010b6..4d71f87 100644
--- a/variable.h
+++ b/variable.h
@@ -16,6 +16,6 @@ struct gen_ivtbl { https://github.com/ruby/ruby/blob/trunk/variable.h#L16
     VALUE ivptr[FLEX_ARY_LEN];
 };
 
-struct st_table *rb_ivar_generic_ivtbl(VALUE obj);
+int rb_ivar_generic_ivtbl_lookup(VALUE obj, struct gen_ivtbl **);
 
 #endif /* RUBY_TOPLEVEL_VARIABLE_H */
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index ea93414..275e5f7 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1101,7 +1101,7 @@ vm_getivar(VALUE obj, ID id, IVC ic, const struct rb_callcache *cc, int is_attr) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1101
         else if (FL_TEST_RAW(obj, FL_EXIVAR)) {
             struct gen_ivtbl *ivtbl;
 
-            if (LIKELY(st_lookup(rb_ivar_generic_ivtbl(obj), (st_data_t)obj, (st_data_t *)&ivtbl)) &&
+            if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl)) &&
                 LIKELY(index < ivtbl->numiv)) {
                 val = ivtbl->ivptr[index];
             }
@@ -1123,7 +1123,7 @@ vm_getivar(VALUE obj, ID id, IVC ic, const struct rb_callcache *cc, int is_attr) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1123
 	}
         else if (FL_TEST_RAW(obj, FL_EXIVAR)) {
             struct gen_ivtbl *ivtbl;
-            if (LIKELY(st_lookup(rb_ivar_generic_ivtbl(obj), (st_data_t)obj, (st_data_t *)&ivtbl))) {
+            if (LIKELY(rb_ivar_generic_ivtbl_lookup(obj, &ivtbl))) {
                 numiv = ivtbl->numiv;
                 ivptr = ivtbl->ivptr;
                 iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj));
-- 
cgit v0.10.2


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

[前][次][番号順一覧][スレッド一覧]