Skip to content

Avoid array allocation for *nil, by not calling nil.to_a#12597

Merged
jeremyevans merged 1 commit intoruby:masterfrom
jeremyevans:avoid-array-allocation-nil-splat
Mar 27, 2025
Merged

Avoid array allocation for *nil, by not calling nil.to_a#12597
jeremyevans merged 1 commit intoruby:masterfrom
jeremyevans:avoid-array-allocation-nil-splat

Conversation

@jeremyevans
Copy link
Copy Markdown
Contributor

A method call such as a(*nil) previously allocated an array, because it calls nil.to_a, but I have determined this array allocation is unnecessary. The instructions in this case are:

0000 putself                                                          (   1)[Li]
0001 putnil
0002 splatarray                             false
0004 opt_send_without_block                 <calldata!mid:a, argc:1, ARGS_SPLAT|FCALL>
0006 leave

The method call uses ARGS_SPLAT without ARGS_SPLAT_MUT, so the returned array doesn't need to be mutable. I believe all cases where splatarray false are used allow the returned object to be frozen, since the false means to not duplicate the array. The optimization in this case is to have splatarray false push a shared empty frozen array, instead of calling nil.to_a to return a newly allocated array.

There is a slightly backwards incompatibility with this optimization, in that nil.to_a is not called. However, I believe the new behavior of *nil not calling nil.to_a is more consistent with how **nil does not call nil.to_hash. Also, so much Ruby code would break if nil.to_a returned something different from the empty hash, that it's difficult to imagine anyone actually doing that in real code, though we have a few tests/specs for that.

I think it would be bad for consistency if *nil called nil.to_a in some cases and not others, so this changes other cases to not call nil.to_a:

For [*nil], this uses splatarray true, which now allocates a new array for a nil argument without calling nil.to_a.

For [1, *nil], this uses concattoarray, which now returns the first array if the second array is nil.

This updates the allocation tests to check that the array allocations are avoided where possible.

@jeremyevans jeremyevans requested a review from ko1 January 20, 2025 07:55
@jeremyevans jeremyevans force-pushed the avoid-array-allocation-nil-splat branch from a53d283 to 9bc5d59 Compare January 20, 2025 23:40
@ko1
Copy link
Copy Markdown
Contributor

ko1 commented Feb 7, 2025

Is this PR for performance? Please show us the benchmark results.
Anyway, I didn't know the details around this calling flag, so I believe @jeremyevans

@jeremyevans
Copy link
Copy Markdown
Contributor Author

This isn't just for performance, though it improves performance for the *nil case as it avoids a method call and avoids an array allocation. It's also for consistency with **nil. The behavior change has been approved by Matz already (https://bugs.ruby-lang.org/issues/21047).

I've pushed a benchmark, it shows a 4.45x speedup:

                            splat
      miniruby-after:  23202704.8 i/s 
     miniruby-before:   5213879.6 i/s - 4.45x  slower

@jeremyevans jeremyevans force-pushed the avoid-array-allocation-nil-splat branch from 9b9f0d4 to c98760d Compare March 25, 2025 19:38
@launchable-app

This comment has been minimized.

@jeremyevans jeremyevans force-pushed the avoid-array-allocation-nil-splat branch 3 times, most recently from a9a488a to 2e81bc4 Compare March 26, 2025 16:10
The following method call:

```ruby
a(*nil)
```

A method call such as `a(*nil)` previously allocated an array, because
it calls `nil.to_a`, but I have determined this array allocation is
unnecessary.  The instructions in this case are:

```
0000 putself                                                          (   1)[Li]
0001 putnil
0002 splatarray                             false
0004 opt_send_without_block                 <calldata!mid:a, argc:1, ARGS_SPLAT|FCALL>
0006 leave
```

The method call uses `ARGS_SPLAT` without `ARGS_SPLAT_MUT`, so the
returned array doesn't need to be mutable.  I believe all cases where
`splatarray false` are used allow the returned object to be frozen,
since the `false` means to not duplicate the array.  The optimization
in this case is to have `splatarray false` push a shared empty frozen
array, instead of calling `nil.to_a` to return a newly allocated array.

There is a slightly backwards incompatibility with this optimization,
in that `nil.to_a` is not called.  However, I believe the new behavior
of `*nil` not calling `nil.to_a` is more consistent with how `**nil`
does not call `nil.to_hash`.  Also, so much Ruby code would break if
`nil.to_a` returned something different from the empty hash, that it's
difficult to imagine anyone actually doing that in real code, though
we have a few tests/specs for that.

I think it would be bad for consistency if `*nil` called `nil.to_a`
in some cases and not others, so this changes other cases to not
call `nil.to_a`:

For `[*nil]`, this uses `splatarray true`, which now allocates a
new array for a `nil` argument without calling `nil.to_a`.

For `[1, *nil]`, this uses `concattoarray`, which now returns
the first array if the second array is `nil`.

This updates the allocation tests to check that the array allocations
are avoided where possible.

Implements [Feature #21047]
@jeremyevans jeremyevans force-pushed the avoid-array-allocation-nil-splat branch from 2e81bc4 to 6b338a6 Compare March 27, 2025 16:51
@jeremyevans jeremyevans merged commit 67d1dd2 into ruby:master Mar 27, 2025
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants