Do not clone method entries when bind_call is used

I noticed that this site unconditionally clones the method entry, which
means that `bind_call` always allocates a `T_IMEMO`. While this clone
is necessary for `bind`, it is not necessary for `bind_call`.

I work at Stripe, and the sorbet_runtime gem uses bind call as part
of it's [call validation](https://github.com/sorbet/sorbet/blob/master/gems/sorbet-runtime/lib/types/private/methods/call_validation.rb#L157)
so this can save us a lot of allocations.

This patch adds a `clone` parameter to `convert_umethod_to_method_components`,
which then controls whether or not we do this cloning. This patch passed
Stripe CI and works in our QA environment. I reviewed it with @tenderlove
to talk about correctness also.
This commit is contained in:
Penelope Phippen 2022-08-15 15:45:51 -04:00 коммит произвёл Aaron Patterson
Родитель 4e66b3f47b
Коммит e49db0f760
1 изменённых файлов: 14 добавлений и 4 удалений

18
proc.c
Просмотреть файл

@ -2543,7 +2543,7 @@ rb_method_call_with_block(int argc, const VALUE *argv, VALUE method, VALUE passe
*/
static void
convert_umethod_to_method_components(const struct METHOD *data, VALUE recv, VALUE *methclass_out, VALUE *klass_out, VALUE *iclass_out, const rb_method_entry_t **me_out)
convert_umethod_to_method_components(const struct METHOD *data, VALUE recv, VALUE *methclass_out, VALUE *klass_out, VALUE *iclass_out, const rb_method_entry_t **me_out, const bool clone)
{
VALUE methclass = data->me->owner;
VALUE iclass = data->me->defined_class;
@ -2565,9 +2565,19 @@ convert_umethod_to_method_components(const struct METHOD *data, VALUE recv, VALU
}
}
const rb_method_entry_t *me = rb_method_entry_clone(data->me);
const rb_method_entry_t *me;
if (clone) {
me = rb_method_entry_clone(data->me);
} else {
me = data->me;
}
if (RB_TYPE_P(me->owner, T_MODULE)) {
if (!clone) {
// if we didn't previously clone the method entry, then we need to clone it now
// because this branch manipualtes it in rb_method_entry_complement_defined_class
me = rb_method_entry_clone(me);
}
VALUE ic = rb_class_search_ancestor(klass, me->owner);
if (ic) {
klass = ic;
@ -2627,7 +2637,7 @@ umethod_bind(VALUE method, VALUE recv)
const rb_method_entry_t *me;
const struct METHOD *data;
TypedData_Get_Struct(method, struct METHOD, &method_data_type, data);
convert_umethod_to_method_components(data, recv, &methclass, &klass, &iclass, &me);
convert_umethod_to_method_components(data, recv, &methclass, &klass, &iclass, &me, true);
struct METHOD *bound;
method = TypedData_Make_Struct(rb_cMethod, struct METHOD, &method_data_type, bound);
@ -2669,7 +2679,7 @@ umethod_bind_call(int argc, VALUE *argv, VALUE method)
else {
VALUE methclass, klass, iclass;
const rb_method_entry_t *me;
convert_umethod_to_method_components(data, recv, &methclass, &klass, &iclass, &me);
convert_umethod_to_method_components(data, recv, &methclass, &klass, &iclass, &me, false);
struct METHOD bound = { recv, klass, 0, me };
return call_method_data(ec, &bound, argc, argv, passed_procval, RB_PASS_CALLED_KEYWORDS);