Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of implicit type conversion #1537

Closed
wants to merge 5 commits into from

Conversation

Watson1978
Copy link
Contributor

@Watson1978 Watson1978 commented Mar 13, 2017

At least, Array#flatten will be faster around 20%.
Seems that strncmp() & strcmp() in convert_type() are slightly heavy to look up the method's id for type conversion.
(

ruby/object.c

Lines 2634 to 2643 in 4f2db15

if (strncmp(prefix, method, sizeof(prefix)-1) == 0) {
const char *const meth = &method[sizeof(prefix)-1];
for (i=0; i < numberof(conv_method_names); i++) {
if (conv_method_names[i].method[0] == meth[0] &&
strcmp(conv_method_names[i].method, meth) == 0) {
m = conv_method_names[i].id;
break;
}
}
}
)

This patch will use known method's id directly.

  • Before
                                             user     system      total        real
Array#flatten (rb_check_convert_type2)   1.000000   0.000000   1.000000 (  1.001917)
Array#+ (rb_convert_type2)               1.010000   0.000000   1.010000 (  1.006383)
  • After
                                             user     system      total        real
Array#flatten (rb_check_convert_type2)   0.830000   0.000000   0.830000 (  0.833411)
Array#+ (rb_convert_type2)               0.950000   0.000000   0.950000 (  0.953832)
  • Test Code
require 'benchmark'

Benchmark.bmbm do |x|

  ary = []
  100.times { |i| ary << i }
  array = [ary]

  x.report "Array#flatten (rb_check_convert_type2)"do
    100000.times do
      array.flatten
    end
  end

  x.report "Array#+ (rb_convert_type2)"do
    class Foo
      def to_ary
        [1,2,3]
      end
    end
    obj = Foo.new

    2000000.times do
      array + obj
    end
  end

end

https://bugs.ruby-lang.org/issues/13341

Copy link
Member

@nurse nurse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but rb_convert_type2() is bad name; use rb_convert_type_with_id or something.
(I know you follow CRuby's old style, but we now consider it is bad style)

@Watson1978 Watson1978 force-pushed the type_convert branch 2 times, most recently from 30dae7c to 3266102 Compare May 31, 2017 06:32
At least, Array#flatten will be faster around 20%.
Seems that strncmp() & strcmp() in convert_type() are slightly heavy to look up the method's id for type conversion.
(https://github.com/ruby/ruby/blob/4f2db15b42d7b8eb5b304a92ba2296632dba3edf/object.c#L2634-L2643)

This patch will use known method's id directly.

* Before
                                             user     system      total        real
Array#flatten (rb_check_convert_type2)   1.000000   0.000000   1.000000 (  1.001917)
Array#+ (rb_convert_type2)               1.010000   0.000000   1.010000 (  1.006383)

* After
                                             user     system      total        real
Array#flatten (rb_check_convert_type2)   0.830000   0.000000   0.830000 (  0.833411)
Array#+ (rb_convert_type2)               0.950000   0.000000   0.950000 (  0.953832)

* Test Code
require 'benchmark'

Benchmark.bmbm do |x|

  ary = []
  100.times { |i| ary << i }
  array = [ary]

  x.report "Array#flatten (rb_check_convert_type2)"do
    100000.times do
      array.flatten
    end
  end

  x.report "Array#+ (rb_convert_type2)"do
    class Foo
      def to_ary
        [1,2,3]
      end
    end
    obj = Foo.new

    2000000.times do
      array + obj
    end
  end

end
@Watson1978
Copy link
Contributor Author

Rebased to HEAD (cc50ed4) in trunk branch

@hsbt hsbt closed this in d0015e4 May 31, 2017
@Watson1978 Watson1978 deleted the type_convert branch October 14, 2017 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants