Rework plugins system and speed up rubygems #3108

deivid-rodriguez · 2020-01-27T00:20:43Z

Description:

I'm pretty excited about this work. The idea is to speed up rubygems require time. This is the improvement in my computer:

Before

$ time gem env version
NOTE: Gem::Specification#rubyforge_project= is deprecated with no replacement. It will be removed on or after 2019-12-01.
Gem::Specification#rubyforge_project= called from /home/deivid/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/specifications/erubis-2.7.0.gemspec:17.
3.1.2

real	0m0,235s
user	0m0,230s
sys	0m0,007s

After

$ time gem env version
3.2.0.pre1

real	0m0,103s
user	0m0,099s
sys	0m0,007s

The current (slow) rubygems plugin system

The idea comes from noticing that the rubygems plugin system is quite heavy because everytime rubygems is required, it looks in all installed gems for files named rubygems_plugin.rb, so that it can load them. This is not just a simple system file search, rubygems also needs to evaluate every gemspec, to know what the require_paths for each gem are, and look for a rubygems_plugin.rb file in there.

As you can imagine, this is quite inefficient. And also, it explains why simple commands such as gem env version print warnings about deprecated gemspec syntax (#2984).

The new (faster) rubygems plugin system

Instead, I propose to keep rubygems plugins in a well known location, so when rubygems is required, it simply needs to require whatever plugins are in there. This is, of course, much faster and does not require to look for files or evaluate gemspecs.

The tricky part is to keep this new plugins location up to date. To achieve that, we do something similar to what's currently done with binstubs. When we install a gem, we also install a plugin wrapper in there that simply requires the plugin inside the gem. When we uninstall it, we also delete that wrapper. gem update --system regenerates plugins, automatically migrating rubygems to the new layout.

Tasks:

Describe the problem / feature
Write tests
Write code to solve the problem
Get code review from coworkers / friends

I will abide by the code of conduct.

@bronzdoc

3109: Cleanup some stuff r=bronzdoc a=deivid-rodriguez # Description: This is some code cleanup that I made while working on #3108. To make that PR as focused as possible and make its future review easier, I'm extracting these changes here to be reviewed and merged separately. /cc @bronzdoc Since you're doing some spring cleanup :) # Tasks: - [x] Describe the problem / feature - [ ] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

@bronzdoc

3109: Cleanup some stuff r=bronzdoc a=deivid-rodriguez # Description: This is some code cleanup that I made while working on #3108. To make that PR as focused as possible and make its future review easier, I'm extracting these changes here to be reviewed and merged separately. /cc @bronzdoc Since you're doing some spring cleanup :) # Tasks: - [x] Describe the problem / feature - [ ] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

3115: Fix incorrect test to test what it wants to test r=deivid-rodriguez a=deivid-rodriguez # Description: This PR fixes an incorrect test that I found while working on #3108. Previously `util_setup_installer` was ignoring the block passed to it, the user installed gem was not even named "default". However, since it installed an executable with the same name as the default gem, the test still passed. # Tasks: - [x] Describe the problem / feature - [ ] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

deivid-rodriguez · 2020-01-30T18:22:15Z

I updated the PR description, and the implementation should be ready.

I'm still thinking of adding some higher level tests, but I'm not sure whether to add them to test/rubygems/test_gem_commands_install_command.rb and test/rubygems/test_gem_commands_uninstall_command.rb, or to add a realworld tests similar to the one I've been running locally:

rm -f ~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/plugins/yard_plugin.rb && rake install && gem install yard:0.9.23 && cat ~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/plugins/yard_plugin.rb && gem install yard:0.9.24 && cat ~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/plugins/yard_plugin.rb && gem uninstall yard:0.24.0 --executables --backtrace && cat ~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/plugins/yard_plugin.rb && gem uninstall yard:0.23.0 --executables --backtrace && ls ~/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/plugins/ -la

Essentially, install and uninstall gems with plugins and make sure the plugins location is properly updated.

deivid-rodriguez · 2020-01-30T18:24:30Z

Oh, also something to be noticed, if you try this locally and don't see a great improvement, it might be because you have etiher the executable-hooks or the gem-wrappers gems installed. These gems have rubygems plugins that are quite slow, so that's the bottleneck in that case, so the improvement is not apparent. But if you have yard installed, which also includes a rubygems plugin, it should still be much faster.

indirect

This is awesome! Super exciting to see things get faster. 😁

The spec should never be `nil` at this point.

Take the following scenario: * You have a rubygems version with the new plugins system. * You run `gem install yard`, so that `<gem_home>/plugins/yard_plugin.rb` is generared. * You downgrade rubygems to a version with the old plugin system, say, 3.1.2. * You uninstall yard. * You upgrade rubygems again. In this case, the final upgrade will fail because rubygems will try to require a `yard` rubygems plugin to points to a file that does not exist because the `yard` gem is not installed. This change fixes that problem.

deivid-rodriguez · 2020-01-31T20:49:07Z

Ok, so I found and fixed some bugs, and added a bunch of tests, so I'm happy with this now! Removing the WIP tag :)

deivid-rodriguez · 2020-01-31T20:52:49Z

lib/rubygems.rb


  def self.load_plugins
-    load_plugin_files find_latest_files('rubygems_plugin', false)
+    load_plugin_files Gem::Util.glob_files_in_dir("*#{Gem.plugin_suffix_pattern}", plugins_dir)


This is the most important change in this PR.

We go from:

Look for requirable files named 'rubygems_plugin' in all installed gems, which means looking in every installed gem's folder, evaluate its gemspec to find its 'require_paths' (usually lib/) and actually look in that folder.

to:

Require any files in the standard rubygems plugins folder.

The rest of the code in this PR is essentially code to be able to do this particular change safely and in a backwards compatible way.

Will this respect $LOAD_PATH? If I activate a gem, I'd expect its plugin to be added to gem, and if I deactivate it, I'd expect its plugin not to be added.

What do you mean exactly by "I expect its plugin to be added to gem"? In any case, the end user behavior of rubygems should be unchanged by this PR.

No problem! So the current way to load plugins is to scan for all active gems that have a rubygems_plugin.rb in the lib folder. So, if a gem isn't activated, its plugins aren't added, and when it is activated (by having its lib folder listed in the $LOAD_PATH), its plugins are added.

What I mean by "I expect its plugin to be added to gem" is just the first part, the normal, regular expected behaviour. I mentioned it to be symmetrical with the deactivation case.

I think, with this refactor, if I install a gem, its plugin will be added to gem regardless of if the gem is activated or not. With the pre-refactor implementation, gems that weren't activated didn't have their plugins added to gem, because activation meant "had their gem lib folder added to $LOAD_PATH", and that's where Gem::Specification.latest_specs looks.

I'm pretty sure it doesn't work the way you think, although I agree it would probably make more sense the way you explain, but I didn't want to change the behaviour just now.

Rubygems loads plugins for the latest version of every installed gem (in addition to plugins present in the $LOAD_PATH), no matter whether it has yet been activated or not.

The ones that register themselves by creating a file in Gem.plugins_dir.

That is all rubygems plugins actually. If your gem includes a rubygems_plugin.rb file, it will be registered like that in the next rubygems version. In other words, gem-compiler is an example of such a plugin, and it works as expected for me.

ah, because the load plugin statement in load_plugin_files catches the exception when the require fails, because the gem is not in the $LOAD_PATH. So, the answer to my original question is 'yes, it does respect $LOAD_PATH'.

Will you be removing the call to load_env_plugins in the future?

I don't think that rescue is actually involved here, since the require uses absolute paths, so it does not involve $LOAD_PATH. The reason this "works" is that even if the "compile" command is actually registered, it can never be loaded when running through bundler since the command file is not on the $LOAD_PATH.

See the error against current rubygems:

$ bundle exec gem compile ERROR: While executing gem ... (Gem::CommandLineError) Unknown command compile

vs the new error

$ bundle exec gem compile ERROR: Loading command: compile (LoadError) cannot load such file -- rubygems/commands/compile_command ERROR: While executing gem ... (NoMethodError) undefined method `invoke_with_build_args' for nil:NilClass

This means we do need to adapt bundler to work with the new plugin system.

I'm working on a fix for this here.

Ok, so I spent a fair bit of time on this today. Turns out the different error messages are unrelated to this change in rubygems, but depend on the fact whether gem-compiler is installed globally (in addition to through the Gemfile or not).

Even if not caused by this particular change in rubygems, I think that is a bug since the global environment should never "pollute" a bundle exec context like that, so I'm fixing it in the branch I pointed to.

hsbt

Finished to review. I understood your proposal and the new plugin layout. It's the great works!

deivid-rodriguez · 2020-02-01T00:20:40Z

Thanks both for having a look, let's do it!

@bundlerbot merge

3108: Rework plugins system and speed up rubygems r=deivid-rodriguez a=deivid-rodriguez # Description: I'm pretty excited about this work. The idea is to speed up rubygems require time. This is the improvement in my computer: #### Before ``` $ time gem env version NOTE: Gem::Specification#rubyforge_project= is deprecated with no replacement. It will be removed on or after 2019-12-01. Gem::Specification#rubyforge_project= called from /home/deivid/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/specifications/erubis-2.7.0.gemspec:17. 3.1.2 real 0m0,235s user 0m0,230s sys 0m0,007s ``` #### After ``` $ time gem env version 3.2.0.pre1 real 0m0,103s user 0m0,099s sys 0m0,007s ``` ### The current (slow) rubygems plugin system The idea comes from noticing that the rubygems plugin system is quite heavy because everytime rubygems is required, it looks in _all_ installed gems for files named `rubygems_plugin.rb`, so that it can load them. This is not just a simple system file search, rubygems also needs to evaluate every gemspec, to know what the `require_paths` for each gem are, and look for a `rubygems_plugin.rb` file in there. As you can imagine, this is quite inefficient. And also, it explains why simple commands such as `gem env version` print warnings about deprecated gemspec syntax (#2984). ### The new (faster) rubygems plugin system Instead, I propose to keep rubygems plugins in a well known location, so when rubygems is required, it simply needs to require whatever plugins are in there. This is, of course, much faster and does not require to look for files or evaluate gemspecs. The tricky part is to keep this new plugins location up to date. To achieve that, we do something similar to what's currently done with binstubs. When we install a gem, we also install a plugin wrapper in there that simply requires the plugin inside the gem. When we uninstall it, we also delete that wrapper. `gem update --system` regenerates plugins, automatically migrating rubygems to the new layout. # Tasks: - [x] Describe the problem / feature - [x] Write tests - [x] Write code to solve the problem - [ ] Get code review from coworkers / friends I will abide by the [code of conduct](https://github.com/rubygems/rubygems/blob/master/CODE_OF_CONDUCT.md). Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

ghost · 2020-02-01T01:05:44Z

Build succeeded

install (2.3.8)
install (2.4.9)
install (2.5.7)
install (2.6.5)
install (jruby-9.2.9.0)
macos (2.4.x)
macos (2.5.x)
macos (2.6.x)
ruby_master
ubuntu (2.4.x, bundler)
ubuntu (2.4.x, rubygems)
ubuntu (2.5.x, bundler)
ubuntu (2.5.x, rubygems)
ubuntu (2.6.x, bundler)
ubuntu (2.6.x, rubygems)
ubuntu_bundler_master (2.6.x)
ubuntu_lint
ubuntu_rvm (2.3.8)
ubuntu_rvm (jruby-9.2.9.0)
ubuntu_rvm (ruby-head)
windows (2.4.x)
windows (2.5.x)
windows (2.6.x)

Rubygems plugin loading was changed in rubygems >= 3.2 this leads to gem-wrappers not being loaded in a new context. This fix registers the gem-wrappers plugin before its being used. See rubygems/rubygems#3108

Originally, when the plugins system was reworked as part of rubygems#3108, the plugin directory was considered global. But that is mistake. The plugins are always related to specific gems and therefore the directories should be handled via `Gem::Specification`. This will prevent issues such as leftovers after gem uninstallation as demonstred by rubygems#6452: ~~~ $ gem uni rubygems-server Successfully uninstalled rubygems-server-0.3.0 $ gem install rubygems-server --document=ri,rdoc Error loading RubyGems plugin "/builddir/.local/share/gem/ruby/plugins/rubygems-server_plugin.rb": cannot load such file -- /builddir/.local/share/gem/ruby/gems/rubygems-server-0.3.0/lib/rubygems_plugin.rb (LoadError) Fetching rubygems-server-0.3.0.gem WARNING: You don't have /builddir/bin in your PATH, gem executables will not run. Successfully installed rubygems-server-0.3.0 Parsing documentation for rubygems-server-0.3.0 Installing ri documentation for rubygems-server-0.3.0 Installing fedora::darkfish documentation for rubygems-server-0.3.0 Done installing documentation for rubygems-server after 0 seconds 1 gem installed ~~~

bronzdoc added the status: triage label Jan 27, 2020

deivid-rodriguez added the wip label Jan 27, 2020

deivid-rodriguez force-pushed the rework_plugins_and_speed_up_rubygems branch from bbad3b7 to 62a3776 Compare January 27, 2020 00:26

deivid-rodriguez mentioned this pull request Jan 27, 2020

Cleanup some stuff #3109

Merged

4 tasks

deivid-rodriguez mentioned this pull request Jan 30, 2020

Fix incorrect test to test what it wants to test #3115

Merged

4 tasks

deivid-rodriguez force-pushed the rework_plugins_and_speed_up_rubygems branch 2 times, most recently from 0068356 to 5e9d761 Compare January 30, 2020 17:59

indirect approved these changes Jan 30, 2020

View reviewed changes

deivid-rodriguez force-pushed the rework_plugins_and_speed_up_rubygems branch from 5e9d761 to 71388a8 Compare January 31, 2020 11:28

Remove unnecessary nil check

f48cb73

The spec should never be `nil` at this point.

deivid-rodriguez force-pushed the rework_plugins_and_speed_up_rubygems branch 3 times, most recently from 8567139 to b3edaa5 Compare January 31, 2020 19:57

deivid-rodriguez added 2 commits January 31, 2020 21:18

Rework plugin system

27d786e

deivid-rodriguez force-pushed the rework_plugins_and_speed_up_rubygems branch from b3edaa5 to cb88467 Compare January 31, 2020 20:18

deivid-rodriguez removed the wip label Jan 31, 2020

deivid-rodriguez commented Jan 31, 2020

View reviewed changes

indirect added category: #gem or #require rubygems: performance RubyGems status: ready and removed status: triage labels Jan 31, 2020

Small DRY up

35aa25c

hsbt self-requested a review January 31, 2020 22:26

hsbt approved these changes Jan 31, 2020

View reviewed changes

ghost merged commit 35aa25c into master Feb 1, 2020

ghost deleted the rework_plugins_and_speed_up_rubygems branch February 1, 2020 01:05

deivid-rodriguez mentioned this pull request Feb 11, 2020

Fix rubyforge deprecated warnings #3047

Closed

4 tasks

duckinator mentioned this pull request Feb 11, 2020

All gem commands print warnings on master+3.1.0pre2, if certain gems are installed #2984

Closed

5 tasks

deivid-rodriguez mentioned this pull request Mar 26, 2020

Fix race condition on bundler's parallel installer #3440

Merged

4 tasks

deivid-rodriguez mentioned this pull request Apr 7, 2020

Explicitly deprecate rubyforge_project #2798

Merged

4 tasks

deivid-rodriguez mentioned this pull request Sep 14, 2020

bundle install does not run the gems hooks #3239

Open

rreinhardt9 mentioned this pull request Oct 3, 2020

Update overview of how plugins work to reflect improvement in plugin system rubygems/guides#268

Closed

Irakaj93 mentioned this pull request Dec 26, 2020

Error occurs everytime i try to create a new rails app #4197

Closed

wintersolutions mentioned this pull request Dec 27, 2020

Rubygems v3.2.3 bug in command 'gem wrappers regenerate' #4204

Closed

5 tasks

jasonadamyoung mentioned this pull request Dec 28, 2020

Wrappers plugin erroring with Rubygems >= 3.2.x rvm/gem-wrappers#14

Closed

pedrofurtado mentioned this pull request Dec 28, 2020

Error when generating default wrappers in 'rvm install 3.0.0' rvm/rvm#5020

Closed

wintersolutions mentioned this pull request Jan 6, 2021

Fix unknown command wrappers rvm/rvm#5027

Merged

voxik mentioned this pull request Aug 5, 2021

User installed plugins are not loaded anymore #4823

Closed

deivid-rodriguez mentioned this pull request Dec 30, 2022

Change stub line separator to : #6209

Open

4 tasks

voxik mentioned this pull request Mar 9, 2023

Fix plugins uninstallation for user installed gems #6456

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework plugins system and speed up rubygems #3108

Rework plugins system and speed up rubygems #3108

deivid-rodriguez commented Jan 27, 2020 •

edited

Loading

deivid-rodriguez commented Jan 30, 2020

deivid-rodriguez commented Jan 30, 2020

indirect left a comment

deivid-rodriguez commented Jan 31, 2020 •

edited

Loading

deivid-rodriguez Jan 31, 2020 •

edited

Loading

dgholz Feb 12, 2020

deivid-rodriguez Feb 12, 2020

dgholz Feb 17, 2020 •

edited

Loading

deivid-rodriguez Feb 17, 2020

dgholz Feb 18, 2020

deivid-rodriguez Feb 19, 2020

dgholz Feb 20, 2020

deivid-rodriguez Feb 21, 2020

deivid-rodriguez Feb 21, 2020

hsbt left a comment

deivid-rodriguez commented Feb 1, 2020

ghost commented Feb 1, 2020

Rework plugins system and speed up rubygems #3108

Rework plugins system and speed up rubygems #3108

Conversation

deivid-rodriguez commented Jan 27, 2020 • edited Loading

Description:

Before

After

The current (slow) rubygems plugin system

The new (faster) rubygems plugin system

Tasks:

deivid-rodriguez commented Jan 30, 2020

deivid-rodriguez commented Jan 30, 2020

indirect left a comment

Choose a reason for hiding this comment

deivid-rodriguez commented Jan 31, 2020 • edited Loading

deivid-rodriguez Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgholz Feb 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hsbt left a comment

Choose a reason for hiding this comment

deivid-rodriguez commented Feb 1, 2020

ghost commented Feb 1, 2020

Build succeeded

deivid-rodriguez commented Jan 27, 2020 •

edited

Loading

deivid-rodriguez commented Jan 31, 2020 •

edited

Loading

deivid-rodriguez Jan 31, 2020 •

edited

Loading

dgholz Feb 17, 2020 •

edited

Loading