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.
This pull request improves the performance of DefiniitonBuilder for
rbs validate
.How
RBS::DefinitionBuilder#merge_method
is slow (see the profiling section). Because the method creates new object for all method types withsub
method.This pull request reduces method calls of
sub
method.sub
method replaces type parameters, so it is not necessary if the type parameter mapping is empty.Behavior change
It changes behavior of
instance
type. Previouslyinstance
type is replaced withTypes::Bases::Instance.new(location: nil)
withSubstitution
.rbs/lib/rbs/substitution.rb
Line 14 in 77aaea0
But it will be not changed by this patch.
I think it is not a problem because the previous behavior only changes
location
actually. Thelocation
will be kept, but I guess it is not affect anything.Benchmark
I used https://github.com/pocke/rbs_rails for the benchmark because it contains large RBS files.
$ rbs -rlogger -rpathname -rmutex_m -Isig -Iassets/sig validate > /dev/null
before: 15.85s
after: 4.20s
3.77x faster 🚀
Profiling
I used stackprof for the above command, and got the following result.
We can confirm
RBS::DefinitionBuilder#merge_method
takes 37.5% exetuion time.