From 4a3e7984bfbaab45cbd6e73373ee1bba5ce21dc1 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 7 Dec 2021 15:18:11 -0800 Subject: [PATCH] Avoid Array allocation when appending to args array (#5211) * Use duparray when possible for argspush ARGSPUSH is the node we see with a single value pushed to the end of a splatted array. ARGSCAT is similar, but is used when multiple values are being concatenated to the list. Previously only ARGSCAT had an optimization where when all the values were static it would use duparray instead of newarray to create the intermediate array. This commit adds similar behaviour for ARGSPUSH, using duparray instead of putobject/newarray. * Replace duparray with putobject before concatarray When performing duparray/concatarray we know we'll never use the intermediate array being created by duparray, so we should be able to use it as a temporary object. This avoids an extra array allocation for NODE_ARGSPUSH (ex. [*foo, 1]) and NODE_ARGSCAT (ex. [*foo, 1, 2]). --- compile.c | 56 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/compile.c b/compile.c index fc9f2eee6b..82593d8774 100644 --- a/compile.c +++ b/compile.c @@ -3053,6 +3053,22 @@ iseq_peephole_optimize(rb_iseq_t *iseq, LINK_ELEMENT *list, const int do_tailcal remove_unreachable_chunk(iseq, iobj->link.next); } + /* + * ... + * duparray [...] + * concatarray + * => + * ... + * putobject [...] + * concatarray + */ + if (IS_INSN_ID(iobj, duparray)) { + LINK_ELEMENT *next = iobj->link.next; + if (IS_INSN(next) && IS_INSN_ID(next, concatarray)) { + iobj->insn_id = BIN(putobject); + } + } + if (IS_INSN_ID(iobj, branchif) || IS_INSN_ID(iobj, branchnil) || IS_INSN_ID(iobj, branchunless)) { @@ -4178,7 +4194,6 @@ compile_args(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, static inline int static_literal_node_p(const NODE *node, const rb_iseq_t *iseq) { - node = node->nd_head; switch (nd_type(node)) { case NODE_LIT: case NODE_NIL: @@ -4195,7 +4210,6 @@ static_literal_node_p(const NODE *node, const rb_iseq_t *iseq) static inline VALUE static_literal_value(const NODE *node, rb_iseq_t *iseq) { - node = node->nd_head; switch (nd_type(node)) { case NODE_NIL: return Qnil; @@ -4294,10 +4308,10 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int pop int count = 1; /* pre-allocation check (this branch can be omittable) */ - if (static_literal_node_p(node, iseq)) { + if (static_literal_node_p(node->nd_head, iseq)) { /* count the elements that are optimizable */ const NODE *node_tmp = node->nd_next; - for (; node_tmp && static_literal_node_p(node_tmp, iseq); node_tmp = node_tmp->nd_next) + for (; node_tmp && static_literal_node_p(node_tmp->nd_head, iseq); node_tmp = node_tmp->nd_next) count++; if ((first_chunk && stack_len == 0 && !node_tmp) || count >= min_tmp_ary_len) { @@ -4306,7 +4320,7 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int pop /* Create a hidden array */ for (; count; count--, node = node->nd_next) - rb_ary_push(ary, static_literal_value(node, iseq)); + rb_ary_push(ary, static_literal_value(node->nd_head, iseq)); OBJ_FREEZE(ary); /* Emit optimized code */ @@ -4348,10 +4362,29 @@ compile_array(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int pop return 1; } +/* Compile an array containing the single element represented by node */ +static int +compile_array_1(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node) +{ + if (static_literal_node_p(node, iseq)) { + VALUE ary = rb_ary_tmp_new(1); + rb_ary_push(ary, static_literal_value(node, iseq)); + OBJ_FREEZE(ary); + + ADD_INSN1(ret, node, duparray, ary); + } + else { + CHECK(COMPILE_(ret, "array element", node, FALSE)); + ADD_INSN1(ret, node, newarray, INT2FIX(1)); + } + + return 1; +} + static inline int static_literal_node_pair_p(const NODE *node, const rb_iseq_t *iseq) { - return node->nd_head && static_literal_node_p(node, iseq) && static_literal_node_p(node->nd_next, iseq); + return node->nd_head && static_literal_node_p(node->nd_head, iseq) && static_literal_node_p(node->nd_next->nd_head, iseq); } static int @@ -4436,8 +4469,8 @@ compile_hash(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int meth /* Create a hidden hash */ for (; count; count--, node = node->nd_next->nd_next) { VALUE elem[2]; - elem[0] = static_literal_value(node, iseq); - elem[1] = static_literal_value(node->nd_next, iseq); + elem[0] = static_literal_value(node->nd_head, iseq); + elem[1] = static_literal_value(node->nd_next->nd_head, iseq); rb_ary_cat(ary, elem, 2); } VALUE hash = rb_hash_new_with_size(RARRAY_LEN(ary) / 2); @@ -9490,15 +9523,14 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const no } case NODE_ARGSPUSH:{ if (popped) { - CHECK(COMPILE(ret, "arsgpush head", node->nd_head)); + CHECK(COMPILE(ret, "argspush head", node->nd_head)); ADD_INSN1(ret, node, splatarray, Qfalse); ADD_INSN(ret, node, pop); CHECK(COMPILE_(ret, "argspush body", node->nd_body, popped)); } else { - CHECK(COMPILE(ret, "arsgpush head", node->nd_head)); - CHECK(COMPILE_(ret, "argspush body", node->nd_body, popped)); - ADD_INSN1(ret, node, newarray, INT2FIX(1)); + CHECK(COMPILE(ret, "argspush head", node->nd_head)); + CHECK(compile_array_1(iseq, ret, node->nd_body)); ADD_INSN(ret, node, concatarray); } break;