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.
This commit is contained in:
Jeremy Evans 2024-06-19 11:27:01 -07:00
Родитель 48e7112baa
Коммит 0ee3960685
2 изменённых файлов: 103 добавлений и 48 удалений

Просмотреть файл

@ -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)
* Eliminate array 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
* duphash
* send
*/
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)));
VM_CALL_ARGS_SPLAT|VM_CALL_KW_SPLAT|VM_CALL_KW_SPLAT_MUT, VM_CALL_ARGS_BLOCKARG, 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})
* Eliminate array 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
* duphash
* getlocal / getinstancevariable / getblockparamproxy
* send ARGS_SPLAT|KW_SPLAT|ARGS_BLOCKARG
* send
*/
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)));
}
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, 0);
}
}
}
optimized_splat:
if (IS_INSN_ID(iobj, splatarray) && OPERAND_AT(iobj, 0) == false) {
LINK_ELEMENT *niobj = &iobj->link;
if (IS_NEXT_INSN_ID(niobj, duphash)) {
niobj = niobj->next;
LINK_ELEMENT *siobj;
unsigned int set_flags = 0, unset_flags = 0;
/*
* Eliminate hash allocation for f(*a, kw: 1)
*
* splatarray false
* duphash
* send ARGS_SPLAT|KW_SPLAT|KW_SPLAT_MUT and not ARGS_BLOCKARG
* =>
* splatarray false
* putobject
* send ARGS_SPLAT|KW_SPLAT
*/
if (IS_NEXT_INSN_ID(niobj, send)) {
siobj = niobj->next;
set_flags = VM_CALL_ARGS_SPLAT|VM_CALL_KW_SPLAT|VM_CALL_KW_SPLAT_MUT;
unset_flags = VM_CALL_ARGS_BLOCKARG;
}
/*
* Eliminate hash allocation for f(*a, kw: 1, &{arg,lvar,@iv})
*
* splatarray false
* duphash
* getlocal / getinstancevariable / getblockparamproxy
* send ARGS_SPLAT|KW_SPLAT|KW_SPLAT_MUT|ARGS_BLOCKARG
* =>
* splatarray false
* putobject
* getlocal / getinstancevariable / getblockparamproxy
* send ARGS_SPLAT|KW_SPLAT|ARGS_BLOCKARG
*/
else if ((IS_NEXT_INSN_ID(niobj, getlocal) || IS_NEXT_INSN_ID(niobj, getinstancevariable) ||
IS_NEXT_INSN_ID(niobj, getblockparamproxy)) && (IS_NEXT_INSN_ID(niobj->next, send))) {
siobj = niobj->next->next;
set_flags = VM_CALL_ARGS_SPLAT|VM_CALL_KW_SPLAT|VM_CALL_KW_SPLAT_MUT|VM_CALL_ARGS_BLOCKARG;
}
if (set_flags) {
const struct rb_callinfo *ci = (const struct rb_callinfo *)OPERAND_AT(siobj, 0);
unsigned int flags = vm_ci_flag(ci);
if ((flags & set_flags) == set_flags && !(flags & unset_flags)) {
((INSN*)niobj)->insn_id = BIN(putobject);
OPERAND_AT(niobj, 0) = rb_hash_freeze(rb_hash_resurrect(OPERAND_AT(niobj, 0)));
const struct rb_callinfo *nci = vm_ci_new(vm_ci_mid(ci),
flags & ~VM_CALL_KW_SPLAT_MUT, vm_ci_argc(ci), vm_ci_kwarg(ci));
RB_OBJ_WRITTEN(iseq, ci, nci);
OPERAND_AT(siobj, 0) = (VALUE)nci;
}
}
}
}
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 (flag_ptr) *flag_ptr |= VM_CALL_ARGS_SPLAT;
int recurse_dup_rest = 1;
if (nd_type_p(RNODE_ARGSPUSH(argn)->nd_head, NODE_SPLAT) &&
nd_type_p(RNODE_ARGSPUSH(argn)->nd_body, NODE_HASH) &&
!RNODE_HASH(RNODE_ARGSPUSH(argn)->nd_body)->nd_brace) {
recurse_dup_rest = 0;
}
else if (flag_ptr) {
*flag_ptr |= VM_CALL_ARGS_SPLAT_MUT;
}
int argc = setup_args_core(iseq, args, RNODE_ARGSPUSH(argn)->nd_head, recurse_dup_rest, 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})")
check_allocations(0, 1, "keyword(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 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})")
check_allocations(0, 1, "keyword_splat(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 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})")
check_allocations(0, 1, "keyword_and_keyword_splat(*empty_array, a: 2, **empty_hash#{block})")
check_allocations(0, 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})")
check_allocations(0, 1, "required_and_keyword(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "required_and_keyword(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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})")
check_allocations(0, 1, "required_and_keyword_splat(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "required_and_keyword_splat(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "anon_splat_and_anon_keyword_splat(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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, 1, "argument_forwarding(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "argument_forwarding(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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, 1, "argument_forwarding(*array1, **empty_hash, a: 2#{block})")
check_allocations(0, 1, "argument_forwarding(*array1, **hash1, **empty_hash#{block})")
check_allocations(0, 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})")