From dd723771c118da71aa58bb74537cacaec425542a Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Fri, 29 Nov 2019 03:02:44 +0900 Subject: [PATCH] fastpath for ivar read of FL_EXIVAR objects. vm_getivar() provides fastpath for T_OBJECT by caching an index of ivar. This patch also provides fastpath for FL_EXIVAR objects. FL_EXIVAR objects have an each ivar array and index can be cached as T_OBJECT. To access this ivar array, generic_iv_tbl is exposed by rb_ivar_generic_ivtbl() (declared in variable.h which is newly introduced). Benchmark script: Benchmark.driver(repeat_count: 3){|x| x.executable name: 'clean', command: %w'../clean/miniruby' x.executable name: 'trunk', command: %w'./miniruby' objs = [Object.new, 'str', {a: 1, b: 2}, [1, 2]] objs.each.with_index{|obj, i| rep = obj.inspect rep = 'Object.new' if /\#/ =~ rep x.prelude str = %Q{ v#{i} = #{rep} def v#{i}.foo @iv # ivar access method (attr_reader) end v#{i}.instance_variable_set(:@iv, :iv) } puts str x.report %Q{ v#{i}.foo } } } Result: v0.foo # T_OBJECT clean: 85387141.8 i/s trunk: 85249373.6 i/s - 1.00x slower v1.foo # T_STRING trunk: 57894407.5 i/s clean: 39957178.6 i/s - 1.45x slower v2.foo # T_HASH trunk: 56629413.2 i/s clean: 39227088.9 i/s - 1.44x slower v3.foo # T_ARRAY trunk: 55797530.2 i/s clean: 38263572.9 i/s - 1.46x slower --- common.mk | 2 + test/ruby/test_variable.rb | 19 +++++++ variable.c | 13 ++--- variable.h | 9 ++++ vm_insnhelper.c | 106 ++++++++++++++++++++++++++----------- 5 files changed, 113 insertions(+), 36 deletions(-) create mode 100644 variable.h diff --git a/common.mk b/common.mk index dceeeabd48..55f3411046 100644 --- a/common.mk +++ b/common.mk @@ -3156,6 +3156,7 @@ variable.$(OBJEXT): {$(VPATH)}thread_native.h variable.$(OBJEXT): {$(VPATH)}transient_heap.h variable.$(OBJEXT): {$(VPATH)}util.h variable.$(OBJEXT): {$(VPATH)}variable.c +variable.$(OBJEXT): {$(VPATH)}variable.h variable.$(OBJEXT): {$(VPATH)}vm_core.h variable.$(OBJEXT): {$(VPATH)}vm_opts.h version.$(OBJEXT): $(CCAN_DIR)/check_type/check_type.h @@ -3223,6 +3224,7 @@ vm.$(OBJEXT): {$(VPATH)}st.h vm.$(OBJEXT): {$(VPATH)}subst.h vm.$(OBJEXT): {$(VPATH)}thread_$(THREAD_MODEL).h vm.$(OBJEXT): {$(VPATH)}thread_native.h +vm.$(OBJEXT): {$(VPATH)}variable.h vm.$(OBJEXT): {$(VPATH)}vm.c vm.$(OBJEXT): {$(VPATH)}vm.h vm.$(OBJEXT): {$(VPATH)}vm.inc diff --git a/test/ruby/test_variable.rb b/test/ruby/test_variable.rb index f14b4019df..0772066143 100644 --- a/test/ruby/test_variable.rb +++ b/test/ruby/test_variable.rb @@ -178,6 +178,25 @@ class TestVariable < Test::Unit::TestCase end end + class ExIvar < Hash + def initialize + @a = 1 + @b = 2 + @c = 3 + end + + def ivars + [@a, @b, @c] + end + end + + def test_external_ivars + 3.times{ + # check inline cache for external ivar access + assert_equal [1, 2, 3], ExIvar.new.ivars + } + end + def test_local_variables_with_kwarg bug11674 = '[ruby-core:71437] [Bug #11674]' v = with_kwargs_11(v1:1,v2:2,v3:3,v4:4,v5:5,v6:6,v7:7,v8:8,v9:9,v10:10,v11:11) diff --git a/variable.c b/variable.c index 46573523b4..d6e2f2a601 100644 --- a/variable.c +++ b/variable.c @@ -23,6 +23,7 @@ #include "debug_counter.h" #include "vm_core.h" #include "transient_heap.h" +#include "variable.h" static struct rb_id_table *rb_global_tbl; static ID autoload, classpath, tmp_classpath; @@ -34,12 +35,6 @@ static VALUE rb_const_search(VALUE klass, ID id, int exclude, int recurse, int v static st_table *generic_iv_tbl; static st_table *generic_iv_tbl_compat; -/* per-object */ -struct gen_ivtbl { - uint32_t numiv; - VALUE ivptr[FLEX_ARY_LEN]; -}; - struct ivar_update { union { st_table *iv_index_tbl; @@ -804,6 +799,12 @@ gen_ivtbl_get(VALUE obj, struct gen_ivtbl **ivtbl) return 0; } +struct st_table * +rb_ivar_generic_ivtbl(void) +{ + return generic_iv_tbl; +} + static VALUE generic_ivar_delete(VALUE obj, ID id, VALUE undef) { diff --git a/variable.h b/variable.h new file mode 100644 index 0000000000..67fe480149 --- /dev/null +++ b/variable.h @@ -0,0 +1,9 @@ + +/* per-object */ + +struct gen_ivtbl { + uint32_t numiv; + VALUE ivptr[FLEX_ARY_LEN]; +}; + +struct st_table *rb_ivar_generic_ivtbl(void); diff --git a/vm_insnhelper.c b/vm_insnhelper.c index a99282891b..94fc7c72bd 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -18,6 +18,7 @@ #include "internal.h" #include "ruby/config.h" #include "debug_counter.h" +#include "variable.h" extern rb_method_definition_t *rb_method_definition_create(rb_method_type_t type, ID mid); extern void rb_method_definition_set(const rb_method_entry_t *me, rb_method_definition_t *def, void *opts); @@ -1011,26 +1012,48 @@ static inline VALUE vm_getivar(VALUE obj, ID id, IC ic, struct rb_call_cache *cc, int is_attr) { #if OPT_IC_FOR_IVAR - if (LIKELY(RB_TYPE_P(obj, T_OBJECT))) { - VALUE val = Qundef; - if (LIKELY(is_attr ? - RB_DEBUG_COUNTER_INC_UNLESS(ivar_get_ic_miss_unset, cc->aux.index > 0) : - RB_DEBUG_COUNTER_INC_UNLESS(ivar_get_ic_miss_serial, - ic->ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass)))) { - st_index_t index = !is_attr ? ic->ic_value.index : (cc->aux.index - 1); - if (LIKELY(index < ROBJECT_NUMIV(obj))) { - val = ROBJECT_IVPTR(obj)[index]; - } - } - else { - st_data_t index; - struct st_table *iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj); + VALUE val = Qundef; + if (SPECIAL_CONST_P(obj)) { + // frozen? + } + else if (LIKELY(is_attr ? + RB_DEBUG_COUNTER_INC_UNLESS(ivar_get_ic_miss_unset, cc->aux.index > 0) : + RB_DEBUG_COUNTER_INC_UNLESS(ivar_get_ic_miss_serial, + ic->ic_serial == RCLASS_SERIAL(RBASIC(obj)->klass)))) { + st_index_t index = !is_attr ? ic->ic_value.index : (cc->aux.index - 1); + + RB_DEBUG_COUNTER_INC(ivar_get_ic_hit); + + if (LIKELY(BUILTIN_TYPE(obj) == T_OBJECT) && + LIKELY(index < ROBJECT_NUMIV(obj))) { + val = ROBJECT_IVPTR(obj)[index]; + } + else if (FL_TEST_RAW(obj, FL_EXIVAR)) { + struct gen_ivtbl *ivtbl; + + if (LIKELY(st_lookup(rb_ivar_generic_ivtbl(), (st_data_t)obj, (st_data_t *)&ivtbl)) && + LIKELY(index < ivtbl->numiv)) { + val = ivtbl->ivptr[index]; + } + } + goto ret; + } + else { + struct st_table *iv_index_tbl; + st_index_t numiv; + VALUE *ivptr; + + st_data_t index; + + if (BUILTIN_TYPE(obj) == T_OBJECT) { + iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj); + numiv = ROBJECT_NUMIV(obj); + ivptr = ROBJECT_IVPTR(obj); + + fill: if (iv_index_tbl) { if (st_lookup(iv_index_tbl, id, &index)) { - if (index < ROBJECT_NUMIV(obj)) { - val = ROBJECT_IVPTR(obj)[index]; - } if (!is_attr) { ic->ic_value.index = index; ic->ic_serial = RCLASS_SERIAL(RBASIC(obj)->klass); @@ -1038,26 +1061,49 @@ vm_getivar(VALUE obj, ID id, IC ic, struct rb_call_cache *cc, int is_attr) else { /* call_info */ cc->aux.index = (int)index + 1; } + + if (index < numiv) { + val = ivptr[index]; + } } } } - if (UNLIKELY(val == Qundef)) { - if (!is_attr && RTEST(ruby_verbose)) - rb_warning("instance variable %"PRIsVALUE" not initialized", QUOTE_ID(id)); - val = Qnil; - } - RB_DEBUG_COUNTER_INC(ivar_get_ic_hit); - return val; - } - else { - RB_DEBUG_COUNTER_INC(ivar_get_ic_miss_noobject); + else if (FL_TEST_RAW(obj, FL_EXIVAR)) { + struct gen_ivtbl *ivtbl; + + if (LIKELY(st_lookup(rb_ivar_generic_ivtbl(), (st_data_t)obj, (st_data_t *)&ivtbl))) { + numiv = ivtbl->numiv; + ivptr = ivtbl->ivptr; + iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj)); + goto fill; + } + } + else { + // T_CLASS / T_MODULE + goto general_path; + } + + ret: + if (LIKELY(val != Qundef)) { + return val; + } + else { + if (!is_attr && RTEST(ruby_verbose)) { + rb_warning("instance variable %"PRIsVALUE" not initialized", QUOTE_ID(id)); + } + return Qnil; + } } + general_path: #endif /* OPT_IC_FOR_IVAR */ RB_DEBUG_COUNTER_INC(ivar_get_ic_miss); - if (is_attr) - return rb_attr_get(obj, id); - return rb_ivar_get(obj, id); + if (is_attr) { + return rb_attr_get(obj, id); + } + else { + return rb_ivar_get(obj, id); + } } static inline VALUE