Avoid unnecessary array allocations for f(arg, *arg, **arg, **arg), f(*arg, a: lvar), and other calls
The f(arg, *arg, **arg, **arg) case was previously not optimized.
The optimizer didn't optimize this case because of the multiple
keyword splats, and the compiler didn't optimize it because the f(*arg, **arg, **arg) optimization added in
0ee3960685e283d8e75149a8777eb0109d41509a didn't apply.
I found it difficult to apply this optimization without changing
the setup_args_core API, since by the time you get to the ARGSCAT
case, you don't know whether you were called recursively or directly,
so I'm not sure if it was possible to know at that point whether the
array allocation could be avoided.
This changes the dup_rest argument in setup_args_core from an int
to a pointer to int. This allows us to track whether we have allocated
a caller side array for multiple splats or splat+post across
recursive calls. Check the pointed value (*dup_rest) to determine the splatarray argument. If dup_rest is 1, then use splatarray true
(caller-side array allocation), then set *dup_rest back to 0, ensuring
only a single splatarray true per method call.
Before calling setup_args_core, check whether the array allocation
can be avoided safely using splatarray false. Optimizable cases are:
If so, dup_rest is set to 0 instead of 1 to avoid the allocation.
After calling setup_args_core, check the flag. If the flag
includes VM_CALL_ARGS_SPLAT, and the pointed value has changed,
indicating splatarray true was used, then also set VM_CALL_ARGS_SPLAT_MUT in the flag.
My initial attempt at this broke the f(*ary, &ary.pop) test,
because we were not duplicating the ary in the splat even though
it was modified later (evaluation order issue). The initial attempt
would also break f(*ary, **ary.pop) or f(*ary, kw: ary.pop) cases
for the same reason. I added test cases for those evaluation
order issues.
Add setup_args_dup_rest_p static function that checks that a given
node is safe. Call that on the block pass node to determine if
the block pass node is safe. Also call it on each of the hash
key/value nodes to test that they are safe. If any are not safe,
then set dup_rest = 1 so that splatarray true will be used to
avoid the evaluation order issue.
This new approach has the affect of optimizing most cases of
literal keywords after positional splats. Previously, only
static keyword hashes after positional splats avoided array
allocation for the splat. Now, most dynamic keyword hashes
after positional splats also avoid array allocation.
Add allocation tests for dynamic keyword keyword hashes after
positional splats.
setup_args_dup_rest_p is currently fairly conservative. It
could definitely be expanded to handle additional node types
to reduce allocations in additional cases.
Avoid unnecessary array allocations for f(arg, *arg, **arg, **arg), f(*arg, a: lvar), and other calls
The
f(arg, *arg, **arg, **arg)
case was previously not optimized.The optimizer didn't optimize this case because of the multiple
keyword splats, and the compiler didn't optimize it because the
f(*arg, **arg, **arg)
optimization added in0ee3960685e283d8e75149a8777eb0109d41509a didn't apply.
I found it difficult to apply this optimization without changing
the
setup_args_core
API, since by the time you get to the ARGSCATcase, you don't know whether you were called recursively or directly,
so I'm not sure if it was possible to know at that point whether the
array allocation could be avoided.
This changes the dup_rest argument in
setup_args_core
from an intto a pointer to int. This allows us to track whether we have allocated
a caller side array for multiple splats or splat+post across
recursive calls. Check the pointed value (*dup_rest) to determine the
splatarray
argument. If dup_rest is 1, then usesplatarray true
(caller-side array allocation), then set *dup_rest back to 0, ensuring
only a single
splatarray true
per method call.Before calling
setup_args_core
, check whether the array allocationcan be avoided safely using
splatarray false
. Optimizable cases are:If so, dup_rest is set to 0 instead of 1 to avoid the allocation.
After calling
setup_args_core
, check the flag. If the flagincludes
VM_CALL_ARGS_SPLAT
, and the pointed value has changed,indicating
splatarray true
was used, then also setVM_CALL_ARGS_SPLAT_MUT
in the flag.My initial attempt at this broke the
f(*ary, &ary.pop)
test,because we were not duplicating the ary in the splat even though
it was modified later (evaluation order issue). The initial attempt
would also break
f(*ary, **ary.pop)
orf(*ary, kw: ary.pop)
casesfor the same reason. I added test cases for those evaluation
order issues.
Add setup_args_dup_rest_p static function that checks that a given
node is safe. Call that on the block pass node to determine if
the block pass node is safe. Also call it on each of the hash
key/value nodes to test that they are safe. If any are not safe,
then set dup_rest = 1 so that
splatarray true
will be used toavoid the evaluation order issue.
This new approach has the affect of optimizing most cases of
literal keywords after positional splats. Previously, only
static keyword hashes after positional splats avoided array
allocation for the splat. Now, most dynamic keyword hashes
after positional splats also avoid array allocation.
Add allocation tests for dynamic keyword keyword hashes after
positional splats.
setup_args_dup_rest_p is currently fairly conservative. It
could definitely be expanded to handle additional node types
to reduce allocations in additional cases.