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/