watson1978	2017-05-25 13:25:37 +0900 (Thu, 25 May 2017)

  New Revision: 58880

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

  Log:
    Improve performance of rb_equal()
    
    * object.c (rb_equal): add optimized path to compare the objects using
        rb_equal_opt(). Previously, if not same objects were given, rb_equal() would
        call `==' method via rb_funcall() which took a long time.
    
        rb_equal_opt() has provided faster comparing for Fixnum/Float/String objects.
        Now, Time#eql? uses rb_equal() to compare with argument object and it will
        be faster around 40% on 64-bit environment.
    
    * array.c (rb_ary_index): remove redundant rb_equal_opt() calling.
        Now, rb_equal() was optimized using rb_equal_opt().
        If rb_equal_opt() returns Qundef, it will invoke rb_equal() -> rb_equal_opt(),
        and it will cause the performance regression.
    
        So, this patch will remove first redundant rb_equal_opt() calling.
    
    * array.c (rb_ary_rindex): ditto.
    * array.c (rb_ary_includes): ditto.
    
        [ruby-core:80360] [Bug #13365] [Fix GH-#1552]
    
    ### Before
    Time#eql? with other      7.309M (?\194?\177 1.4%) i/s -     36.647M in   5.014964s
        Array#index(val)      1.433M (?\194?\177 1.2%) i/s -      7.207M in   5.030942s
       Array#rindex(val)      1.418M (?\194?\177 1.6%) i/s -      7.103M in   5.009164s
     Array#include?(val)      1.451M (?\194?\177 0.9%) i/s -      7.295M in   5.026392s
    
    ### After
    Time#eql? with other     10.321M (?\194?\177 1.9%) i/s -     51.684M in   5.009203s
        Array#index(val)      1.474M (?\194?\177 0.9%) i/s -      7.433M in   5.044384s
       Array#rindex(val)      1.449M (?\194?\177 1.7%) i/s -      7.292M in   5.034436s
     Array#include?(val)      1.466M (?\194?\177 1.7%) i/s -      7.373M in   5.030047s
    
    ### Test code
    require 'benchmark/ips'
    
    Benchmark.ips do |x|
      t1 = Time.now
      t2 = Time.now
    
      x.report "Time#eql? with other" do |i|
        i.times { t1.eql?(t2) }
      end
    
      # Benchmarks to check whether it didn't introduce the regression
      obj = Object.new
      x.report "Array#index(val)" do |i|
        ary = [1, 2, true, false, obj]
        i.times { ary.index(obj) }
      end
    
      x.report "Array#rindex(val)" do |i|
        ary = [1, 2, true, false, obj].reverse
        i.times { ary.rindex(obj) }
      end
    
      x.report "Array#include?(val)" do |i|
        ary = [1, 2, true, false, obj]
        i.times { ary.include?(obj) }
      end
    end

  Modified files:
    trunk/array.c
    trunk/object.c
Index: array.c
===================================================================
--- array.c	(revision 58879)
+++ array.c	(revision 58880)
@@ -1465,9 +1465,8 @@ rb_ary_fetch(int argc, VALUE *argv, VALU https://github.com/ruby/ruby/blob/trunk/array.c#L1465
 static VALUE
 rb_ary_index(int argc, VALUE *argv, VALUE ary)
 {
-    const VALUE *ptr;
     VALUE val;
-    long i, len;
+    long i;
 
     if (argc == 0) {
 	RETURN_ENUMERATOR(ary, 0, 0);
@@ -1482,20 +1481,11 @@ rb_ary_index(int argc, VALUE *argv, VALU https://github.com/ruby/ruby/blob/trunk/array.c#L1481
     val = argv[0];
     if (rb_block_given_p())
 	rb_warn("given block not used");
-    len = RARRAY_LEN(ary);
-    ptr = RARRAY_CONST_PTR(ary);
-    for (i=0; i<len; i++) {
-	VALUE e = ptr[i];
-	switch (rb_equal_opt(e, val)) {
-	  case Qundef:
-	    if (!rb_equal(e, val)) break;
-	  case Qtrue:
+    for (i=0; i<RARRAY_LEN(ary); i++) {
+	VALUE e = RARRAY_AREF(ary, i);
+	if (rb_equal(e, val)) {
 	    return LONG2NUM(i);
-	  case Qfalse:
-	    continue;
 	}
-	len = RARRAY_LEN(ary);
-	ptr = RARRAY_CONST_PTR(ary);
     }
     return Qnil;
 }
@@ -1527,7 +1517,6 @@ rb_ary_index(int argc, VALUE *argv, VALU https://github.com/ruby/ruby/blob/trunk/array.c#L1517
 static VALUE
 rb_ary_rindex(int argc, VALUE *argv, VALUE ary)
 {
-    const VALUE *ptr;
     VALUE val;
     long i = RARRAY_LEN(ary), len;
 
@@ -1546,21 +1535,11 @@ rb_ary_rindex(int argc, VALUE *argv, VAL https://github.com/ruby/ruby/blob/trunk/array.c#L1535
     val = argv[0];
     if (rb_block_given_p())
 	rb_warn("given block not used");
-    ptr = RARRAY_CONST_PTR(ary);
     while (i--) {
-	VALUE e = ptr[i];
-	switch (rb_equal_opt(e, val)) {
-	  case Qundef:
-	    if (!rb_equal(e, val)) break;
-	  case Qtrue:
+	VALUE e = RARRAY_AREF(ary, i);
+	if (rb_equal(e, val)) {
 	    return LONG2NUM(i);
-	  case Qfalse:
-	    continue;
-	}
-	if (i > (len = RARRAY_LEN(ary))) {
-	    i = len;
 	}
-	ptr = RARRAY_CONST_PTR(ary);
     }
     return Qnil;
 }
@@ -3992,11 +3971,7 @@ rb_ary_includes(VALUE ary, VALUE item) https://github.com/ruby/ruby/blob/trunk/array.c#L3971
 
     for (i=0; i<RARRAY_LEN(ary); i++) {
 	e = RARRAY_AREF(ary, i);
-	switch (rb_equal_opt(e, item)) {
-	  case Qundef:
-	    if (rb_equal(e, item)) return Qtrue;
-	    break;
-	  case Qtrue:
+	if (rb_equal(e, item)) {
 	    return Qtrue;
 	}
     }
Index: object.c
===================================================================
--- object.c	(revision 58879)
+++ object.c	(revision 58880)
@@ -88,7 +88,10 @@ rb_equal(VALUE obj1, VALUE obj2) https://github.com/ruby/ruby/blob/trunk/object.c#L88
     VALUE result;
 
     if (obj1 == obj2) return Qtrue;
-    result = rb_funcall(obj1, id_eq, 1, obj2);
+    result = rb_equal_opt(obj1, obj2);
+    if (result == Qundef) {
+	result = rb_funcall(obj1, id_eq, 1, obj2);
+    }
     if (RTEST(result)) return Qtrue;
     return Qfalse;
 }

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