Issue #20011 has been updated by jeremyevans0 (Jeremy Evans). ko1 (Koichi Sasada) wrote in #note-4:
Could you provide benchmarks?
I added a benchmark to the pull request. Here are the results: ``` arg_splat after-patch: 11633482.8 i/s before-patch: 6603689.5 i/s - 1.76x slower arg_splat_block after-patch: 10011285.1 i/s before-patch: 6072490.8 i/s - 1.65x slower splat_kw_splat after-patch: 5574867.8 i/s before-patch: 4025629.4 i/s - 1.38x slower splat_kw_splat_block after-patch: 4977728.5 i/s before-patch: 3827400.9 i/s - 1.30x slower splat_kw after-patch: 3686117.6 i/s before-patch: 3103409.3 i/s - 1.19x slower splat_kw_block after-patch: 3535774.1 i/s before-patch: 2957890.6 i/s - 1.20x slower ``` So a 20%-75% increase in performance depending on the type of method call. ---------------------------------------- Feature #20011: Reduce implicit array allocations on caller side of method calling https://bugs.ruby-lang.org/issues/20011#change-105450 * Author: jeremyevans0 (Jeremy Evans) * Status: Open * Priority: Normal ---------------------------------------- I would like to use the peephole optimizer to eliminate caller-side array allocations for the following cases, by switching `splatarray true` to `splatarray false`: ```ruby f(1, *a) f(1, *a, &lvar) f(1, *a, &@ivar) f(*a, **lvar) f(*a, **@ivar) f(*a, **lvar, &lvar) f(*a, **@ivar, &@ivar) f(*a, kw: 1) f(*a, kw:1, &lvar) f(*a, kw:1, &@ivar) ``` In terms of safety, currently, `f(*a, &lvar)` and `f(*a, &@ivar)` both avoid array allocations (`splatarray false`), and all of the above are as safe as those in terms of safety. Note that since at least Ruby 1.8, in pathlogical cases, `lvar.to_proc` can modify `a`, which results in behavior contrary to expected evaluation order: ```ruby ary = [1,2] kwd = Object.new kwd.define_singleton_method(:to_proc) {ary << 4; lambda{}} p(*ary, &kwd) # 4 included in output ``` I think that for both the current `f(*a, &lvar)` and `f(*a, &@ivar)` cases and all of the above cases, the benefit of avoiding the allocation is higher than the costs, considering that only pathologic cases fail. If we do not want the above optimization, for consistency, we should remove the optimization of `f(*a, &lvar)` and `f(*a, &@ivar)` (or update the optimization so that it is only used if `lvar` or `@ivar` is already a proc), which will slow Ruby down. I have submitted a pull request for these changes: https://github.com/ruby/ruby/pull/8853 To make sure these cases are worth optimizing, I did some analysis: For ruby -e '' (just loads rubygems): ``` 5 : f(1, *a) 2 : f(1, *a, &lvar) | f(1, *a, &@ivar) ``` Current stdlib: ``` 139 : f(1, *a) 3 : f(1, *a, &lvar) | f(1, *a, &@ivar) 4 : f(*a, **lvar) | f(*a, **@ivar) ``` Rails master: ``` 77 : f(1, *a) 5 : f(1, *a, &lvar) | f(1, *a, &@ivar) 26 : f(*a, **lvar) | f(*a, **@ivar) 5 : f(*a, kw: 1) ``` minitest 5.20 (bundled gem): ``` 4 : f(1, *a) 1 : f(1, *a, &lvar) | f(1, *a, &@ivar) 2 : f(*a, **lvar) | f(*a, **@ivar) 2 : f(*a, **lvar, &lvar) | f(*a, **@ivar, &@ivar) ``` capybara 3.39.2: ``` 15 : f(1, *a) 2 : f(1, *a, &lvar) | f(1, *a, &@ivar) 19 : f(*a, **lvar) | f(*a, **@ivar) 4 : f(*a, **lvar, &lvar) | f(*a, **@ivar, &@ivar) ``` This shows that all cases optimized except `f(*a, kw:1, &lvar)` and `f(*a, kw:1, &@ivar)` are used in common gems. Those cases could be removed if people think they are not worth optimizing. Code backing the above analysis can be found at https://github.com/ruby/ruby/pull/8853#issuecomment-1817656139 -- https://bugs.ruby-lang.org/