Bug #13341: Improve performance of implicit type conversion

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN

[watson1978 (Shizuo Fujita)]

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

Patch

The patch is in https://github.com/ruby/ruby/pull/1537

[normalperson (Eric Wong)]

+cc ruby-core since this post was English

[e-mail] wrote:

Issue #13341 has been reported by watson1978 (Shizuo Fujita).

---

Bug #13341: Improve performance of implicit type conversion
https://bugs.ruby-lang.org/issues/13341

Interesting...

---

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)

The patch is in https://github.com/ruby/ruby/pull/1537

I use "fetch = +refs/pull/:refs/remotes/pull/" in my
.git/config to check pull/1537/head in ruby.git without using
only standard git (no proprietary JavaScript) and
looked at following commits:

e189f53a26 use rb_convert_type2() for #to_r
5e836acef6 Improve performance of implicit type conversion

So yes, I hate strcmp/strncmp, too.

If we change the API, I prefer we drastically shorten
the function args for common case types.

I'm not sure if including new APIs in ruby/intern.h is a good
idea right away, since that is technically public API.

Perhaps keep it in internal.h for now.

So, maybe:

    rb_convert_type2(a1, idTo_a);
    rb_convert_type2(a1, idTo_ary);

will lookup a small static table based on ID which fills in
T_*** (and tname string for error reporting).

Corner cases like StringIO may use old API, maybe that is
less critical.

[normalperson (Eric Wong)]

+cc ruby-core since this post was English

[e-mail] wrote:

Issue #13341 has been reported by watson1978 (Shizuo Fujita).

---

Bug #13341: Improve performance of implicit type conversion
https://bugs.ruby-lang.org/issues/13341

Interesting...

---

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)

The patch is in https://github.com/ruby/ruby/pull/1537

I use "fetch = +refs/pull/:refs/remotes/pull/" in my
.git/config to check pull/1537/head in ruby.git without using
only standard git (no proprietary JavaScript) and
looked at following commits:

e189f53a26 use rb_convert_type2() for #to_r
5e836acef6 Improve performance of implicit type conversion

So yes, I hate strcmp/strncmp, too.

If we change the API, I prefer we drastically shorten
the function args for common case types.

I'm not sure if including new APIs in ruby/intern.h is a good
idea right away, since that is technically public API.

Perhaps keep it in internal.h for now.

So, maybe:

    rb_convert_type2(a1, idTo_a);
    rb_convert_type2(a1, idTo_ary);

will lookup a small static table based on ID which fills in
T_*** (and tname string for error reporting).

Corner cases like StringIO may use old API, maybe that is
less critical.

[watson1978 (Shizuo Fujita)]

normalperson (Eric Wong) wrote:

I'm not sure if including new APIs in ruby/intern.h is a good
idea right away, since that is technically public API.

Perhaps keep it in internal.h for now.

OK, moved them at https://github.com/ruby/ruby/pull/1537/commits/cfdbfb4c0d9269df9679de8793929130219e662d

[watson1978 (Shizuo Fujita)]

• Status changed from Open to Closed

Applied in changeset trunk|r58978.

---

Improve performance of implicit type conversion

To convert the object implicitly, it has had two parts in convert_type() which are

1. lookink up the method's id
2. calling the method

Seems that strncmp() and strcmp() in convert_type() are slightly heavy to look up
the method's id for type conversion.

This patch will add and use internal APIs (rb_convert_type_with_id, rb_check_convert_type_with_id)
to call the method without looking up the method's id when convert the object.

Array#flatten -> 19 % up
Array#+ -> 3 % up

[ruby-dev:50024] [Bug #13341] [Fix GH-1537]

Before

   Array#flatten    104.119k (± 1.1%) i/s -    525.690k in   5.049517s
         Array#+      1.993M (± 1.8%) i/s -     10.010M in   5.024258s

After

   Array#flatten    124.005k (± 1.0%) i/s -    624.240k in   5.034477s
         Array#+      2.058M (± 4.8%) i/s -     10.302M in   5.019328s

Test Code

require 'benchmark/ips'

class Foo
def to_ary
[1,2,3]
end
end

Benchmark.ips do |x|

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

x.report "Array#flatten" do |i|
i.times { array.flatten }
end

x.report "Array#+" do |i|
obj = Foo.new
i.times { array + obj }
end

end