Skip to content

Commit 353fa6f

Browse files
committed
Avoid allocation for positional splat for literal array keyword argument
If all nodes in the array are safe, then it is safe to avoid allocation for the positional splat: ```ruby m(*a, kw: [:a]) # Safe m(*a, kw: [meth]) # Unsafe ``` This avoids an unnecessary allocation in a Rails method call. Details: https://github.com/rails/rails/pull/54949/files#r2052645431
1 parent d84a811 commit 353fa6f

File tree

3 files changed

+31
-0
lines changed

3 files changed

+31
-0
lines changed

compile.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6643,6 +6643,14 @@ setup_args_dup_rest_p(const NODE *argn)
66436643
return false;
66446644
case NODE_COLON2:
66456645
return setup_args_dup_rest_p(RNODE_COLON2(argn)->nd_head);
6646+
case NODE_LIST:
6647+
while (argn) {
6648+
if (setup_args_dup_rest_p(RNODE_LIST(argn)->nd_head)) {
6649+
return true;
6650+
}
6651+
argn = RNODE_LIST(argn)->nd_next;
6652+
}
6653+
return false;
66466654
default:
66476655
return true;
66486656
}

prism_compile.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1882,6 +1882,15 @@ pm_setup_args_dup_rest_p(const pm_node_t *node)
18821882
}
18831883
case PM_IMPLICIT_NODE:
18841884
return pm_setup_args_dup_rest_p(((const pm_implicit_node_t *) node)->value);
1885+
case PM_ARRAY_NODE: {
1886+
const pm_array_node_t *cast = (const pm_array_node_t *) node;
1887+
for (size_t index = 0; index < cast->elements.size; index++) {
1888+
if (pm_setup_args_dup_rest_p(cast->elements.nodes[index])) {
1889+
return true;
1890+
}
1891+
}
1892+
return false;
1893+
}
18851894
default:
18861895
return true;
18871896
}

test/ruby/test_allocation.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,13 @@ def self.Object; Object end
807807
check_allocations(0, 1, "keyword(*empty_array, a: ->{}#{block})") # LAMBDA
808808
check_allocations(0, 1, "keyword(*empty_array, a: $1#{block})") # NTH_REF
809809
check_allocations(0, 1, "keyword(*empty_array, a: $`#{block})") # BACK_REF
810+
811+
# LIST: Only 1 array (literal [:c]), not 2 (one for [:c] and one for *empty_array)
812+
check_allocations(1, 1, "keyword(*empty_array, a: empty_array, b: [:c]#{block})")
813+
check_allocations(1, 1, "keyword(*empty_array, a: empty_array, b: [:c, $x]#{block})")
814+
# LIST unsafe: 2 (one for [Object()] and one for *empty_array)
815+
check_allocations(2, 1, "keyword(*empty_array, a: empty_array, b: [Object()]#{block})")
816+
check_allocations(2, 1, "keyword(*empty_array, a: empty_array, b: [:c, $x, Object()]#{block})")
810817
RUBY
811818
end
812819

@@ -877,6 +884,13 @@ def self.Object; Object end
877884
check_allocations(0, 1, "keyword.(*empty_array, a: ->{}#{block})") # LAMBDA
878885
check_allocations(0, 1, "keyword.(*empty_array, a: $1#{block})") # NTH_REF
879886
check_allocations(0, 1, "keyword.(*empty_array, a: $`#{block})") # BACK_REF
887+
888+
# LIST safe: Only 1 array (literal [:c]), not 2 (one for [:c] and one for *empty_array)
889+
check_allocations(1, 1, "keyword.(*empty_array, a: empty_array, b: [:c]#{block})")
890+
check_allocations(1, 1, "keyword.(*empty_array, a: empty_array, b: [:c, $x]#{block})")
891+
# LIST unsafe: 2 (one for [:c] and one for *empty_array)
892+
check_allocations(2, 1, "keyword.(*empty_array, a: empty_array, b: [Object()]#{block})")
893+
check_allocations(2, 1, "keyword.(*empty_array, a: empty_array, b: [:c, $x, Object()]#{block})")
880894
RUBY
881895
end
882896

0 commit comments

Comments
 (0)