From 43683e1e9d8f7f6ed3fb4a48598190a0993f8f66 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Sat, 18 Jan 2025 08:54:28 -0800 Subject: [PATCH] Avoid allocation for anonymous positional splat with no arguments Anonymous positional splats cannot be directly accessed, they can only be passed as splats to other methods. So if an anonymous positional splat would be empty, you can use a shared frozen empty array to save an allocation. ```ruby def a(*) end a() ``` This is similar to how anonymous empty keyword splats are optimized, except those use `nil` instead of a shared empty frozen hash. This updates the allocation tests to check that the array allocations are avoided where possible. It also makes a small change to test_iseq.rb to ensure an unfrozen hash is passed as the value of an anonymous splat parameter. --- test/ruby/test_allocation.rb | 10 ++++++++++ test/ruby/test_iseq.rb | 2 +- vm_args.c | 25 +++++++++++++++---------- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/test/ruby/test_allocation.rb b/test/ruby/test_allocation.rb index fef3111483..2b181b1bdc 100644 --- a/test/ruby/test_allocation.rb +++ b/test/ruby/test_allocation.rb @@ -524,6 +524,7 @@ class TestAllocation < Test::Unit::TestCase end def test_anonymous_splat_and_anonymous_keyword_splat_parameters + only_block = block.empty? ? block : block[2..] check_allocations(<<~RUBY) def self.anon_splat_and_anon_keyword_splat(*, **#{block}); end @@ -556,6 +557,10 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, *empty_array, **hash1, **empty_hash#{block})") + check_allocations(0, 0, "anon_splat_and_anon_keyword_splat(#{only_block})") + check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(a: 2#{block})") + check_allocations(0, 0, "anon_splat_and_anon_keyword_splat(**empty_hash#{block})") + check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(1, *empty_array, **hash1, **empty_hash#{block})") check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})") @@ -570,6 +575,7 @@ class TestAllocation < Test::Unit::TestCase end def test_nested_anonymous_splat_and_anonymous_keyword_splat_parameters + only_block = block.empty? ? block : block[2..] check_allocations(<<~RUBY) def self.t(*, **#{block}); end def self.anon_splat_and_anon_keyword_splat(*, **#{block}); t(*, **) end @@ -603,6 +609,10 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, *empty_array, **hash1, **empty_hash#{block})") + check_allocations(0, 0, "anon_splat_and_anon_keyword_splat(#{only_block})") + check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(a: 2#{block})") + check_allocations(0, 0, "anon_splat_and_anon_keyword_splat(**empty_hash#{block})") + check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(1, *empty_array, **hash1, **empty_hash#{block})") check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})") diff --git a/test/ruby/test_iseq.rb b/test/ruby/test_iseq.rb index 337e575b2e..032f78f6a8 100644 --- a/test/ruby/test_iseq.rb +++ b/test/ruby/test_iseq.rb @@ -173,7 +173,7 @@ class TestISeq < Test::Unit::TestCase obj = Object.new def obj.foo(*) nil.instance_eval{ ->{super} } end assert_raise_with_message(Ractor::IsolationError, /refer unshareable object \[\] from variable '\*'/) do - Ractor.make_shareable(obj.foo) + Ractor.make_shareable(obj.foo(*[])) end end diff --git a/vm_args.c b/vm_args.c index 1303243fd3..a84540c61b 100644 --- a/vm_args.c +++ b/vm_args.c @@ -894,16 +894,21 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co } if (ISEQ_BODY(iseq)->param.flags.has_rest) { - args_setup_rest_parameter(args, locals + ISEQ_BODY(iseq)->param.rest_start); - VALUE ary = *(locals + ISEQ_BODY(iseq)->param.rest_start); - VALUE index = RARRAY_LEN(ary) - 1; - if (splat_flagged_keyword_hash && - !ISEQ_BODY(iseq)->param.flags.ruby2_keywords && - !ISEQ_BODY(iseq)->param.flags.has_kw && - !ISEQ_BODY(iseq)->param.flags.has_kwrest && - RARRAY_AREF(ary, index) == splat_flagged_keyword_hash) { - ((struct RHash *)rest_last)->basic.flags &= ~RHASH_PASS_AS_KEYWORDS; - RARRAY_ASET(ary, index, rest_last); + if (UNLIKELY(ISEQ_BODY(iseq)->param.flags.anon_rest && args->argc == 0 && !args->rest && !ISEQ_BODY(iseq)->param.flags.has_post)) { + *(locals + ISEQ_BODY(iseq)->param.rest_start) = args->rest = rb_cArray_empty_frozen; + } + else { + args_setup_rest_parameter(args, locals + ISEQ_BODY(iseq)->param.rest_start); + VALUE ary = *(locals + ISEQ_BODY(iseq)->param.rest_start); + VALUE index = RARRAY_LEN(ary) - 1; + if (splat_flagged_keyword_hash && + !ISEQ_BODY(iseq)->param.flags.ruby2_keywords && + !ISEQ_BODY(iseq)->param.flags.has_kw && + !ISEQ_BODY(iseq)->param.flags.has_kwrest && + RARRAY_AREF(ary, index) == splat_flagged_keyword_hash) { + ((struct RHash *)rest_last)->basic.flags &= ~RHASH_PASS_AS_KEYWORDS; + RARRAY_ASET(ary, index, rest_last); + } } }