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.
github · May 20, 2024 · 86cf074 · 86cf074
1 parent 2433b57
commit 86cf074
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 6 deletions.
diff --git a/test/ruby/test_allocation.rb b/test/ruby/test_allocation.rb
@@ -100,7 +100,7 @@ def self.none(#{only_block}); end
         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 @@ def self.required(x#{block}); end
         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 @@ def self.optional(x=nil#{block}); end
         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
@@ -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);
                 }