From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS46562 107.181.174.0/24 X-Spam-Status: No, score=-2.1 required=3.0 tests=AWL,BAYES_00, RCVD_IN_DNSWL_BLOCKED,RCVD_IN_XBL shortcircuit=no autolearn=no version=3.3.2 X-Original-To: spew@80x24.org Received: from 80x24.org (us.tor-exit.neelc.org [107.181.174.84]) by dcvr.yhbt.net (Postfix) with ESMTP id 0B5DF1F508 for ; Fri, 29 May 2015 00:39:16 +0000 (UTC) From: Eric Wong To: spew@80x24.org Subject: [PATCH] avoid compatibility table with generic ivars Date: Fri, 29 May 2015 00:39:04 +0000 Message-Id: <1432859944-14374-1-git-send-email-e@80x24.org> List-Id: This recovers and improves performance of Marshal.dump/load on Time objects compared to when we implemented generic ivars entirely using st_table. This also recovers some performance on other generic ivar objects, but does not bring bring Marshal.dump/load performance up to previous speeds. benchmark results: minimum results in each 10 measurements. Execution time (sec) name trunk geniv after marshal_dump_flo 0.343 0.334 0.335 marshal_dump_load_geniv 0.487 0.527 0.495 marshal_dump_load_time 1.262 1.401 1.257 Speedup ratio: compare with the result of `trunk' (greater is better) name geniv after marshal_dump_flo 1.026 1.023 marshal_dump_load_geniv 0.925 0.985 marshal_dump_load_time 0.901 1.004 --- benchmark/bm_marshal_dump_load_geniv.rb | 10 ++++++ benchmark/bm_marshal_dump_load_time.rb | 1 + include/ruby/intern.h | 2 +- internal.h | 1 + marshal.c | 45 +++++++++++++---------- time.c | 5 +-- variable.c | 63 +++++++++++++++++++++++++++++++++ 7 files changed, 104 insertions(+), 23 deletions(-) create mode 100644 benchmark/bm_marshal_dump_load_geniv.rb create mode 100644 benchmark/bm_marshal_dump_load_time.rb diff --git a/benchmark/bm_marshal_dump_load_geniv.rb b/benchmark/bm_marshal_dump_load_geniv.rb new file mode 100644 index 0000000..8252ad9 --- /dev/null +++ b/benchmark/bm_marshal_dump_load_geniv.rb @@ -0,0 +1,10 @@ +a = '' +a.instance_eval do + @a = :a + @b = :b + @c = :c +end +100000.times do + a = Marshal.load(Marshal.dump(a)) +end +#p(a.instance_eval { @a == :a && @b == :b && @c == :c }) diff --git a/benchmark/bm_marshal_dump_load_time.rb b/benchmark/bm_marshal_dump_load_time.rb new file mode 100644 index 0000000..e29743b --- /dev/null +++ b/benchmark/bm_marshal_dump_load_time.rb @@ -0,0 +1 @@ +100000.times { Marshal.load(Marshal.dump(Time.now)) } diff --git a/include/ruby/intern.h b/include/ruby/intern.h index f946e7e..9e26c8b 100644 --- a/include/ruby/intern.h +++ b/include/ruby/intern.h @@ -936,7 +936,7 @@ VALUE rb_f_trace_var(int, const VALUE*); VALUE rb_f_untrace_var(int, const VALUE*); VALUE rb_f_global_variables(void); void rb_alias_variable(ID, ID); -struct st_table* rb_generic_ivar_table(VALUE); +DEPRECATED(struct st_table* rb_generic_ivar_table(VALUE)); void rb_copy_generic_ivar(VALUE,VALUE); void rb_free_generic_ivar(VALUE); VALUE rb_ivar_get(VALUE, ID); diff --git a/internal.h b/internal.h index c68d42e..2c37697 100644 --- a/internal.h +++ b/internal.h @@ -1142,6 +1142,7 @@ extern rb_encoding OnigEncodingUTF_8; /* variable.c */ size_t rb_generic_ivar_memsize(VALUE); VALUE rb_search_class_path(VALUE); +VALUE rb_attr_delete(VALUE, ID); /* version.c */ extern VALUE ruby_engine_name; diff --git a/marshal.c b/marshal.c index 8eb7b07..fc33347 100644 --- a/marshal.c +++ b/marshal.c @@ -594,24 +594,33 @@ w_encoding(VALUE encname, struct dump_call_arg *arg) } static st_index_t -has_ivars(VALUE obj, VALUE encname, st_table **ivtbl) +has_ivars(VALUE obj, VALUE encname, VALUE *ivobj) { - st_index_t num = !NIL_P(encname); + st_index_t enc = !NIL_P(encname); + st_index_t num = 0; - *ivtbl = rb_generic_ivar_table(obj); - if (*ivtbl) { - st_foreach_safe(*ivtbl, obj_count_ivars, (st_data_t)&num); + if (SPECIAL_CONST_P(obj)) goto generic; + switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + case T_CLASS: + case T_MODULE: + break; /* counted elsewhere */ + default: + generic: + rb_ivar_foreach(obj, obj_count_ivars, (st_data_t)&num); + if (num) *ivobj = obj; } - return num; + + return num + enc; } static void -w_ivar(st_index_t num, st_table *tbl, VALUE encname, struct dump_call_arg *arg) +w_ivar(st_index_t num, VALUE ivobj, VALUE encname, struct dump_call_arg *arg) { w_long(num, arg->arg); w_encoding(encname, arg); - if (tbl) { - st_foreach_safe(tbl, w_obj_each, (st_data_t)arg); + if (ivobj != Qundef) { + rb_ivar_foreach(ivobj, w_obj_each, (st_data_t)arg); } } @@ -638,7 +647,7 @@ static void w_object(VALUE obj, struct dump_arg *arg, int limit) { struct dump_call_arg c_arg; - st_table *ivtbl = 0; + VALUE ivobj = Qundef; st_data_t num; st_index_t hasiv = 0; VALUE encname = Qnil; @@ -708,7 +717,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) return; } if (rb_obj_respond_to(obj, s_dump, TRUE)) { - st_table *ivtbl2 = 0; + VALUE ivobj2 = Qundef; st_index_t hasiv2; VALUE encname2; @@ -718,18 +727,18 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) if (!RB_TYPE_P(v, T_STRING)) { rb_raise(rb_eTypeError, "_dump() must return string"); } - hasiv = has_ivars(obj, (encname = encoding_name(obj, arg)), &ivtbl); - hasiv2 = has_ivars(v, (encname2 = encoding_name(v, arg)), &ivtbl2); + hasiv = has_ivars(obj, (encname = encoding_name(obj, arg)), &ivobj); + hasiv2 = has_ivars(v, (encname2 = encoding_name(v, arg)), &ivobj2); if (hasiv2) { hasiv = hasiv2; - ivtbl = ivtbl2; + ivobj = ivobj2; encname = encname2; } if (hasiv) w_byte(TYPE_IVAR, arg); w_class(TYPE_USERDEF, obj, arg, FALSE); w_bytes(RSTRING_PTR(v), RSTRING_LEN(v), arg); if (hasiv) { - w_ivar(hasiv, ivtbl, encname, &c_arg); + w_ivar(hasiv, ivobj, encname, &c_arg); } st_add_direct(arg->data, obj, arg->data->num_entries); return; @@ -737,7 +746,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) st_add_direct(arg->data, obj, arg->data->num_entries); - hasiv = has_ivars(obj, (encname = encoding_name(obj, arg)), &ivtbl); + hasiv = has_ivars(obj, (encname = encoding_name(obj, arg)), &ivobj); { st_data_t compat_data; rb_alloc_func_t allocator = rb_get_alloc_func(RBASIC(obj)->klass); @@ -751,7 +760,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) arg->compat_tbl = rb_init_identtable(); } st_insert(arg->compat_tbl, (st_data_t)obj, (st_data_t)real_obj); - if (obj != real_obj && !ivtbl) hasiv = 0; + if (obj != real_obj && ivobj == Qundef) hasiv = 0; } } if (hasiv) w_byte(TYPE_IVAR, arg); @@ -911,7 +920,7 @@ w_object(VALUE obj, struct dump_arg *arg, int limit) RB_GC_GUARD(obj); } if (hasiv) { - w_ivar(hasiv, ivtbl, encname, &c_arg); + w_ivar(hasiv, ivobj, encname, &c_arg); } } diff --git a/time.c b/time.c index 382dc3f..0d696ba 100644 --- a/time.c +++ b/time.c @@ -4735,16 +4735,13 @@ time_mload(VALUE time, VALUE str) long nsec; VALUE submicro, nano_num, nano_den, offset, zone; wideval_t timew; - st_data_t data; time_modify(time); #define get_attr(attr, iffound) \ - attr = rb_attr_get(str, id_##attr); \ + attr = rb_attr_delete(str, id_##attr); \ if (!NIL_P(attr)) { \ - data = id_##attr; \ iffound; \ - st_delete(rb_generic_ivar_table(str), &data, 0); \ } get_attr(nano_num, {}); diff --git a/variable.c b/variable.c index 62c7d6e..0d257f4 100644 --- a/variable.c +++ b/variable.c @@ -1013,6 +1013,27 @@ rb_generic_ivar_table(VALUE obj) } static VALUE +generic_ivar_delete(VALUE obj, ID id, VALUE undef) +{ + struct gen_ivtbl *ivtbl; + + if (gen_ivtbl_get(obj, &ivtbl)) { + st_table *iv_index_tbl = RCLASS_IV_INDEX_TBL(rb_obj_class(obj)); + st_data_t index; + + if (st_lookup(iv_index_tbl, (st_data_t)id, &index)) { + if ((long)index < ivtbl->numiv) { + VALUE ret = ivtbl->ivptr[index]; + + ivtbl->ivptr[index] = Qundef; + return ret == Qundef ? undef : ret; + } + } + } + return undef; +} + +static VALUE generic_ivar_get(VALUE obj, ID id, VALUE undef) { struct gen_ivtbl *ivtbl; @@ -1274,6 +1295,48 @@ rb_attr_get(VALUE obj, ID id) return rb_ivar_lookup(obj, id, Qnil); } +static VALUE +rb_ivar_delete(VALUE obj, ID id, VALUE undef) +{ + VALUE val, *ptr; + struct st_table *iv_index_tbl; + long len; + st_data_t index; + + if (SPECIAL_CONST_P(obj)) goto generic; + switch (BUILTIN_TYPE(obj)) { + case T_OBJECT: + len = ROBJECT_NUMIV(obj); + ptr = ROBJECT_IVPTR(obj); + iv_index_tbl = ROBJECT_IV_INDEX_TBL(obj); + if (!iv_index_tbl) break; + if (!st_lookup(iv_index_tbl, (st_data_t)id, &index)) break; + if (len <= (long)index) break; + val = ptr[index]; + ptr[index] = Qundef; + if (val != Qundef) + return val; + break; + case T_CLASS: + case T_MODULE: + if (RCLASS_IV_TBL(obj) && st_delete(RCLASS_IV_TBL(obj), (st_data_t *)&id, &index)) + return (VALUE)index; + break; + default: + generic: + if (FL_TEST(obj, FL_EXIVAR) || rb_special_const_p(obj)) + return generic_ivar_delete(obj, id, undef); + break; + } + return undef; +} + +VALUE +rb_attr_delete(VALUE obj, ID id) +{ + return rb_ivar_delete(obj, id, Qnil); +} + static st_table * iv_index_tbl_make(VALUE obj) { -- EW