[前][次][番号順一覧][スレッド一覧]

ruby-changes:38599

From: normal <ko1@a...>
Date: Sat, 30 May 2015 09:20:34 +0900 (JST)
Subject: [ruby-changes:38599] normal:r50680 (trunk): variable.c: avoid compatibility table with generic ivars

normal	2015-05-30 09:20:15 +0900 (Sat, 30 May 2015)

  New Revision: 50680

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=50680

  Log:
    variable.c: avoid compatibility table with generic ivars
    
    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
    
    * include/ruby/intern.h (rb_generic_ivar_table): deprecate
    * internal.h (rb_attr_delete): declare
    * marshal.c (has_ivars): use rb_ivar_foreach
      (w_ivar): ditto
      (w_object): update for new interface
    * time.c (time_mload): use rb_attr_delete
    * variable.c (generic_ivar_delete): implement
      (rb_ivar_delete): ditto
      (rb_attr_delete): ditto
      [ruby-core:69323] [Feature #11170]

  Added files:
    trunk/benchmark/bm_marshal_dump_load_geniv.rb
    trunk/benchmark/bm_marshal_dump_load_time.rb
  Modified files:
    trunk/ChangeLog
    trunk/include/ruby/intern.h
    trunk/internal.h
    trunk/marshal.c
    trunk/time.c
    trunk/variable.c
Index: time.c
===================================================================
--- time.c	(revision 50679)
+++ time.c	(revision 50680)
@@ -4735,16 +4735,13 @@ time_mload(VALUE time, VALUE str) https://github.com/ruby/ruby/blob/trunk/time.c#L4735
     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, {});
Index: include/ruby/intern.h
===================================================================
--- include/ruby/intern.h	(revision 50679)
+++ include/ruby/intern.h	(revision 50680)
@@ -936,7 +936,7 @@ VALUE rb_f_trace_var(int, const VALUE*); https://github.com/ruby/ruby/blob/trunk/include/ruby/intern.h#L936
 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);
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 50679)
+++ ChangeLog	(revision 50680)
@@ -1,3 +1,16 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1
+Sat May 30 09:02:51 2015  Eric Wong  <e@8...>
+
+	* include/ruby/intern.h (rb_generic_ivar_table): deprecate
+	* internal.h (rb_attr_delete): declare
+	* marshal.c (has_ivars): use rb_ivar_foreach
+	  (w_ivar): ditto
+	  (w_object): update for new interface
+	* time.c (time_mload): use rb_attr_delete
+	* variable.c (generic_ivar_delete): implement
+	  (rb_ivar_delete): ditto
+	  (rb_attr_delete): ditto
+	  [ruby-core:69323] [Feature #11170]
+
 Sat May 30 09:14:28 2015  Scott Francis  <scott.francis@s...>
 
 	* cont.c (cont_free): check if ruby_current_thread is still valid.
Index: variable.c
===================================================================
--- variable.c	(revision 50679)
+++ variable.c	(revision 50680)
@@ -1013,6 +1013,27 @@ rb_generic_ivar_table(VALUE obj) https://github.com/ruby/ruby/blob/trunk/variable.c#L1013
 }
 
 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) https://github.com/ruby/ruby/blob/trunk/variable.c#L1295
     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)
 {
Index: internal.h
===================================================================
--- internal.h	(revision 50679)
+++ internal.h	(revision 50680)
@@ -1142,6 +1142,7 @@ extern rb_encoding OnigEncodingUTF_8; https://github.com/ruby/ruby/blob/trunk/internal.h#L1142
 /* 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;
Index: benchmark/bm_marshal_dump_load_geniv.rb
===================================================================
--- benchmark/bm_marshal_dump_load_geniv.rb	(revision 0)
+++ benchmark/bm_marshal_dump_load_geniv.rb	(revision 50680)
@@ -0,0 +1,10 @@ https://github.com/ruby/ruby/blob/trunk/benchmark/bm_marshal_dump_load_geniv.rb#L1
+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 })
Index: benchmark/bm_marshal_dump_load_time.rb
===================================================================
--- benchmark/bm_marshal_dump_load_time.rb	(revision 0)
+++ benchmark/bm_marshal_dump_load_time.rb	(revision 50680)
@@ -0,0 +1 @@
+100000.times { Marshal.load(Marshal.dump(Time.now)) }
Index: marshal.c
===================================================================
--- marshal.c	(revision 50679)
+++ marshal.c	(revision 50680)
@@ -594,24 +594,33 @@ w_encoding(VALUE encname, struct dump_ca https://github.com/ruby/ruby/blob/trunk/marshal.c#L594
 }
 
 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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L647
 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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L717
 	    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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L727
 	    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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L746
 
         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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L760
                     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 https://github.com/ruby/ruby/blob/trunk/marshal.c#L920
 	RB_GC_GUARD(obj);
     }
     if (hasiv) {
-	w_ivar(hasiv, ivtbl, encname, &c_arg);
+	w_ivar(hasiv, ivobj, encname, &c_arg);
     }
 }
 

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]