summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeremy Evans <code@jeremyevans.net>2024-02-29 10:57:25 -0800
committerJeremy Evans <code@jeremyevans.net>2024-03-15 08:23:41 -0700
commitaceee71c35e0b387691836e756b4e008efd84cf1 (patch)
tree26346d4fd131a7b7bcc8247bc0811fb38594c051
parenta83703af79dca708736a00d45917f19a3dd698bb (diff)
Avoid a hash allocation when calling def f(kw: 1) with keyword splat
c20e819e8b04e84a4103ca9a036022543459c213 made calling a method that accepts specific keywords but not a keyword splat with a keyword splat allocating a hash when it previously did not. Instead of duplicating the hash and removing hash entries, lookup in the provided hash without duplicating the hash, and track to make sure all hash entries in the hash were looked up (extra hash entries would be errors).
-rw-r--r--vm_args.c50
1 files changed, 40 insertions, 10 deletions
diff --git a/vm_args.c b/vm_args.c
index ec5df37aab..d1a7695c6e 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -398,7 +398,7 @@ args_setup_kw_parameters(rb_execution_context_t *const ec, const rb_iseq_t *cons
static void
args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb_iseq_t *const iseq,
- VALUE keyword_hash, VALUE *const locals)
+ VALUE keyword_hash, VALUE *const locals, bool remove_hash_value)
{
const ID *acceptable_keywords = ISEQ_BODY(iseq)->param.keyword->table;
const int req_key_num = ISEQ_BODY(iseq)->param.keyword->required_num;
@@ -407,13 +407,22 @@ args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb
VALUE missing = 0;
int i, di;
int unspecified_bits = 0;
+ size_t keyword_size = RHASH_SIZE(keyword_hash);
VALUE unspecified_bits_value = Qnil;
for (i=0; i<req_key_num; i++) {
VALUE key = ID2SYM(acceptable_keywords[i]);
- VALUE deleted_value = rb_hash_delete_entry(keyword_hash, key);
- if (!UNDEF_P(deleted_value)) {
- locals[i] = deleted_value;
+ VALUE value;
+ if (remove_hash_value) {
+ value = rb_hash_delete_entry(keyword_hash, key);
+ }
+ else {
+ value = rb_hash_lookup2(keyword_hash, key, Qundef);
+ }
+
+ if (!UNDEF_P(value)) {
+ keyword_size--;
+ locals[i] = value;
}
else {
if (!missing) missing = rb_ary_hidden_new(1);
@@ -425,9 +434,17 @@ args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb
for (di=0; i<key_num; i++, di++) {
VALUE key = ID2SYM(acceptable_keywords[i]);
- VALUE deleted_value = rb_hash_delete_entry(keyword_hash, key);
- if (!UNDEF_P(deleted_value)) {
- locals[i] = deleted_value;
+ VALUE value;
+ if (remove_hash_value) {
+ value = rb_hash_delete_entry(keyword_hash, key);
+ }
+ else {
+ value = rb_hash_lookup2(keyword_hash, key, Qundef);
+ }
+
+ if (!UNDEF_P(value)) {
+ keyword_size--;
+ locals[i] = value;
}
else {
if (UNDEF_P(default_values[di])) {
@@ -462,7 +479,16 @@ args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb
locals[rest_hash_index] = keyword_hash;
}
else {
- if (!RHASH_EMPTY_P(keyword_hash)) {
+ if (!remove_hash_value) {
+ if (keyword_size != 0) {
+ /* Recurse with duplicated keyword hash in remove mode.
+ * This is simpler than writing code to check which entries in the hash do not match.
+ * This will raise an exception, so the additional performance impact shouldn't be material.
+ */
+ args_setup_kw_parameters_from_kwsplat(ec, iseq, rb_hash_dup(keyword_hash), locals, true);
+ }
+ }
+ else if (!RHASH_EMPTY_P(keyword_hash)) {
argument_kw_error(ec, iseq, "unknown", rb_hash_keys(keyword_hash));
}
}
@@ -808,8 +834,12 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
args_setup_kw_parameters(ec, iseq, args->kw_argv, kw_arg->keyword_len, kw_arg->keywords, klocals);
}
else if (!NIL_P(keyword_hash)) {
- keyword_hash = check_kwrestarg(keyword_hash, &kw_flag);
- args_setup_kw_parameters_from_kwsplat(ec, iseq, keyword_hash, klocals);
+ bool remove_hash_value = false;
+ if (ISEQ_BODY(iseq)->param.flags.has_kwrest) {
+ keyword_hash = check_kwrestarg(keyword_hash, &kw_flag);
+ remove_hash_value = true;
+ }
+ args_setup_kw_parameters_from_kwsplat(ec, iseq, keyword_hash, klocals, remove_hash_value);
}
else {
VM_ASSERT(args_argc(args) == 0);