Avoid a hash allocation when calling def f(kw: 1) with keyword splat

c20e819e8b 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).
This commit is contained in:
Jeremy Evans 2024-02-29 10:57:25 -08:00
Родитель a83703af79
Коммит aceee71c35
1 изменённых файлов: 40 добавлений и 10 удалений

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

@ -398,7 +398,7 @@ args_setup_kw_parameters(rb_execution_context_t *const ec, const rb_iseq_t *cons
static void static void
args_setup_kw_parameters_from_kwsplat(rb_execution_context_t *const ec, const rb_iseq_t *const iseq, 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 ID *acceptable_keywords = ISEQ_BODY(iseq)->param.keyword->table;
const int req_key_num = ISEQ_BODY(iseq)->param.keyword->required_num; 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; VALUE missing = 0;
int i, di; int i, di;
int unspecified_bits = 0; int unspecified_bits = 0;
size_t keyword_size = RHASH_SIZE(keyword_hash);
VALUE unspecified_bits_value = Qnil; VALUE unspecified_bits_value = Qnil;
for (i=0; i<req_key_num; i++) { for (i=0; i<req_key_num; i++) {
VALUE key = ID2SYM(acceptable_keywords[i]); VALUE key = ID2SYM(acceptable_keywords[i]);
VALUE deleted_value = rb_hash_delete_entry(keyword_hash, key); VALUE value;
if (!UNDEF_P(deleted_value)) { if (remove_hash_value) {
locals[i] = deleted_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 { else {
if (!missing) missing = rb_ary_hidden_new(1); 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++) { for (di=0; i<key_num; i++, di++) {
VALUE key = ID2SYM(acceptable_keywords[i]); VALUE key = ID2SYM(acceptable_keywords[i]);
VALUE deleted_value = rb_hash_delete_entry(keyword_hash, key); VALUE value;
if (!UNDEF_P(deleted_value)) { if (remove_hash_value) {
locals[i] = deleted_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 { else {
if (UNDEF_P(default_values[di])) { 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; locals[rest_hash_index] = keyword_hash;
} }
else { 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)); 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); args_setup_kw_parameters(ec, iseq, args->kw_argv, kw_arg->keyword_len, kw_arg->keywords, klocals);
} }
else if (!NIL_P(keyword_hash)) { else if (!NIL_P(keyword_hash)) {
keyword_hash = check_kwrestarg(keyword_hash, &kw_flag); bool remove_hash_value = false;
args_setup_kw_parameters_from_kwsplat(ec, iseq, keyword_hash, klocals); 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 { else {
VM_ASSERT(args_argc(args) == 0); VM_ASSERT(args_argc(args) == 0);