From 86cf074fa1dcf73846e094775414d661e47dfc76 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Tue, 14 May 2024 17:03:46 -0700 Subject: [PATCH] Avoid array allocation for empty ruby2_keywords flagged keyword hash If the method being called does not have a positional splat parameter, there is no point in allocating the array, as decrementing given_argc is sufficient to ensure the empty keyword hash is not considered an argument, assuming that we are calling a method/lambda and not a regular proc. --- test/ruby/test_allocation.rb | 8 ++++---- vm_args.c | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/test/ruby/test_allocation.rb b/test/ruby/test_allocation.rb index ab3ae15a9f..fbe0548899 100644 --- a/test/ruby/test_allocation.rb +++ b/test/ruby/test_allocation.rb @@ -100,7 +100,7 @@ class TestAllocation < Test::Unit::TestCase check_allocations(0, 1, "none(**empty_hash, **empty_hash#{block})") check_allocations(1, 1, "none(*empty_array, *empty_array, **empty_hash, **empty_hash#{block})") - check_allocations(#{block.empty?} ? 0 : 1, 0, "none(*r2k_empty_array#{block})") + check_allocations(0, 0, "none(*r2k_empty_array#{block})") RUBY end @@ -120,7 +120,7 @@ class TestAllocation < Test::Unit::TestCase check_allocations(0, 1, "required(**hash1, **empty_hash#{block})") check_allocations(1, 0, "required(*array1, *empty_array, **empty_hash#{block})") - check_allocations(#{block.empty?} ? 0 : 1, 0, "required(*r2k_empty_array1#{block})") + check_allocations(0, 0, "required(*r2k_empty_array1#{block})") check_allocations(0, 1, "required(*r2k_array#{block})") # Currently allocates 1 array unnecessarily due to splatarray true @@ -144,8 +144,8 @@ class TestAllocation < Test::Unit::TestCase check_allocations(0, 1, "optional(**hash1, **empty_hash#{block})") check_allocations(1, 0, "optional(*array1, *empty_array, **empty_hash#{block})") - check_allocations(#{block.empty?} ? 0 : 1, 0, "optional(*r2k_empty_array#{block})") - check_allocations(#{block.empty?} ? 0 : 1, 0, "optional(*r2k_empty_array1#{block})") + check_allocations(0, 0, "optional(*r2k_empty_array#{block})") + check_allocations(0, 0, "optional(*r2k_empty_array1#{block})") check_allocations(0, 1, "optional(*r2k_array#{block})") # Currently allocates 1 array unnecessarily due to splatarray true diff --git a/vm_args.c b/vm_args.c index 1db62dc7e2..0c1a26cfad 100644 --- a/vm_args.c +++ b/vm_args.c @@ -693,8 +693,11 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co args->rest_dupped = false; if (ignore_keyword_hash_p(rest_last, iseq, &kw_flag, &converted_keyword_hash)) { - arg_rest_dup(args); - rb_ary_pop(args->rest); + if (ISEQ_BODY(iseq)->param.flags.has_rest || arg_setup_type != arg_setup_method) { + // Only duplicate/modify splat array if it will be used + arg_rest_dup(args); + rb_ary_pop(args->rest); + } given_argc--; kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); }