Skip to content

Commit

Permalink
Avoid allocation when passing no keywords to anonymous kwrest methods
Browse files Browse the repository at this point in the history
Thanks to the new semantics from [ruby-core:115808], `**nil` is now
equivalent to `**{}`. Since the only thing one could do with anonymous
keyword rest parameter is to delegate it with `**`, nil is just as good
as an empty hash. Using nil avoids allocating an empty hash.

This is particularly important for `...` methods since they now use
`**kwrest` under the hood after 4f77d8d. Most calls don't pass
keywords.

    Comparison:
                             fw_no_kw
                    post:   9816800.9 i/s
                     pre:   8570297.0 i/s - 1.15x  slower
  • Loading branch information
XrXr committed Feb 13, 2024
1 parent 4e481c7 commit e4272fd
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
2 changes: 2 additions & 0 deletions benchmark/vm_method_splat_calls2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ prelude: |
def anon_fw_to_named(*, **) named_arg_kw_splat(*, **) end
def fw_to_named(...) named_arg_kw_splat(...) end
def fw_to_anon_to_named(...) anon_fw_to_named(...) end
def fw_no_kw(...) named_arg_splat(...) end
a = [1]
kw = {y: 1}
benchmark:
Expand All @@ -23,3 +24,4 @@ benchmark:
fw_to_named_no_splat: "fw_to_named(1, y: 1)"
fw_to_anon_to_named_splat: "fw_to_anon_to_named(*a, **kw)"
fw_to_anon_to_named_no_splat: "fw_to_anon_to_named(1, y: 1)"
fw_no_kw: "fw_no_kw(1, 2)"
9 changes: 6 additions & 3 deletions vm_args.c
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,12 @@ args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb
}

static inline void
args_setup_kw_rest_parameter(VALUE keyword_hash, VALUE *locals, int kw_flag)
args_setup_kw_rest_parameter(VALUE keyword_hash, VALUE *locals, int kw_flag, bool anon_kwrest)
{
if (NIL_P(keyword_hash)) {
keyword_hash = rb_hash_new();
if (!anon_kwrest) {
keyword_hash = rb_hash_new();
}
}
else if (!(kw_flag & VM_CALL_KW_SPLAT_MUT)) {
keyword_hash = rb_hash_dup(keyword_hash);
Expand Down Expand Up @@ -816,7 +818,8 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
}
}
else if (ISEQ_BODY(iseq)->param.flags.has_kwrest) {
args_setup_kw_rest_parameter(keyword_hash, locals + ISEQ_BODY(iseq)->param.keyword->rest_start, kw_flag);
args_setup_kw_rest_parameter(keyword_hash, locals + ISEQ_BODY(iseq)->param.keyword->rest_start,
kw_flag, ISEQ_BODY(iseq)->param.flags.anon_kwrest);
}
else if (!NIL_P(keyword_hash) && RHASH_SIZE(keyword_hash) > 0 && arg_setup_type == arg_setup_method) {
argument_kw_error(ec, iseq, "unknown", rb_hash_keys(keyword_hash));
Expand Down

0 comments on commit e4272fd

Please sign in to comment.