Skip to content

Candidate fix for #258

gitlab-qa-bot requested to merge get_original_method_early into master

Created by: myronmarston

This is one possible fix for #258. Pros:

  • This is far simpler than the older code. Deleting code is good!
  • This solves #258.

Cons:

  • It still needs special casing code for any_instance. (I'd love to not need that...)
  • This causes a performance slowdown. I haven't formally benchmarked this, but in running rspec-mocks master vs this branch I was seeing about a 10% slowdown from this. (Note that users would never see a slowdown that bad; that's just because these specs are testing rspec-mocks and are therefore stubbing or mocking methods all over the place). I had originally considered this solution when I first wrote and_call_original, but rejected it due to concerns about the perf hit we'd take to always get the original method handle even though and_call_original is a rarely used feature. I also originally thought that the other way that I wound up doing it wouldn't be too complicated; in practice, it has gotten more complicated over time and is practically re-implementing ruby's method dispatch system.

Can anyone figure out a way to do something like this w/o taking the perf hit?

/cc @alindeman @dchelimsky

Merge request reports