Improve require performance, particularly on systems with a lot of gems installed

[pocke]

This patch improves the performance of Specification#missing_extensions? method.

What was the end-user or developer problem that led to this PR?

missing_extensions? is slow, because it calls default_gem? method and it allocates String instances.

What is your fix for the problem, implemented in this PR?

I benchmarked require 'rails', then I found a bottleneck about missing_extensions?.
Note that there are many gems (gem list | wc -l # => 481) in my environment.

Profilings

Gem::Specification#missing_extensions? takes 6.4% of require 'rails', and default_gem? takes 4.4%.

require 'stackprof'
StackProf.run(mode: :wall, out: 'stackprof-out', raw: true) do
  require "rails"
end

$ stackprof stackprof-out -m Gem::Specification#missing_extensions
Gem::Specification#missing_extensions? (/home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/specification.rb:2122)
  samples:     0 self (0.0%)  /     65 total (6.4%)
  callers:
      65  (  100.0%)  Gem::BasicSpecification#contains_requirable_file?
  callees (65 total):
      45  (   69.2%)  Gem::BasicSpecification#default_gem?
      14  (   21.5%)  File.exist?
       6  (    9.2%)  Gem::BasicSpecification#gem_build_complete_path
  code:
                                  |  2122  |   def missing_extensions?
   45    (4.4%)                   |  2123  |     return false if default_gem?
                                  |  2124  |     return false if extensions.empty?
   20    (2.0%)                   |  2125  |     return false if File.exist? gem_build_complete_path
                                  |  2126  | 

$ stackprof stackprof-out -m default_gem?
Gem::BasicSpecification#default_gem? (/home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/basic_specification.rb:89)
  samples:     1 self (0.1%)  /     45 total (4.4%)
  callers:
      45  (  100.0%)  Gem::Specification#missing_extensions?
  callees (44 total):
      31  (   70.5%)  Gem.default_specifications_dir
      13  (   29.5%)  File.dirname
  code:
                                  |    89  |   def default_gem?
                                  |    90  |     loaded_from &&
   44    (4.3%)                   |    91  |       File.dirname(loaded_from) == Gem.default_specifications_dir
    1    (0.1%) /     1   (0.1%)  |    92  |   end
                                  |    93  | 

Because default_gem? allocates two String instances with File.dirname(loaded_from) and Gem.default_specifications_dir.
I confirmed it with memory_profiler gem.

# m.rb
require 'memory_profiler'

report = MemoryProfiler.report do
  require "rails"
end

report.pretty_print

$ ruby m.rb
(snip)

Allocated String Report
-----------------------------------
     25246  "/home/pocke/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/specifications/default"
     23150  /home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/defaults.rb:62
      2096  /home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/basic_specification.rb:91

     21178  "/home/pocke/.rbenv/versions/trunk/lib/ruby/gems/3.1.0/specifications"
     21054  /home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/basic_specification.rb:91
       124  /home/pocke/.rbenv/versions/trunk/lib/ruby/3.1.0/rubygems/specification.rb:2032

(snip)

How to fix

I've fixed this problem with two solutions.

First, swap default_gem? and extensions.empty? order.
Previously missing_extensions? first called default_gem?, then extensions.empty?. But extensions.empty? is faster than default_gem? because it just calls Array#empty?. So I swapped them to avoid calling default_gem? method if extensions.empty? == true.

Second, cache Gem.default_specifications_dir. It depends on Gem.default_dir but Gem.default_dir is also cached. So I think caching Gem.default_specification_dir is no problem too.

Micro Benchmark

5x faster 🚀

require 'benchmark'


N = 1000

specs = Gem::Specification.each.to_a

Benchmark.bmbm(20) do |x|
  x.report(){N.times{specs.each{|s|s.missing_extensions?}}}
end

# Before
$ ruby -Ilib bench.rb
Rehearsal --------------------------------------------------------
                       1.386008   0.106482   1.492490 (  1.493904)
----------------------------------------------- total: 1.492490sec

                           user     system      total        real
                       1.386751   0.096393   1.483144 (  1.484664)

# After
$ ruby -Ilib bench.rb
Rehearsal --------------------------------------------------------
                       0.223023   0.073411   0.296434 (  0.296806)
----------------------------------------------- total: 0.296434sec

                           user     system      total        real
                       0.206841   0.089902   0.296743 (  0.297007)

Real-world benchmark

1.059x faster 🚀

require 'benchmark'

Benchmark.bmbm(20) do |x|
  x.report{10.times{system 'ruby', '-Ilib', '-e', 'require "rails"'}}
end

# before
$ ruby t.rb
Rehearsal --------------------------------------------------------
                       0.000000   0.002783  10.914849 ( 10.943345)
---------------------------------------------- total: 10.914849sec

                           user     system      total        real
                       0.002203   0.000518  10.974635 ( 11.004077)

# after
$ ruby t.rb
Rehearsal --------------------------------------------------------
                       0.000000   0.002462  10.550915 ( 10.580447)
---------------------------------------------- total: 10.550915sec

                           user     system      total        real
                       0.002003   0.000095  10.359463 ( 10.386789)

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[welcome]

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

[deivid-rodriguez]

This is awesome, thank you!

[deivid-rodriguez]

There's one spec failure that needs some tweaks in bundler to pass now.

[pocke]

I've applied another fix to solve CI failure on JRuby. 825d0e4

It works in my environment, but I'm not sure it is the right way. It cleans default_specifications_dir cache but doesn't clean default_dir cache, it looks a bit strange 🤔

[deivid-rodriguez]

@pocke Your fix looks good to me. JRuby monkeypatches the Gem.default_dir method so that it's based off RbConfig::CONFIG["default_gem_home"] and it doesn't memoize it.

https://github.com/jruby/jruby/blob/a309a88614916621de4cc5dc3693f279dae58d0c/lib/ruby/stdlib/rubygems/defaults/jruby.rb#L30-L40

[deivid-rodriguez]

Can you rebase and squash everything into one commit?

[pocke]

@pocke Your fix looks good to me. JRuby monkeypatches the Gem.default_dir method so that it's based off RbConfig::CONFIG["default_gem_home"] and it doesn't memoize it.

https://github.com/jruby/jruby/blob/a309a88614916621de4cc5dc3693f279dae58d0c/lib/ruby/stdlib/rubygems/defaults/jruby.rb#L30-L40

I didn't know that. Thanks!

Can you rebase and squash everything into one commit?

Sure, I'll do that 🚀