ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:88555] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
@ 2018-08-19 16:25 ` chopraanmol1
  2018-08-20  5:13 ` [ruby-core:88561] " mame
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-19 16:25 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been reported by chopraanmol1 (Anmol Chopra).

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                       user     system      total        real
benchmark_method   0.408000   0.000000   0.408000 (  0.405603)
rest_method        0.992000   0.000000   0.992000 (  0.992706)
lead_method        1.000000   0.000000   1.000000 (  0.999311)
post_method        2.464000   0.000000   2.464000 (  2.464712)
lead_post_method   1.800000   0.000000   1.800000 (  1.800882)
~~~

modified
~~~
                       user     system      total        real
benchmark_method   0.400000   0.000000   0.400000 (  0.401134)
rest_method        0.740000   0.000000   0.740000 (  0.741038)
lead_method        0.748000   0.000000   0.748000 (  0.746265)
post_method        1.992000   0.000000   1.992000 (  1.992200)
lead_post_method   1.632000   0.000000   1.632000 (  1.631994)
~~~

---Files--------------------------------
bench_method_arg.rb (774 Bytes)
improve_rest_parameters_setup.patch (1.76 KB)


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

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

* [ruby-core:88561] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
  2018-08-19 16:25 ` [ruby-core:88555] [Ruby trunk Feature#15010] Reduce allocation for rest parameters chopraanmol1
@ 2018-08-20  5:13 ` mame
  2018-08-20  6:29 ` [ruby-core:88564] " chopraanmol1
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: mame @ 2018-08-20  5:13 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by mame (Yusuke Endoh).


Looks good to me.  Though destructive operation to the rest array may make the source code unclear, performance is more important in this case, I think.

Some other functions in vm_args.c also use rb_ary_dup.  There may be more room to optimize.

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73618

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                       user     system      total        real
benchmark_method   0.408000   0.000000   0.408000 (  0.405603)
rest_method        0.992000   0.000000   0.992000 (  0.992706)
lead_method        1.000000   0.000000   1.000000 (  0.999311)
post_method        2.464000   0.000000   2.464000 (  2.464712)
lead_post_method   1.800000   0.000000   1.800000 (  1.800882)
~~~

modified
~~~
                       user     system      total        real
benchmark_method   0.400000   0.000000   0.400000 (  0.401134)
rest_method        0.740000   0.000000   0.740000 (  0.741038)
lead_method        0.748000   0.000000   0.748000 (  0.746265)
post_method        1.992000   0.000000   1.992000 (  1.992200)
lead_post_method   1.632000   0.000000   1.632000 (  1.631994)
~~~

---Files--------------------------------
bench_method_arg.rb (774 Bytes)
improve_rest_parameters_setup.patch (1.76 KB)


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

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

* [ruby-core:88564] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
  2018-08-19 16:25 ` [ruby-core:88555] [Ruby trunk Feature#15010] Reduce allocation for rest parameters chopraanmol1
  2018-08-20  5:13 ` [ruby-core:88561] " mame
@ 2018-08-20  6:29 ` chopraanmol1
  2018-08-20  6:32   ` [ruby-core:88565] " Eric Wong
  2018-08-20 10:27 ` [ruby-core:88568] " chopraanmol1
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 21+ messages in thread
From: chopraanmol1 @ 2018-08-20  6:29 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


mame (Yusuke Endoh) wrote:
> Some other functions in vm_args.c also use rb_ary_dup.  There may be more room to optimize.

Yes, it can be further optimized for keyword argument and argument setup for the block. I'll modify the patch in a day or two. 

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73620

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                       user     system      total        real
benchmark_method   0.408000   0.000000   0.408000 (  0.405603)
rest_method        0.992000   0.000000   0.992000 (  0.992706)
lead_method        1.000000   0.000000   1.000000 (  0.999311)
post_method        2.464000   0.000000   2.464000 (  2.464712)
lead_post_method   1.800000   0.000000   1.800000 (  1.800882)
~~~

modified
~~~
                       user     system      total        real
benchmark_method   0.400000   0.000000   0.400000 (  0.401134)
rest_method        0.740000   0.000000   0.740000 (  0.741038)
lead_method        0.748000   0.000000   0.748000 (  0.746265)
post_method        1.992000   0.000000   1.992000 (  1.992200)
lead_post_method   1.632000   0.000000   1.632000 (  1.631994)
~~~

---Files--------------------------------
bench_method_arg.rb (774 Bytes)
improve_rest_parameters_setup.patch (1.76 KB)


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

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

* [ruby-core:88565] Re: [Ruby trunk Feature#15010] Reduce allocation for rest parameters
  2018-08-20  6:29 ` [ruby-core:88564] " chopraanmol1
@ 2018-08-20  6:32   ` Eric Wong
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Wong @ 2018-08-20  6:32 UTC (permalink / raw
  To: ruby-core

chopraanmol1@gmail.com wrote:
> Yes, it can be further optimized for keyword argument and argument setup for the block. I'll modify the patch in a day or two. 

Cool!  It's probably worth implementing something like
rb_ary_shift_m (but without the return value) to avoid looping
on rb_ary_shift.

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

* [ruby-core:88568] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (2 preceding siblings ...)
  2018-08-20  6:29 ` [ruby-core:88564] " chopraanmol1
@ 2018-08-20 10:27 ` chopraanmol1
  2018-08-20 19:00   ` [ruby-core:88575] " Eric Wong
  2018-08-20 22:15 ` [ruby-core:88579] " mame
                   ` (14 subsequent siblings)
  18 siblings, 1 reply; 21+ messages in thread
From: chopraanmol1 @ 2018-08-20 10:27 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


normalperson (Eric Wong) wrote:
>  Cool!  It's probably worth implementing something like
>  rb_ary_shift_m (but without the return value) to avoid looping
>  on rb_ary_shift.

Added rb_ary_clear_m (suggestion for a better name will be appreciated) with suggested changes.


mame (Yusuke Endoh) wrote:
> Some other functions in vm_args.c also use rb_ary_dup.  There may be more room to optimize.

Modified patch to ensure rb_ary_dup is called at most once.

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73624

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
improve_rest_parameters_setup.patch (1.76 KB)
bench_method_arg.rb (1.32 KB)
improve_rest_parameters_setup_20_8_2018.patch (8.75 KB)


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

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

* [ruby-core:88575] Re: [Ruby trunk Feature#15010] Reduce allocation for rest parameters
  2018-08-20 10:27 ` [ruby-core:88568] " chopraanmol1
@ 2018-08-20 19:00   ` Eric Wong
  0 siblings, 0 replies; 21+ messages in thread
From: Eric Wong @ 2018-08-20 19:00 UTC (permalink / raw
  To: ruby-core

chopraanmol1@gmail.com wrote:
> normalperson (Eric Wong) wrote:
> >  Cool!  It's probably worth implementing something like
> >  rb_ary_shift_m (but without the return value) to avoid looping
> >  on rb_ary_shift.

> Added rb_ary_clear_m (suggestion for a better name will be
> appreciated) with suggested changes.

Thanks; for internal functions the name isn't as important :)

New functions prototypes should go into internal.h, though.
ruby/intern.h ended up being part of the public API and external
libraries depend on it :<

There's no reason for arg_rest_dup to be a macro instead of a
static inline function.  Static inlines are preferred because
they make life easier for the compiler and debugger.

Also, multi-line macros without "do {} while (0)" is dangerous
to control flow.

Thanks again.

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

* [ruby-core:88579] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (3 preceding siblings ...)
  2018-08-20 10:27 ` [ruby-core:88568] " chopraanmol1
@ 2018-08-20 22:15 ` mame
  2018-08-20 23:14 ` [ruby-core:88581] " nobu
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: mame @ 2018-08-20 22:15 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by mame (Yusuke Endoh).


Thank you, too.  Two points:

First, the prefix `_m` is often used for an entry function of Ruby-level method that is passed to `rb_define_method`.  Though it is just an internal function, it would be better to avoid the prefix.  How about `rb_ary_remove_first`?

Second, I agree with ensuring rb_ary_dup is called at most once.  But I'm afraid if rewriting the array without dup may cause obscure incompatibility.  It is difficult for me to review your patch.

@ko1, could you review this?

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73635

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.68 KB)


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

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

* [ruby-core:88581] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (4 preceding siblings ...)
  2018-08-20 22:15 ` [ruby-core:88579] " mame
@ 2018-08-20 23:14 ` nobu
  2018-08-21  4:46 ` [ruby-core:88585] " chopraanmol1
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: nobu @ 2018-08-20 23:14 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by nobu (Nobuyoshi Nakada).


normalperson (Eric Wong) wrote:
>  > Added rb_ary_clear_m (suggestion for a better name will be
>  > appreciated) with suggested changes.
>  
>  Thanks; for internal functions the name isn't as important :)

What about rb_ary_clear_head? (or rb_ary_behead :)


----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73638

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.68 KB)


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

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

* [ruby-core:88585] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (5 preceding siblings ...)
  2018-08-20 23:14 ` [ruby-core:88581] " nobu
@ 2018-08-21  4:46 ` chopraanmol1
  2018-08-21  5:20 ` [ruby-core:88586] " chopraanmol1
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-21  4:46 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


normalperson (Eric Wong) wrote:
>  
>  New functions prototypes should go into internal.h, though.
>  ruby/intern.h ended up being part of the public API and external
>  libraries depend on it :<
>  
>  There's no reason for arg_rest_dup to be a macro instead of a
>  static inline function.  Static inlines are preferred because
>  they make life easier for the compiler and debugger.
>  

Updated.



mame (Yusuke Endoh) wrote:
>
> First, the prefix `_m` is often used for an entry function of Ruby-level method that is passed to `rb_define_method`.  Though it is just an internal function, it would be better to avoid the prefix.  How about `rb_ary_remove_first`?
> 

For now, I'm renaming the method to rb_ary_behead (suggested by nobu)




----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73642

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.71 KB)


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

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

* [ruby-core:88586] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (6 preceding siblings ...)
  2018-08-21  4:46 ` [ruby-core:88585] " chopraanmol1
@ 2018-08-21  5:20 ` chopraanmol1
  2018-08-21  5:36 ` [ruby-core:88587] " chopraanmol1
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-21  5:20 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


I'm also thinking of an alternate solution which will avoid passing the skip_dup_flag variable around, If we can ensure that args->rest is not used/assigned until args_copy is called. To do this when VM_CALL_ARGS_SPLAT flag is on instead of assigning args->rest we could expand the splat arg to locals / args->argv.

Unless it breaks test beyond repair, I'll add this alternate patch with the respective benchmark(It probably will be slower for the large array), so it can be compared side by side. In this solution, args_setup_post_parameters can be further modified to use args->argv instead of args->rest which makes zero allocation for the following example:
~~~
def opt_post(a,b,c=1,d=2,e,f); end
~~~
 

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73643

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.71 KB)


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

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

* [ruby-core:88587] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (7 preceding siblings ...)
  2018-08-21  5:20 ` [ruby-core:88586] " chopraanmol1
@ 2018-08-21  5:36 ` chopraanmol1
  2018-08-21  5:59 ` [ruby-core:88588] " nobu
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-21  5:36 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


normalperson (Eric Wong) wrote:
>  New functions prototypes should go into internal.h, though.
>  ruby/intern.h ended up being part of the public API and external
>  libraries depend on it :<
>  

Moving method to internal.h breaks jit https://travis-ci.org/ruby/ruby/builds/418529271 , I'm not sure how to fix this failure.


----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73644

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.71 KB)


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

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

* [ruby-core:88588] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (8 preceding siblings ...)
  2018-08-21  5:36 ` [ruby-core:88587] " chopraanmol1
@ 2018-08-21  5:59 ` nobu
  2018-08-21  7:26 ` [ruby-core:88589] " chopraanmol1
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: nobu @ 2018-08-21  5:59 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by nobu (Nobuyoshi Nakada).


chopraanmol1 (Anmol Chopra) wrote:
> Moving method to internal.h breaks jit https://travis-ci.org/ruby/ruby/builds/418529271 , I'm not sure how to fix this failure.

Define the function with `MJIT_FUNC_EXPORTED`.

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73645

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.71 KB)


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

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

* [ruby-core:88589] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (9 preceding siblings ...)
  2018-08-21  5:59 ` [ruby-core:88588] " nobu
@ 2018-08-21  7:26 ` chopraanmol1
  2018-08-21 12:45 ` [ruby-core:88594] " chopraanmol1
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-21  7:26 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


chopraanmol1 (Anmol Chopra) wrote:
> I'm also thinking of an alternate solution which will avoid passing the skip_dup_flag variable around, If we can ensure that args->rest is not used/assigned until args_copy is called. To do this when VM_CALL_ARGS_SPLAT flag is on instead of assigning args->rest we could expand the splat arg to locals / args->argv.
> 

Implementation: https://github.com/ruby/ruby/compare/trunk...chopraanmol1:improve_rest_parameters_setup_v2

Benchmark result

~~~

                               trunk        patch 1       patch 2
benchmark_method               0.196346     0.197841      0.196466
rest_method                    0.788287     0.539768      0.535512
lead_method                    0.792892     0.547752      0.533818
post_method                    1.133035     0.636972      0.540609
lead_post_method               0.867869     0.709440      0.370182
benchmark_method *args         0.227389     0.230066      0.227671
rest_method *args              0.826490     0.559881      0.563779
lead_method *args              0.821036     0.602590      0.565583
post_method *args              1.157621     0.649459      0.570189
lead_post_method *args         1.064632     0.687248      0.387054
rest_method *long_args         0.985696     0.766369      0.779729
lead_method *long_args         0.997824     0.870107      0.794615
post_method *long_args         1.703731     0.863923      0.813282
lead_post_method *long_args    1.707543     0.989116      0.802757
rest_with_named_parameter      1.862414     1.293406      1.255951
bench proc                     0.275176     0.263893      0.260555
lead_proc underflow_args       1.149043     0.801893      0.363017
opt_post_proc overflow_args    1.025754     0.717966      0.312920

~~~



chopraanmol1 (Anmol Chopra) wrote:
> In this solution, args_setup_post_parameters can be further modified to use args->argv instead of args->rest which makes zero allocation for the following example:

args->argv and locals are pointing to same address so it is not feasible.


Note: This second patch is not the final implementation, there are few more changes. In second patch args_check_block_arg0 function can still assign args->rest, I'll look into this later (only if the second patch seems more acceptable over first) if it can be completely avoided. In case it can be avoided most of the method handling args->rest can be cleaned after that, which will also ensure that args->rest_index is never modified. As a result, we could even avoid calling rb_ary_behead.




----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73646

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
0001-Reduce-allocation-for-rest-parameters.patch (7.71 KB)


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

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

* [ruby-core:88594] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (10 preceding siblings ...)
  2018-08-21  7:26 ` [ruby-core:88589] " chopraanmol1
@ 2018-08-21 12:45 ` chopraanmol1
  2018-08-23  7:22 ` [ruby-core:88611] " ko1
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-21 12:45 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


Limitation of patch 2. 

1. Patch 2 gets slower than Patch 1 for a large array. Array with length 100 - 200 have similar performance but beyond that patch 1 is faster in most of the case. 

2. Patch 2 results in segmentation fault for following: https://github.com/ruby/ruby/blob/8e66ffc1d756c42ee025a56672ad71f2200ca6be/test/ruby/test_method.rb#L951


Ignoring above limitation patch 2 do perform better for the small array. One Hack Solution can be to check the length of splat arg against some arbitrary number to decide if splat arg should be expanded to args->argv or should be assigned to args->rest. But it doesn't sound like a nice solution.

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73650

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.73 KB)
bench_method_arg_v2.rb (3.15 KB)


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

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

* [ruby-core:88611] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (11 preceding siblings ...)
  2018-08-21 12:45 ` [ruby-core:88594] " chopraanmol1
@ 2018-08-23  7:22 ` ko1
  2018-08-23  7:41 ` [ruby-core:88614] " chopraanmol1
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: ko1 @ 2018-08-23  7:22 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by ko1 (Koichi Sasada).


sorry, which patch should I review?


----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73672

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.73 KB)
bench_method_arg_v2.rb (3.15 KB)


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

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

* [ruby-core:88614] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (12 preceding siblings ...)
  2018-08-23  7:22 ` [ruby-core:88611] " ko1
@ 2018-08-23  7:41 ` chopraanmol1
  2018-08-27  5:26 ` [ruby-core:88667] " chopraanmol1
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-23  7:41 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


ko1 (Koichi Sasada) wrote:
> sorry, which patch should I review?

Reduce-allocation-for-rest-parameters-v1.patch 

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73675

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.73 KB)
bench_method_arg_v2.rb (3.15 KB)


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

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

* [ruby-core:88667] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (13 preceding siblings ...)
  2018-08-23  7:41 ` [ruby-core:88614] " chopraanmol1
@ 2018-08-27  5:26 ` chopraanmol1
  2018-08-27  6:33 ` [ruby-core:88675] " ko1
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-27  5:26 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


@ko1, It would be great if you could review https://bugs.ruby-lang.org/attachments/7343/Reduce-allocation-for-rest-parameters-v1.patch

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73725

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
bench_method_arg_v2.rb (3.15 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.71 KB)


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

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

* [ruby-core:88675] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (14 preceding siblings ...)
  2018-08-27  5:26 ` [ruby-core:88667] " chopraanmol1
@ 2018-08-27  6:33 ` ko1
  2018-08-27  7:41 ` [ruby-core:88679] " chopraanmol1
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: ko1 @ 2018-08-27  6:33 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by ko1 (Koichi Sasada).


Sorry for late response.

# Idea (as my understanding)

~a rest parameter" is dup multiple times because of current implementation. Only 1 "dup" is needed. They should be eliminate.
The patch try to manage "dup'ed or not" by passing `skip_rest_ary_dup`, and if it is true, then we don't need to dup the rest parameter again.

# Comment

I'm fine to introduce your idea.
Why don't you put a new field in `args_info`?


----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73732

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
bench_method_arg_v2.rb (3.15 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.71 KB)


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

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

* [ruby-core:88679] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (15 preceding siblings ...)
  2018-08-27  6:33 ` [ruby-core:88675] " ko1
@ 2018-08-27  7:41 ` chopraanmol1
  2018-08-27 10:09 ` [ruby-core:88683] " chopraanmol1
  2018-08-28  6:53 ` [ruby-core:88707] " ko1
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-27  7:41 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


ko1 (Koichi Sasada) wrote:
> # Idea (as my understanding)
> 
> ~a rest parameter" is dup multiple times because of current implementation. Only 1 "dup" is needed. They should be eliminate.
> The patch try to manage "dup'ed or not" by passing `skip_rest_ary_dup`, and if it is true, then we don't need to dup the rest parameter again.

Yes, and once a rest parameter is duped it mutates the array in case if rest_index is modified (Previously, only args_setup_post_parameters used to mutate rest parameter). 
> 
> # Comment
> 
> I'm fine to introduce your idea.
> Why don't you put a new field in `args_info`?

This suggestion makes a lot of sense as it will simplify this patch, I'll update the patch soon to reflect this.


----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73735

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
bench_method_arg_v2.rb (3.15 KB)
Reduce-allocation-for-rest-parameters-v1.patch (7.71 KB)


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

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

* [ruby-core:88683] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (16 preceding siblings ...)
  2018-08-27  7:41 ` [ruby-core:88679] " chopraanmol1
@ 2018-08-27 10:09 ` chopraanmol1
  2018-08-28  6:53 ` [ruby-core:88707] " ko1
  18 siblings, 0 replies; 21+ messages in thread
From: chopraanmol1 @ 2018-08-27 10:09 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by chopraanmol1 (Anmol Chopra).


@ko1, I've added new field rest_dupped to args_info. 

Updated patch https://bugs.ruby-lang.org/attachments/7344/Reduce-allocation-for-rest-parameters-v1.patch 



----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73741

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
bench_method_arg_v2.rb (3.15 KB)
Reduce-allocation-for-rest-parameters-v1.patch (5.43 KB)


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

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

* [ruby-core:88707] [Ruby trunk Feature#15010] Reduce allocation for rest parameters
       [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
                   ` (17 preceding siblings ...)
  2018-08-27 10:09 ` [ruby-core:88683] " chopraanmol1
@ 2018-08-28  6:53 ` ko1
  18 siblings, 0 replies; 21+ messages in thread
From: ko1 @ 2018-08-28  6:53 UTC (permalink / raw
  To: ruby-core

Issue #15010 has been updated by ko1 (Koichi Sasada).


It seems fine.
I'll commit it.

----------------------------------------
Feature #15010: Reduce allocation for rest parameters
https://bugs.ruby-lang.org/issues/15010#change-73764

* Author: chopraanmol1 (Anmol Chopra)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
----------------------------------------
Currently multiple arrays are allocated while making a call to method with rest parameter.

E.g.
~~~
def rest_method(*args) #-> This will create 2 arrays
end

def post_method(*args,last) #-> This will create 3 arrays
end
~~~

Applying following set of changes will reduce creation of array to 1

https://github.com/ruby/ruby/pull/1935

Benchmark Result:

trunk
~~~
                                  user     system      total        real
benchmark_method              0.340000   0.000000   0.340000 (  0.337035)
rest_method                   0.964000   0.000000   0.964000 (  0.964660)
lead_method                   0.976000   0.000000   0.976000 (  0.976011)
post_method                   2.424000   0.000000   2.424000 (  2.421732)
lead_post_method              1.800000   0.000000   1.800000 (  1.799500)
rest_with_named_parameter     2.040000   0.000000   2.040000 (  2.040323)
lead_proc underflow_args      1.224000   0.000000   1.224000 (  1.225237)
opt_post_proc overflow_args   1.056000   0.000000   1.056000 (  1.057402)
~~~

modified
~~~
                                  user     system      total        real
benchmark_method              0.336000   0.000000   0.336000 (  0.336911)
rest_method                   0.708000   0.000000   0.708000 (  0.706142)
lead_method                   0.720000   0.000000   0.720000 (  0.717971)
post_method                   1.896000   0.000000   1.896000 (  1.894426)
lead_post_method              1.560000   0.000000   1.560000 (  1.560495)
rest_with_named_parameter     1.464000   0.000000   1.464000 (  1.467313)
lead_proc underflow_args      0.864000   0.000000   0.864000 (  0.863980)
opt_post_proc overflow_args   0.772000   0.000000   0.772000 (  0.770364)
~~~

---Files--------------------------------
bench_method_arg.rb (1.32 KB)
Reduce-allocation-for-rest-parameters-v2.patch (3.57 KB)
bench_method_arg_v2.rb (3.15 KB)
Reduce-allocation-for-rest-parameters-v1.patch (5.43 KB)


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

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

end of thread, other threads:[~2018-08-28  6:53 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <redmine.issue-15010.20180819162534@ruby-lang.org>
2018-08-19 16:25 ` [ruby-core:88555] [Ruby trunk Feature#15010] Reduce allocation for rest parameters chopraanmol1
2018-08-20  5:13 ` [ruby-core:88561] " mame
2018-08-20  6:29 ` [ruby-core:88564] " chopraanmol1
2018-08-20  6:32   ` [ruby-core:88565] " Eric Wong
2018-08-20 10:27 ` [ruby-core:88568] " chopraanmol1
2018-08-20 19:00   ` [ruby-core:88575] " Eric Wong
2018-08-20 22:15 ` [ruby-core:88579] " mame
2018-08-20 23:14 ` [ruby-core:88581] " nobu
2018-08-21  4:46 ` [ruby-core:88585] " chopraanmol1
2018-08-21  5:20 ` [ruby-core:88586] " chopraanmol1
2018-08-21  5:36 ` [ruby-core:88587] " chopraanmol1
2018-08-21  5:59 ` [ruby-core:88588] " nobu
2018-08-21  7:26 ` [ruby-core:88589] " chopraanmol1
2018-08-21 12:45 ` [ruby-core:88594] " chopraanmol1
2018-08-23  7:22 ` [ruby-core:88611] " ko1
2018-08-23  7:41 ` [ruby-core:88614] " chopraanmol1
2018-08-27  5:26 ` [ruby-core:88667] " chopraanmol1
2018-08-27  6:33 ` [ruby-core:88675] " ko1
2018-08-27  7:41 ` [ruby-core:88679] " chopraanmol1
2018-08-27 10:09 ` [ruby-core:88683] " chopraanmol1
2018-08-28  6:53 ` [ruby-core:88707] " ko1

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