1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid array allocation for f(*r2k_ary) when def f(x)

Browse Source 
When calling a method that does not accept a positional splat
parameter with a splatted array with a ruby2_keywords flagged hash,
there is no need to duplicate the splatted array.  Previously,
Ruby would duplicate the splatted array and potentially modify
it before flattening it to the VM stack

Use a similar approach as the f(*ary, **hash) optimization,
flattening the splatted array to the VM stack without modifying
it, and make any modifications needed to the VM stack.
This commit is contained in:
Jeremy Evans 2024-07-12 15:33:59 -07:00
parent 1cc5a64dd8
commit 6428ce80f0
Notes: git 2024-07-19 05:17:39 +00:00 
Merged: https://github.com/ruby/ruby/pull/11161
2 changed files with 34 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
test/ruby/test_allocation.rb
View File

@ -265,7 +265,7 @@ class TestAllocation < Test::Unit::TestCase
check_allocations(1, 0, "keyword(*empty_array, *empty_array, **empty_hash#{block})")
check_allocations(0, 0, "keyword(*r2k_empty_array#{block})")
check_allocations(1, 0, "keyword(*r2k_array#{block})")
check_allocations(0, 0, "keyword(*r2k_array#{block})")
check_allocations(0, 1, "keyword(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 1, "keyword(*empty_array, **hash1, **empty_hash#{block})")
@ -291,8 +291,8 @@ class TestAllocation < Test::Unit::TestCase
check_allocations(0, 1, "keyword_splat(*empty_array#{block})")
check_allocations(1, 1, "keyword_splat(*empty_array, *empty_array, **empty_hash#{block})")
check_allocations(1, 1, "keyword_splat(*r2k_empty_array#{block})")
check_allocations(1, 1, "keyword_splat(*r2k_array#{block})")
check_allocations(0, 1, "keyword_splat(*r2k_empty_array#{block})")
check_allocations(0, 1, "keyword_splat(*r2k_array#{block})")
check_allocations(0, 1, "keyword_splat(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 1, "keyword_splat(*empty_array, **hash1, **empty_hash#{block})")
@ -318,8 +318,8 @@ class TestAllocation < Test::Unit::TestCase
check_allocations(0, 1, "keyword_and_keyword_splat(*empty_array#{block})")
check_allocations(1, 1, "keyword_and_keyword_splat(*empty_array, *empty_array, **empty_hash#{block})")
check_allocations(1, 1, "keyword_and_keyword_splat(*r2k_empty_array#{block})")
check_allocations(1, 1, "keyword_and_keyword_splat(*r2k_array#{block})")
check_allocations(0, 1, "keyword_and_keyword_splat(*r2k_empty_array#{block})")
check_allocations(0, 1, "keyword_and_keyword_splat(*r2k_array#{block})")
check_allocations(0, 1, "keyword_and_keyword_splat(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 1, "keyword_and_keyword_splat(*empty_array, **hash1, **empty_hash#{block})")
@ -359,7 +359,7 @@ class TestAllocation < Test::Unit::TestCase
check_allocations(1, 1, "required_and_keyword(*array1, *empty_array, **hash1, **empty_hash#{block})")
check_allocations(0, 0, "required_and_keyword(*r2k_empty_array1#{block})")
check_allocations(1, 0, "required_and_keyword(*r2k_array1#{block})")
check_allocations(0, 0, "required_and_keyword(*r2k_array1#{block})")
check_allocations(0, 1, "required_and_keyword(1, *empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 1, "required_and_keyword(1, *empty_array, **hash1, **empty_hash#{block})")
@ -446,8 +446,8 @@ class TestAllocation < Test::Unit::TestCase
check_allocations(1, 1, "required_and_keyword_splat(*array1, *empty_array, a: 2, **empty_hash#{block})")
check_allocations(1, 1, "required_and_keyword_splat(*array1, *empty_array, **hash1, **empty_hash#{block})")
check_allocations(1, 1, "required_and_keyword_splat(*r2k_empty_array1#{block})")
check_allocations(1, 1, "required_and_keyword_splat(*r2k_array1#{block})")
check_allocations(0, 1, "required_and_keyword_splat(*r2k_empty_array1#{block})")
check_allocations(0, 1, "required_and_keyword_splat(*r2k_array1#{block})")
check_allocations(0, 1, "required_and_keyword_splat(1, *empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 1, "required_and_keyword_splat(1, *empty_array, **hash1, **empty_hash#{block})")

26
vm_args.c
View File

@ -734,6 +734,32 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
}
given_argc--;
}
else if (!ISEQ_BODY(iseq)->param.flags.has_rest) {
// Avoid duping rest when not necessary
// Copy rest elements and converted keyword hash directly to VM stack
const VALUE *argv = RARRAY_CONST_PTR(args->rest);
int j, i=args->argc, rest_len = RARRAY_LENINT(args->rest)-1;
args->argc += rest_len;
if (rest_len) {
CHECK_VM_STACK_OVERFLOW(ec->cfp, rest_len+1);
for (i, j=0; rest_len > 0; rest_len--, i++, j++) {
locals[i] = argv[j];
}
}
args->rest = Qfalse;
ci_flag &= ~VM_CALL_ARGS_SPLAT;
if (ISEQ_BODY(iseq)->param.flags.has_kw || ISEQ_BODY(iseq)->param.flags.has_kwrest) {
given_argc--;
keyword_hash = converted_keyword_hash;
}
else {
args->argc += 1;
locals[i] = converted_keyword_hash;
keyword_hash = Qnil;
kw_flag = 0;
}
}
else {
if (rest_last != converted_keyword_hash) {
rest_last = converted_keyword_hash;