diff options
author | Jeremy Evans <[email protected]> | 2024-06-19 11:27:01 -0700 |
---|---|---|
committer | Jeremy Evans <[email protected]> | 2024-07-10 14:45:38 -0700 |
commit | 0ee3960685e283d8e75149a8777eb0109d41509a () | |
tree | 4fc0e7fa2279ef99a80fc439dd5d682d64afcae6 | |
parent | 48e7112baaf338fa350f68f0386e05085d26606c (diff) |
Eliminate array allocations for single splat followed by mutable keywords
For calls such as: m(*ary, a: 2, **h) m(*ary, **h, **h, **h) Where m does not take a positional argument splat, there was previously an array allocation (splatarray true) to dup ary, even though it was not necessary to do so. This is because the elimination of the array allocation (splatarray false) was performed in the optimizer, and the optimizer didn't handle this case, because the instructions for the keywords can be of arbitrary length. Move part of the optimization from the optimizer to the compiler, detecting parse trees of the form: ARGS_PUSH: head: SPLAT tail: HASH (without brace) And using splatarray false instead of splatarray true for them. Unfortunately, moving part of the optimization to the compiler broke the hash allocation elimination optimization for calls of the form: m(*ary, a: 2) That's because the compiler had already set splatarray false, and the optimizer code was looking for splatarray true. Split the array allocation elimination and hash allocation elimination in the optimizer so that the hash allocation elimination will still apply if the compiler performs the splatarray false optimization.
-rw-r--r-- | compile.c | 90 | ||||
-rw-r--r-- | test/ruby/test_allocation.rb | 55 |
2 files changed, 100 insertions, 45 deletions
@@ -4019,21 +4019,18 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal niobj = niobj->next; /* - * Eliminate array and hash allocation for f(*a, kw: 1) * * splatarray true * duphash * send ARGS_SPLAT|KW_SPLAT|KW_SPLAT_MUT and not ARGS_BLOCKARG * => * splatarray false - * putobject - * send ARGS_SPLAT|KW_SPLAT */ if (optimize_args_splat_no_copy(iseq, iobj, niobj, - VM_CALL_ARGS_SPLAT|VM_CALL_KW_SPLAT|VM_CALL_KW_SPLAT_MUT, VM_CALL_ARGS_BLOCKARG, VM_CALL_KW_SPLAT_MUT)) { - - ((INSN*)niobj)->insn_id = BIN(putobject); - OPERAND_AT(niobj, 0) = rb_hash_freeze(rb_hash_resurrect(OPERAND_AT(niobj, 0))); goto optimized_splat; } @@ -4041,7 +4038,7 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal if (IS_NEXT_INSN_ID(niobj, getlocal) || IS_NEXT_INSN_ID(niobj, getinstancevariable) || IS_NEXT_INSN_ID(niobj, getblockparamproxy)) { /* - * Eliminate array and hash allocation for f(*a, kw: 1, &{arg,lvar,@iv}) * * splatarray true * duphash @@ -4049,20 +4046,73 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal * send ARGS_SPLAT|KW_SPLAT|KW_SPLAT_MUT|ARGS_BLOCKARG * => * splatarray false - * putobject * getlocal / getinstancevariable / getblockparamproxy - * send ARGS_SPLAT|KW_SPLAT|ARGS_BLOCKARG */ - if (optimize_args_splat_no_copy(iseq, iobj, niobj->next, - VM_CALL_ARGS_SPLAT|VM_CALL_KW_SPLAT|VM_CALL_KW_SPLAT_MUT|VM_CALL_ARGS_BLOCKARG, 0, VM_CALL_KW_SPLAT_MUT)) { ((INSN*)niobj)->insn_id = BIN(putobject); OPERAND_AT(niobj, 0) = rb_hash_freeze(rb_hash_resurrect(OPERAND_AT(niobj, 0))); } } } } - optimized_splat: return COMPILE_OK; } @@ -6323,8 +6373,18 @@ setup_args_core(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn, return argc; } case NODE_ARGSPUSH: { - if (flag_ptr) *flag_ptr |= VM_CALL_ARGS_SPLAT | VM_CALL_ARGS_SPLAT_MUT; - int argc = setup_args_core(iseq, args, RNODE_ARGSPUSH(argn)->nd_head, 1, NULL, NULL); if (nd_type_p(RNODE_ARGSPUSH(argn)->nd_body, NODE_LIST)) { int rest_len = compile_args(iseq, args, RNODE_ARGSPUSH(argn)->nd_body, &kwnode); @@ -273,16 +273,11 @@ 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(0, 0, "keyword(*empty_array#{block})") - 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(1, 1, "keyword(*r2k_array#{block})") - # Currently allocates 1 array unnecessarily due to splatarray true - check_allocations(1, 1, "keyword(*empty_array, a: 2, **empty_hash#{block})") - check_allocations(1, 1, "keyword(*empty_array, **hash1, **empty_hash#{block})") RUBY end @@ -314,8 +309,8 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "keyword_splat(*r2k_array#{block})") # Currently allocates 1 array unnecessarily due to splatarray true - check_allocations(1, 1, "keyword_splat(*empty_array, a: 2, **empty_hash#{block})") - check_allocations(1, 1, "keyword_splat(*empty_array, **hash1, **empty_hash#{block})") RUBY end @@ -346,9 +341,8 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "keyword_and_keyword_splat(*r2k_empty_array#{block})") check_allocations(1, 1, "keyword_and_keyword_splat(*r2k_array#{block})") - # Currently allocates 1 array unnecessarily due to splatarray true - check_allocations(1, 1, "keyword_and_keyword_splat(*empty_array, a: 2, **empty_hash#{block})") - check_allocations(1, 1, "keyword_and_keyword_splat(*empty_array, **hash1, **empty_hash#{block})") RUBY end @@ -391,9 +385,10 @@ class TestAllocation < Test::Unit::TestCase # Currently allocates 1 array unnecessarily due to splatarray true check_allocations(1, 1, "required_and_keyword(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "required_and_keyword(1, *empty_array, **hash1, **empty_hash#{block})") - check_allocations(1, 1, "required_and_keyword(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "required_and_keyword(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 0, "required_and_keyword(*array1, **nil#{block})") RUBY end @@ -482,9 +477,9 @@ class TestAllocation < Test::Unit::TestCase # Currently allocates 1 array unnecessarily due to splatarray true check_allocations(1, 1, "required_and_keyword_splat(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "required_and_keyword_splat(1, *empty_array, **hash1, **empty_hash#{block})") - check_allocations(1, 1, "required_and_keyword_splat(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "required_and_keyword_splat(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 1, "required_and_keyword_splat(*array1, **nil#{block})") RUBY end @@ -569,9 +564,9 @@ class TestAllocation < Test::Unit::TestCase 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(1, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 0, "anon_splat_and_anon_keyword_splat(*array1, **nil#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*r2k_empty_array#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*r2k_array#{block})") @@ -615,9 +610,9 @@ class TestAllocation < Test::Unit::TestCase 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(1, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 0, "anon_splat_and_anon_keyword_splat(*array1, **nil#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*r2k_empty_array#{block})") check_allocations(1, 1, "anon_splat_and_anon_keyword_splat(*r2k_array#{block})") @@ -661,9 +656,9 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "argument_forwarding(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "argument_forwarding(1, *empty_array, **hash1, **empty_hash#{block})") - check_allocations(1, 1, "argument_forwarding(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "argument_forwarding(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 0, "argument_forwarding(*array1, **nil#{block})") check_allocations(0, 0, "argument_forwarding(*r2k_empty_array#{block})") check_allocations(0, 0, "argument_forwarding(*r2k_array#{block})") @@ -707,9 +702,9 @@ class TestAllocation < Test::Unit::TestCase check_allocations(1, 1, "argument_forwarding(1, *empty_array, a: 2, **empty_hash#{block})") check_allocations(1, 1, "argument_forwarding(1, *empty_array, **hash1, **empty_hash#{block})") - check_allocations(1, 1, "argument_forwarding(*array1, **empty_hash, a: 2#{block})") - check_allocations(1, 1, "argument_forwarding(*array1, **hash1, **empty_hash#{block})") - check_allocations(1, 0, "argument_forwarding(*array1, **nil#{block})") check_allocations(0, 0, "argument_forwarding(*r2k_empty_array#{block})") check_allocations(0, 0, "argument_forwarding(*r2k_array#{block})") |