summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPenelope Phippen <penelope@hey.com>2022-08-15 15:45:51 -0400
committerAaron Patterson <aaron.patterson@gmail.com>2022-08-15 15:41:35 -0700
commite49db0f760722bf44ed2c5b31f67d929e9156dbe (patch)
treed71130b269fdfe16d070fc39fee336d62b5bd703
parent4e66b3f47b2ad0d6cca1f2227dd38fdf117c0d3c (diff)
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.
Notes
Notes: Merged: https://github.com/ruby/ruby/pull/6244
-rw-r--r--proc.c18
1 files 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);