From e49db0f760722bf44ed2c5b31f67d929e9156dbe Mon Sep 17 00:00:00 2001 From: Penelope Phippen Date: Mon, 15 Aug 2022 15:45:51 -0400 Subject: 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. --- proc.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'proc.c') diff --git a/proc.c b/proc.c index c234ed3f93..3c52fb06a7 100644 --- a/proc.c +++ b/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); -- cgit v1.2.1