ruby-changes:64738
From: Koichi <ko1@a...>
Date: Tue, 5 Jan 2021 02:28:23 +0900 (JST)
Subject: [ruby-changes:64738] e7fc353f04 (master): enable constant cache on ractors
https://git.ruby-lang.org/ruby.git/commit/?id=e7fc353f04 From e7fc353f044f9280222ca41b029b1368d2bf2fe3 Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Mon, 4 Jan 2021 18:08:25 +0900 Subject: enable constant cache on ractors constant cache `IC` is accessed by non-atomic manner and there are thread-safety issues, so Ruby 3.0 disables to use const cache on non-main ractors. This patch enables it by introducing `imemo_constcache` and allocates it by every re-fill of const cache like `imemo_callcache`. [Bug #17510] Now `IC` only has one entry `IC::entry` and it points to `iseq_inline_constant_cache_entry`, managed by T_IMEMO object. `IC` is atomic data structure so `rb_mjit_before_vm_ic_update()` and `rb_mjit_after_vm_ic_update()` is not needed. diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 843714a..56add68 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -967,6 +967,18 @@ assert_equal 'can not access non-shareable objects in constant C::CONST by non-m https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L967 end } +# Constant cache should care about non-sharable constants +assert_equal "can not access non-shareable objects in constant Object::STR by non-main Ractor.", %q{ + STR = "hello" + def str; STR; end + s = str() # fill const cache + begin + Ractor.new{ str() }.take + rescue Ractor::RemoteError => e + e.cause.message + end +} + # Setting non-shareable objects into constants by other Ractors is not allowed assert_equal 'can not set constants with non-shareable objects by non-main Ractors', %q{ class C diff --git a/compile.c b/compile.c index df0f8b9..5e0fea0 100644 --- a/compile.c +++ b/compile.c @@ -2359,11 +2359,8 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) https://github.com/ruby/ruby/blob/trunk/compile.c#L2359 } break; } - case TS_ISE: /* inline storage entry */ - /* Treated as an IC, but may contain a markable VALUE */ - FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); - /* fall through */ case TS_IC: /* inline cache */ + case TS_ISE: /* inline storage entry */ case TS_IVC: /* inline ivar cache */ { unsigned int ic_index = FIX2UINT(operands[j]); @@ -2375,8 +2372,7 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) https://github.com/ruby/ruby/blob/trunk/compile.c#L2372 ic_index, body->is_size); } generated_iseq[code_index + 1 + j] = (VALUE)ic; - - if (type == TS_IVC) FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); + FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); break; } case TS_CALLDATA: @@ -9481,14 +9477,13 @@ iseq_build_from_ary_body(rb_iseq_t *iseq, LINK_ANCHOR *const anchor, https://github.com/ruby/ruby/blob/trunk/compile.c#L9477 } break; case TS_ISE: - FL_SET((VALUE)iseq, ISEQ_MARKABLE_ISEQ); - /* fall through */ case TS_IC: case TS_IVC: /* inline ivar cache */ argv[j] = op; if (NUM2UINT(op) >= iseq->body->is_size) { iseq->body->is_size = NUM2INT(op) + 1; } + FL_SET((VALUE)iseq, ISEQ_MARKABLE_ISEQ); break; case TS_CALLDATA: argv[j] = iseq_build_callinfo_from_hash(iseq, op); @@ -10466,14 +10461,13 @@ ibf_load_code(const struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t bytecod https://github.com/ruby/ruby/blob/trunk/compile.c#L10461 break; } case TS_ISE: - FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); - /* fall through */ case TS_IC: case TS_IVC: { VALUE op = ibf_load_small_value(load, &reading_pos); code[code_index] = (VALUE)&is_entries[op]; } + FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); break; case TS_CALLDATA: { diff --git a/debug_counter.h b/debug_counter.h index 8acca27..6dc66ef 100644 --- a/debug_counter.h +++ b/debug_counter.h @@ -309,6 +309,7 @@ RB_DEBUG_COUNTER(obj_imemo_memo) https://github.com/ruby/ruby/blob/trunk/debug_counter.h#L309 RB_DEBUG_COUNTER(obj_imemo_parser_strterm) RB_DEBUG_COUNTER(obj_imemo_callinfo) RB_DEBUG_COUNTER(obj_imemo_callcache) +RB_DEBUG_COUNTER(obj_imemo_constcache) /* ar_table */ RB_DEBUG_COUNTER(artable_hint_hit) diff --git a/ext/objspace/objspace.c b/ext/objspace/objspace.c index 0930e10..7afdfc1 100644 --- a/ext/objspace/objspace.c +++ b/ext/objspace/objspace.c @@ -650,6 +650,7 @@ count_imemo_objects(int argc, VALUE *argv, VALUE self) https://github.com/ruby/ruby/blob/trunk/ext/objspace/objspace.c#L650 imemo_type_ids[10] = rb_intern("imemo_parser_strterm"); imemo_type_ids[11] = rb_intern("imemo_callinfo"); imemo_type_ids[12] = rb_intern("imemo_callcache"); + imemo_type_ids[13] = rb_intern("imemo_constcache"); } each_object_with_flags(count_imemo_objects_i, (void *)hash); diff --git a/gc.c b/gc.c index f8810d1..ad2406c 100644 --- a/gc.c +++ b/gc.c @@ -2398,6 +2398,7 @@ rb_imemo_name(enum imemo_type type) https://github.com/ruby/ruby/blob/trunk/gc.c#L2398 IMEMO_NAME(parser_strterm); IMEMO_NAME(callinfo); IMEMO_NAME(callcache); + IMEMO_NAME(constcache); #undef IMEMO_NAME } return "unknown"; @@ -3102,9 +3103,9 @@ obj_free(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L3103 case imemo_callcache: RB_DEBUG_COUNTER_INC(obj_imemo_callcache); break; - default: - /* unreachable */ - break; + case imemo_constcache: + RB_DEBUG_COUNTER_INC(obj_imemo_constcache); + break; } return 0; @@ -6260,6 +6261,12 @@ gc_mark_imemo(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L6261 gc_mark(objspace, (VALUE)vm_cc_cme(cc)); } return; + case imemo_constcache: + { + const struct iseq_inline_constant_cache_entry *ice = (struct iseq_inline_constant_cache_entry *)obj; + gc_mark(objspace, ice->value); + } + return; #if VM_CHECK_MODE > 0 default: VM_UNREACHABLE(gc_mark_imemo); @@ -8985,6 +8992,12 @@ gc_ref_update_imemo(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L8992 } } break; + case imemo_constcache: + { + const struct iseq_inline_constant_cache_entry *ice = (struct iseq_inline_constant_cache_entry *)obj; + UPDATE_IF_MOVED(objspace, ice->value); + } + break; case imemo_parser_strterm: case imemo_tmpbuf: case imemo_callinfo: diff --git a/insns.def b/insns.def index 73a07e4..e3c7ad5 100644 --- a/insns.def +++ b/insns.def @@ -1023,9 +1023,10 @@ opt_getinlinecache https://github.com/ruby/ruby/blob/trunk/insns.def#L1023 () (VALUE val) { - if (vm_ic_hit_p(ic->ic_serial, ic->ic_cref, GET_EP())) { - val = ic->value; - JUMP(dst); + struct iseq_inline_constant_cache_entry *ice = ic->entry; + if (ice && vm_ic_hit_p(ice, GET_EP())) { + val = ice->value; + JUMP(dst); } else { val = Qnil; @@ -1039,7 +1040,7 @@ opt_setinlinecache https://github.com/ruby/ruby/blob/trunk/insns.def#L1040 (VALUE val) (VALUE val) { - vm_ic_update(ic, val, GET_EP()); + vm_ic_update(GET_ISEQ(), ic, val, GET_EP()); } /* run iseq only once */ diff --git a/internal/imemo.h b/internal/imemo.h index d10f89c..a9e2136 100644 --- a/internal/imemo.h +++ b/internal/imemo.h @@ -45,6 +45,7 @@ enum imemo_type { https://github.com/ruby/ruby/blob/trunk/internal/imemo.h#L45 imemo_parser_strterm = 10, imemo_callinfo = 11, imemo_callcache = 12, + imemo_constcache = 13, }; /* CREF (Class REFerence) is defined in method.h */ diff --git a/iseq.c b/iseq.c index 4a67ee1..096d456 100644 --- a/iseq.c +++ b/iseq.c @@ -182,6 +182,17 @@ iseq_extract_values(VALUE *code, size_t pos, iseq_value_itr_t * func, void *data https://github.com/ruby/ruby/blob/trunk/iseq.c#L182 } } break; + case TS_IC: + { + IC ic = (IC)code[pos + op_no + 1]; + if (ic->entry) { + VALUE nv = func(data, (VALUE)ic->entry); + if ((VALUE)ic->entry != nv) { + ic->entry = (void *)nv; + } + } + } + break; case TS_IVC: { IVC ivc = (IVC)code[pos + op_no + 1]; diff --git a/mjit.c b/mjit.c index 36416c6..68d902a 100644 --- a/mjit.c +++ b/mjit.c @@ -82,24 +82,6 @@ mjit_gc_exit_hook(void) https://github.com/ruby/ruby/blob/trunk/mjit.c#L82 CRITICAL_SECTION_FINISH(4, "mjit_gc_exit_hook"); } -// Lock setinlinecache -void -rb_mjit_before_vm_ic_update(void) -{ - if (!mjit_enabled) - return; - CRITICAL_SECTION_START(3, "before vm_ic_update"); -} - -// Unlock setinlinecache -void -rb_mjit_after_vm_ic_update(void) -{ - if (!mjit_enabled) - return; - CRITICAL_SECTION_FINISH(3, "after vm_ic_update"); -} - // Deal with ISeq movement from compactor void mjit_update_references(const rb_iseq_t *iseq) diff --git a/mjit.h b/mjit.h index 3ef3379..986a3ad 100644 --- a/mjit.h +++ b/mjit.h @@ -83,8 +83,6 @@ RUBY_EXTERN bool mjit_call_p; https://github.com/ruby/ruby/blob/trunk/mjit.h#L83 extern void rb_mjit_add_iseq_to_process(const rb_iseq_t *iseq); extern VALUE rb_mjit_wait_call(rb_execution_context_t *ec, struct rb_iseq_constant_body *body); extern struct rb_mjit_compile_info* rb_mjit_iseq_compile_info(const struct rb_iseq_constant_body *body); - (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/