ruby-changes:63366
From: Koichi <ko1@a...>
Date: Sat, 17 Oct 2020 08:18:25 +0900 (JST)
Subject: [ruby-changes:63366] f6661f5085 (master): sync RClass::ext::iv_index_tbl
https://git.ruby-lang.org/ruby.git/commit/?id=f6661f5085 From f6661f50854e0cdccb03ee516a21ce62adf6c802 Mon Sep 17 00:00:00 2001 From: Koichi Sasada <ko1@a...> Date: Fri, 16 Oct 2020 15:20:40 +0900 Subject: sync RClass::ext::iv_index_tbl iv_index_tbl manages instance variable indexes (ID -> index). This data structure should be synchronized with other ractors so introduce some VM locks. This patch also introduced atomic ivar cache used by set/getinlinecache instructions. To make updating ivar cache (IVC), we changed iv_index_tbl data structure to manage (ID -> entry) and an entry points serial and index. IVC points to this entry so that cache update becomes atomically. diff --git a/common.mk b/common.mk index 5c87f95..e3bf8f9 100644 --- a/common.mk +++ b/common.mk @@ -6760,6 +6760,7 @@ iseq.$(OBJEXT): $(hdrdir)/ruby.h https://github.com/ruby/ruby/blob/trunk/common.mk#L6760 iseq.$(OBJEXT): $(hdrdir)/ruby/ruby.h iseq.$(OBJEXT): $(top_srcdir)/internal/array.h iseq.$(OBJEXT): $(top_srcdir)/internal/bits.h +iseq.$(OBJEXT): $(top_srcdir)/internal/class.h iseq.$(OBJEXT): $(top_srcdir)/internal/compile.h iseq.$(OBJEXT): $(top_srcdir)/internal/compilers.h iseq.$(OBJEXT): $(top_srcdir)/internal/error.h diff --git a/compile.c b/compile.c index 0b9c276..ff63dfe 100644 --- a/compile.c +++ b/compile.c @@ -2332,6 +2332,8 @@ iseq_set_sequence(rb_iseq_t *iseq, LINK_ANCHOR *const anchor) https://github.com/ruby/ruby/blob/trunk/compile.c#L2332 ic_index, body->is_size); } generated_iseq[code_index + 1 + j] = (VALUE)ic; + + if (type == TS_IVC) FL_SET(iseqv, ISEQ_MARKABLE_ISEQ); break; } case TS_CALLDATA: diff --git a/gc.c b/gc.c index 3b84026..81d3bc2 100644 --- a/gc.c +++ b/gc.c @@ -2535,6 +2535,19 @@ rb_free_const_table(struct rb_id_table *tbl) https://github.com/ruby/ruby/blob/trunk/gc.c#L2535 rb_id_table_free(tbl); } +static int +free_iv_index_tbl_free_i(st_data_t key, st_data_t value, st_data_t data) +{ + xfree((void *)value); + return ST_CONTINUE; +} + +static void +iv_index_tbl_free(struct st_table *tbl) +{ + st_foreach(tbl, free_iv_index_tbl_free_i, 0); +} + // alive: if false, target pointers can be freed already. // To check it, we need objspace parameter. static void @@ -2756,7 +2769,7 @@ obj_free(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L2769 rb_free_const_table(RCLASS_CONST_TBL(obj)); } if (RCLASS_IV_INDEX_TBL(obj)) { - st_free_table(RCLASS_IV_INDEX_TBL(obj)); + iv_index_tbl_free(RCLASS_IV_INDEX_TBL(obj)); } if (RCLASS_EXT(obj)->subclasses) { if (BUILTIN_TYPE(obj) == T_MODULE) { @@ -4088,6 +4101,7 @@ obj_memsize_of(VALUE obj, int use_all_types) https://github.com/ruby/ruby/blob/trunk/gc.c#L4101 size += st_memsize(RCLASS_IV_TBL(obj)); } if (RCLASS_IV_INDEX_TBL(obj)) { + // TODO: more correct value size += st_memsize(RCLASS_IV_INDEX_TBL(obj)); } if (RCLASS(obj)->ptr->iv_tbl) { @@ -8543,12 +8557,26 @@ update_subclass_entries(rb_objspace_t *objspace, rb_subclass_entry_t *entry) https://github.com/ruby/ruby/blob/trunk/gc.c#L8557 } } +static int +update_iv_index_tbl_i(st_data_t key, st_data_t value, st_data_t arg) +{ + rb_objspace_t *objspace = (rb_objspace_t *)arg; + struct rb_iv_index_tbl_entry *ent = (struct rb_iv_index_tbl_entry *)value; + UPDATE_IF_MOVED(objspace, ent->class_value); + return ST_CONTINUE; +} + static void update_class_ext(rb_objspace_t *objspace, rb_classext_t *ext) { UPDATE_IF_MOVED(objspace, ext->origin_); UPDATE_IF_MOVED(objspace, ext->refined_class); update_subclass_entries(objspace, ext->subclasses); + + // ext->iv_index_tbl + if (ext->iv_index_tbl) { + st_foreach(ext->iv_index_tbl, update_iv_index_tbl_i, (st_data_t)objspace); + } } static void diff --git a/insns.def b/insns.def index f6f802f..3dd65f1 100644 --- a/insns.def +++ b/insns.def @@ -213,7 +213,7 @@ getinstancevariable https://github.com/ruby/ruby/blob/trunk/insns.def#L213 /* "instance variable not initialized" warning can be hooked. */ // attr bool leaf = false; /* has rb_warning() */ { - val = vm_getinstancevariable(GET_SELF(), id, ic); + val = vm_getinstancevariable(GET_ISEQ(), GET_SELF(), id, ic); } /* Set value of instance variable id of self to val. */ @@ -224,7 +224,7 @@ setinstancevariable https://github.com/ruby/ruby/blob/trunk/insns.def#L224 () // attr bool leaf = false; /* has rb_check_frozen_internal() */ { - vm_setinstancevariable(GET_SELF(), id, val, ic); + vm_setinstancevariable(GET_ISEQ(), GET_SELF(), id, val, ic); } /* Get value of class variable id of klass as val. */ diff --git a/internal/class.h b/internal/class.h index bb12825..eade920 100644 --- a/internal/class.h +++ b/internal/class.h @@ -25,8 +25,14 @@ struct rb_subclass_entry { https://github.com/ruby/ruby/blob/trunk/internal/class.h#L25 struct rb_subclass_entry *next; }; +struct rb_iv_index_tbl_entry { + uint32_t index; + rb_serial_t class_serial; + VALUE class_value; +}; + struct rb_classext_struct { - struct st_table *iv_index_tbl; + struct st_table *iv_index_tbl; // ID -> struct rb_iv_index_tbl_entry struct st_table *iv_tbl; #if SIZEOF_SERIAL_T == SIZEOF_VALUE /* otherwise m_tbl is in struct RClass */ struct rb_id_table *m_tbl; diff --git a/iseq.c b/iseq.c index 05a77c8..2f10cd6 100644 --- a/iseq.c +++ b/iseq.c @@ -23,6 +23,7 @@ https://github.com/ruby/ruby/blob/trunk/iseq.c#L23 #include "id_table.h" #include "internal.h" #include "internal/bits.h" +#include "internal/class.h" #include "internal/compile.h" #include "internal/error.h" #include "internal/file.h" @@ -180,6 +181,20 @@ iseq_extract_values(VALUE *code, size_t pos, iseq_value_itr_t * func, void *data https://github.com/ruby/ruby/blob/trunk/iseq.c#L181 } } break; + case TS_IVC: + { + IVC ivc = (IVC)code[pos + op_no + 1]; + if (ivc->entry) { + if (RB_TYPE_P(ivc->entry->class_value, T_NONE)) { + rb_bug("!! %u", ivc->entry->index); + } + VALUE nv = func(data, ivc->entry->class_value); + if (ivc->entry->class_value != nv) { + ivc->entry->class_value = nv; + } + } + } + break; case TS_ISE: { union iseq_inline_storage_entry *const is = (union iseq_inline_storage_entry *)code[pos + op_no + 1]; diff --git a/mjit_compile.c b/mjit_compile.c index d583773..6371acc 100644 --- a/mjit_compile.c +++ b/mjit_compile.c @@ -443,17 +443,17 @@ init_ivar_compile_status(const struct rb_iseq_constant_body *body, struct compil https://github.com/ruby/ruby/blob/trunk/mjit_compile.c#L443 if (insn == BIN(getinstancevariable) || insn == BIN(setinstancevariable)) { IVC ic = (IVC)body->iseq_encoded[pos+2]; IVC ic_copy = &(status->is_entries + ((union iseq_inline_storage_entry *)ic - body->is_entries))->iv_cache; - if (ic_copy->ic_serial) { // Only initialized (ic_serial > 0) IVCs are optimized + if (ic_copy->entry) { // Only initialized (ic_serial > 0) IVCs are optimized num_ivars++; - if (status->max_ivar_index < ic_copy->index) { - status->max_ivar_index = ic_copy->index; + if (status->max_ivar_index < ic_copy->entry->index) { + status->max_ivar_index = ic_copy->entry->index; } if (status->ivar_serial == 0) { - status->ivar_serial = ic_copy->ic_serial; + status->ivar_serial = ic_copy->entry->class_serial; } - else if (status->ivar_serial != ic_copy->ic_serial) { + else if (status->ivar_serial != ic_copy->entry->class_serial) { // Multiple classes have used this ISeq. Give up assuming one serial. status->merge_ivar_guards_p = false; return; diff --git a/st.c b/st.c index 8be466b..fe7a21c 100644 --- a/st.c +++ b/st.c @@ -2238,4 +2238,19 @@ rb_hash_bulk_insert_into_st_table(long argc, const VALUE *argv, VALUE hash) https://github.com/ruby/ruby/blob/trunk/st.c#L2238 else st_insert_generic(tab, argc, argv, hash); } + +// to iterate iv_index_tbl +st_data_t +rb_st_nth_key(st_table *tab, st_index_t index) +{ + if (LIKELY(tab->entries_start == 0 && + tab->num_entries == tab->entries_bound && + index < tab->num_entries)) { + return tab->entries[index].key; + } + else { + rb_bug("unreachable"); + } +} + #endif diff --git a/tool/ruby_vm/views/_mjit_compile_ivar.erb b/tool/ruby_vm/views/_mjit_compile_ivar.erb index eb05f4d..01d35b0 100644 --- a/tool/ruby_vm/views/_mjit_compile_ivar.erb +++ b/tool/ruby_vm/views/_mjit_compile_ivar.erb @@ -16,18 +16,18 @@ https://github.com/ruby/ruby/blob/trunk/tool/ruby_vm/views/_mjit_compile_ivar.erb#L16 % # compiler: Use copied IVC to avoid race condition IVC ic_copy = &(status->is_entries + ((union iseq_inline_storage_entry *)ic - body->is_entries))->iv_cache; % - if (!status->compile_info->disable_ivar_cache && ic_copy->ic_serial) { // Only initialized (ic_serial > 0) IVCs are optimized + if (!status->compile_info->disable_ivar_cache && ic_copy->entry) { // Only ic_copy is enabled. % # JIT: optimize away motion of sp and pc. This path does not call rb_warning() and so it's always leaf and not `handles_sp`. % # <%= render 'mjit_compile_pc_and_sp', locals: { insn: insn } -%> % % # JIT: prepare vm_getivar/vm_setivar arguments and variables fprintf(f, "{\n"); fprintf(f, " VALUE obj = GET_SELF();\n"); - fprintf(f, " const (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/