ruby-dev (Japanese) list archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-dev:50024] [Ruby trunk Bug#13341] Improve performance of implicit type conversion
       [not found] <redmine.issue-13341.20170321004512@ruby-lang.org>
@ 2017-03-21  0:45 ` watson1978
  2017-04-15  7:49   ` [ruby-dev:50080] " Eric Wong
  2017-04-15 10:12 ` [ruby-dev:50081] " watson1978
  1 sibling, 1 reply; 3+ messages in thread
From: watson1978 @ 2017-03-21  0:45 UTC (permalink / raw
  To: ruby-dev

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

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

* Author: watson1978 (Shizuo Fujita)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
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



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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [ruby-dev:50080] Re: [Ruby trunk Bug#13341] Improve performance of implicit type conversion
  2017-03-21  0:45 ` [ruby-dev:50024] [Ruby trunk Bug#13341] Improve performance of implicit type conversion watson1978
@ 2017-04-15  7:49   ` Eric Wong
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Wong @ 2017-04-15  7:49 UTC (permalink / raw
  To: Ruby developers (Japanese); +Cc: ruby-core

+cc ruby-core since this post was English

watson1978@gmail•com 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)
> ~~~

<snip>

> 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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [ruby-dev:50081] [Ruby trunk Bug#13341] Improve performance of implicit type conversion
       [not found] <redmine.issue-13341.20170321004512@ruby-lang.org>
  2017-03-21  0:45 ` [ruby-dev:50024] [Ruby trunk Bug#13341] Improve performance of implicit type conversion watson1978
@ 2017-04-15 10:12 ` watson1978
  1 sibling, 0 replies; 3+ messages in thread
From: watson1978 @ 2017-04-15 10:12 UTC (permalink / raw
  To: ruby-dev

Issue #13341 has been updated by 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

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

* Author: watson1978 (Shizuo Fujita)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN
----------------------------------------
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



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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-04-15  9:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <redmine.issue-13341.20170321004512@ruby-lang.org>
2017-03-21  0:45 ` [ruby-dev:50024] [Ruby trunk Bug#13341] Improve performance of implicit type conversion watson1978
2017-04-15  7:49   ` [ruby-dev:50080] " Eric Wong
2017-04-15 10:12 ` [ruby-dev:50081] " watson1978

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).