From e49db0f760722bf44ed2c5b31f67d929e9156dbe Mon Sep 17 00:00:00 2001
From: Penelope Phippen <penelope@hey.com>
Date: Mon, 15 Aug 2022 15:45:51 -0400
Subject: [PATCH] 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(-)

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);