Short circuit eval regexes in finding module definition #2253
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was profiling gem RBI generation for Tapioca and found that it was spending half its time in
Pry::WrappedModule::Candidate#class_regexes
. I noticed that method evaluates three regexes on every invocation since this change, and have reverted it to the prior short-circuit implementation. I've verified that tests pass locally and that the output of Tapioca RBI generation is unchanged (while running 8% faster). Tapioca has since removed the dependency on this Pry code, but I thought you might find this patch useful regardless.Note that i think there could be a few more possible optimizations here:
\A
) should probably be preferred over the start-of-line anchor (^
) in each regex.match?
should be preferred over=~
once the minimum supported ruby is 2.4#name
are susceptible to the this performance bug, and should be cached in an ivar. I've reduced the invocations by two, but not taken on a larger refactor ofWrappedModule
.