зеркало из https://github.com/github/ruby.git
Avoid a hash allocation when keyword splatting empty hash when calling ruby2_keywords method
Treat this similar to keyword splatting nil, using goto ignore. However, keep previous behavior if the method accepts a keyword splat, to avoid double hash allocation. This also can avoid an array allocation when calling a method that doesn't have any splat parameters but supports literal keyword parameters, because ignore_keyword_hash_p was not ignoring the keyword hash in that case. This change doesn't remove the empty ruby2_keywords hash from the array, which caused an assertion failure if the method being called accepted keywords in some cases. Modify the assertion to handle this case. An alternative approach would add a flag to the args struct so the args_argc calculation could handle this case and report the correct argc, but such an approach would likely be slower.
This commit is contained in:
Родитель
8c69caa495
Коммит
bfba96a106
|
@ -273,7 +273,7 @@ class TestAllocation < Test::Unit::TestCase
|
|||
check_allocations(0, 1, "keyword(**hash1, **empty_hash#{block})")
|
||||
check_allocations(1, 0, "keyword(*empty_array, *empty_array, **empty_hash#{block})")
|
||||
|
||||
check_allocations(1, 0, "keyword(*r2k_empty_array#{block})")
|
||||
check_allocations(0, 0, "keyword(*r2k_empty_array#{block})")
|
||||
check_allocations(1, 1, "keyword(*r2k_array#{block})")
|
||||
|
||||
check_allocations(0, 1, "keyword(*empty_array, a: 2, **empty_hash#{block})")
|
||||
|
@ -379,7 +379,7 @@ class TestAllocation < Test::Unit::TestCase
|
|||
check_allocations(1, 1, "required_and_keyword(*array1, *empty_array, a: 2, **empty_hash#{block})")
|
||||
check_allocations(1, 1, "required_and_keyword(*array1, *empty_array, **hash1, **empty_hash#{block})")
|
||||
|
||||
check_allocations(1, 0, "required_and_keyword(*r2k_empty_array1#{block})")
|
||||
check_allocations(0, 0, "required_and_keyword(*r2k_empty_array1#{block})")
|
||||
check_allocations(1, 1, "required_and_keyword(*r2k_array1#{block})")
|
||||
|
||||
# Currently allocates 1 array unnecessarily due to splatarray true
|
||||
|
@ -724,7 +724,7 @@ class TestAllocation < Test::Unit::TestCase
|
|||
check_allocations(1, 1, "r2k(1, **empty_hash, a: 2#{block})")
|
||||
|
||||
check_allocations(1, 0, "r2k(1, **nil#{block})")
|
||||
check_allocations(1, 1, "r2k(1, **empty_hash#{block})")
|
||||
check_allocations(1, 0, "r2k(1, **empty_hash#{block})")
|
||||
check_allocations(1, 1, "r2k(1, **hash1#{block})")
|
||||
check_allocations(1, 1, "r2k(1, *empty_array, **hash1#{block})")
|
||||
check_allocations(1, 1, "r2k(1, **hash1, **empty_hash#{block})")
|
||||
|
@ -732,17 +732,17 @@ class TestAllocation < Test::Unit::TestCase
|
|||
|
||||
check_allocations(1, 0, "r2k(1, *empty_array#{block})")
|
||||
check_allocations(1, 1, "r2k(1, **hash1, **empty_hash#{block})")
|
||||
check_allocations(1, 1, "r2k(1, *empty_array, *empty_array, **empty_hash#{block})")
|
||||
check_allocations(1, 0, "r2k(1, *empty_array, *empty_array, **empty_hash#{block})")
|
||||
|
||||
check_allocations(1, 1, "r2k(*array1, a: 2#{block})")
|
||||
|
||||
check_allocations(1, 0, "r2k(*array1, **nill#{block})")
|
||||
check_allocations(1, 1, "r2k(*array1, **empty_hash#{block})")
|
||||
check_allocations(1, 0, "r2k(*array1, **empty_hash#{block})")
|
||||
check_allocations(1, 1, "r2k(*array1, **hash1#{block})")
|
||||
check_allocations(1, 1, "r2k(*array1, *empty_array, **hash1#{block})")
|
||||
|
||||
check_allocations(1, 0, "r2k(*array1, *empty_array#{block})")
|
||||
check_allocations(1, 1, "r2k(*array1, *empty_array, **empty_hash#{block})")
|
||||
check_allocations(1, 0, "r2k(*array1, *empty_array, **empty_hash#{block})")
|
||||
|
||||
check_allocations(1, 1, "r2k(*array1, *empty_array, a: 2, **empty_hash#{block})")
|
||||
check_allocations(1, 1, "r2k(*array1, *empty_array, **hash1, **empty_hash#{block})")
|
||||
|
|
17
vm_args.c
17
vm_args.c
|
@ -535,6 +535,10 @@ ignore_keyword_hash_p(VALUE keyword_hash, const rb_iseq_t * const iseq, unsigned
|
|||
}
|
||||
}
|
||||
|
||||
if (RHASH_EMPTY_P(keyword_hash) && !ISEQ_BODY(iseq)->param.flags.has_kwrest) {
|
||||
goto ignore;
|
||||
}
|
||||
|
||||
if (!(*kw_flag & VM_CALL_KW_SPLAT_MUT) &&
|
||||
(ISEQ_BODY(iseq)->param.flags.has_kwrest ||
|
||||
ISEQ_BODY(iseq)->param.flags.ruby2_keywords)) {
|
||||
|
@ -856,7 +860,18 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
|
|||
args_setup_kw_parameters_from_kwsplat(ec, iseq, keyword_hash, klocals, remove_hash_value);
|
||||
}
|
||||
else {
|
||||
VM_ASSERT(args_argc(args) == 0);
|
||||
#if VM_CHECK_MODE > 0
|
||||
if (args_argc(args) != 0) {
|
||||
VM_ASSERT(ci_flag & VM_CALL_ARGS_SPLAT);
|
||||
VM_ASSERT(!(ci_flag & (VM_CALL_KWARG | VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT)));
|
||||
VM_ASSERT(!kw_flag);
|
||||
VM_ASSERT(!ISEQ_BODY(iseq)->param.flags.has_rest);
|
||||
VM_ASSERT(RARRAY_LENINT(args->rest) > 0);
|
||||
VM_ASSERT(RB_TYPE_P(rest_last, T_HASH));
|
||||
VM_ASSERT(FL_TEST_RAW(rest_last, RHASH_PASS_AS_KEYWORDS));
|
||||
VM_ASSERT(args_argc(args) == 1);
|
||||
}
|
||||
#endif
|
||||
args_setup_kw_parameters(ec, iseq, NULL, 0, NULL, klocals);
|
||||
}
|
||||
}
|
||||
|
|
Загрузка…
Ссылка в новой задаче