Avoid clone same repo for Git gems #109
Conversation
When a Gemfile installs rails master, it clones rails for 12 times. By storing Rails‘s subgems in the pending queue under the same name "rails". We can avoid clone rails gems multiple times.
Wow, that sounds pretty slow still! Curious if there's any other leads as to why this grinds? |
|
This is a nice find! I had at one point suspected this was happening, but then failed to prove it. As @kaspth mentioned, it feels a bit awkward to maintain a hard-coded list of multi-gem repos. I think we could instead use a hash (as an instance variable inside Installer), to keep track of whether we've already seen |
Use a hash to keep track of whether we've already checked out a repo by remote+revision.
|
Thanks @kaspth and @matthewd for your reviews!! I updated the PR to check if we already clone a given remote + revision as suggested
Sorry this was my mistake. The 354s was from I also tried with fresh bundle install and it took about 5 minutes. |
.. and then show them all in the "Using" message, too.
|
@JuanitoFatas I made a small tweak, building off the implementation you had in there ☝🏻 -- does it seem reasonable to you? As for testing... I haven't tried, but I think you might be able to construct an Installer, then use |
|
Wanted to see the runtime for myself so tried testing the Gemfile locally with @matthewd's commit on top and I get a RuntimeError. I'm assuming there's a Gem I don't have access to, so I'll leave it for here. 3 minutes still sound like a long time, especially with parallelization Gel does. But maybe not worth looking into on top of this. See the RuntimeErrorkaspth-imac 2.7.0 ~/code/geltest rm -rf ~/.local/gel ~/.cache/gel && time ../gel/exe/gel install
Fetching sources....Unhandled exception in work pool "gel-catalog-prep" for job "catalog":
#<RuntimeError: git rev-parse failed>
/Users/kaspth/code/gel/lib/gel/git_depot.rb:47:in `resolve'
/Users/kaspth/code/gel/lib/gel/git_depot.rb:55:in `resolve_and_checkout'
/Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `block in checkout_result'
/Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `synchronize'
/Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `checkout_result'
/Users/kaspth/code/gel/lib/gel/git_catalog.rb:36:in `prepare'
/Users/kaspth/code/gel/lib/gel/environment.rb:205:in `block (3 levels) in solve_for_gemfile'
/Users/kaspth/code/gel/lib/gel/work_pool.rb:62:in `block (4 levels) in start'
/Users/kaspth/code/gel/lib/gel/work_pool.rb:47:in `loop'
/Users/kaspth/code/gel/lib/gel/work_pool.rb:47:in `block (3 levels) in start'
/Users/kaspth/code/gel/lib/gel/work_pool.rb:46:in `catch'
/Users/kaspth/code/gel/lib/gel/work_pool.rb:46:in `block (2 levels) in start'
......
Resolving dependencies...
===== Gel Internal Error =====
Traceback (most recent call last):
26: from ../gel/exe/gel:13:in `<main>'
25: from /Users/kaspth/code/gel/lib/gel/command.rb:13:in `run'
24: from /Users/kaspth/code/gel/lib/gel/command/install.rb:5:in `run'
23: from /Users/kaspth/code/gel/lib/gel/environment.rb:358:in `activate'
22: from /Users/kaspth/code/gel/lib/gel/environment.rb:326:in `write_lock'
21: from /Users/kaspth/code/gel/lib/gel/environment.rb:232:in `solve_for_gemfile'
20: from /Users/kaspth/.local/gel/ruby/gems/pub_grub-0.5.0/lib/pub_grub/version_solver.rb:40:in `work'
19: from /Users/kaspth/.local/gel/ruby/gems/pub_grub-0.5.0/lib/pub_grub/version_solver.rb:122:in `choose_package_version'
18: from /Users/kaspth/code/gel/lib/gel/pub_grub/solver.rb:19:in `next_package_to_try'
17: from /Users/kaspth/code/gel/lib/gel/pub_grub/solver.rb:19:in `min_by'
16: from /Users/kaspth/code/gel/lib/gel/pub_grub/solver.rb:19:in `each'
15: from /Users/kaspth/code/gel/lib/gel/pub_grub/solver.rb:21:in `block in next_package_to_try'
14: from /Users/kaspth/.local/gel/ruby/gems/pub_grub-0.5.0/lib/pub_grub/basic_package_source.rb:120:in `versions_for'
13: from /Users/kaspth/.local/gel/ruby/gems/pub_grub-0.5.0/lib/pub_grub/basic_package_source.rb:103:in `block in initialize'
12: from /Users/kaspth/.local/gel/ruby/gems/pub_grub-0.5.0/lib/pub_grub/basic_package_source.rb:100:in `block in initialize'
11: from /Users/kaspth/code/gel/lib/gel/pub_grub/source.rb:44:in `all_versions_for'
10: from /Users/kaspth/code/gel/lib/gel/catalog_set.rb:24:in `entries_for'
9: from /Users/kaspth/code/gel/lib/gel/catalog_set.rb:72:in `fetch_package_info'
8: from /Users/kaspth/code/gel/lib/gel/catalog_set.rb:72:in `each'
7: from /Users/kaspth/code/gel/lib/gel/catalog_set.rb:78:in `block in fetch_package_info'
6: from /Users/kaspth/code/gel/lib/gel/git_catalog.rb:28:in `gem_info'
5: from /Users/kaspth/code/gel/lib/gel/git_catalog.rb:32:in `path_catalog'
4: from /Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `checkout_result'
3: from /Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `synchronize'
2: from /Users/kaspth/code/gel/lib/gel/git_catalog.rb:20:in `block in checkout_result'
1: from /Users/kaspth/code/gel/lib/gel/git_depot.rb:55:in `resolve_and_checkout'
/Users/kaspth/code/gel/lib/gel/git_depot.rb:47:in `resolve': git rev-parse failed (RuntimeError)
real 1m14.453s
user 0m16.426s
sys 0m5.008s |
|
@kaspth yeah I hit that too -- it definitely reveals some poor error handling 😅, but is because bkeepers/dotenv#404 was merged. I didn't actually time things out, but in my casual observation, I think it was the git clone of Rails that accounted for most of the time. Possibly we should consider [the tradeoffs of] a shallower clone strategy. (But then again, a first-run git clone of rails is a rare thing to do.) |
|
Ah, cool! If I just use kaspth-imac 2.7.0 ~/code/gel/tmp less-clone-for-git-gems= rm -rf ~/.local/gel ~/.cache/gel && time ../exe/gel install
Fetching sources...........
Resolving dependencies..........
Writing lockfile to /Users/kaspth/code/gel/tmp/Gemfile.lock
Installing zeitwerk (2.3.0)
Installing mini_portile2 (2.4.0)
Installing i18n (1.8.2)
Installing minitest (5.14.0)
Installing tzinfo (2.0.1)
Installing rails-dom-testing (2.0.3)
Installing crass (1.0.6)
Installing rails-html-sanitizer (1.3.0)
Installing loofah (2.4.0)
Installing builder (3.2.4)
Installing rack (2.2.2)
Installing erubi (1.9.0)
Installing rack-test (1.1.0)
Installing concurrent-ruby (1.1.6)
Installing globalid (0.4.2)
Installing method_source (1.0.0)
Installing thor (1.0.1)
Installing marcel (0.3.3)
Installing rake (13.0.1)
Installing mini_mime (1.0.2)
Installing rspec-support (3.9.2)
Installing public_suffix (4.0.3)
Installing mimemagic (0.3.4)
Installing addressable (2.7.0)
Installing diff-lcs (1.3)
Installing tilt (2.0.10)
Installing websocket-extensions (0.1.4)
Installing multipart-post (2.1.1)
Installing ragel-bitmap (0.2.1)
Installing execjs (2.7.0)
Installing rubyzip (2.3.0)
Installing rb-inotify (0.10.1)
Installing safe_yaml (1.0.5)
Installing faraday (1.0.1)
Installing rb-fsevent (0.10.3)
Installing childprocess (3.0.0)
Installing temple (0.8.2)
Installing coderay (1.1.2)
Installing coffee-script-source (1.12.2)
Installing get_process_mem (0.2.5)
Installing coffee-script (2.4.1)
Installing statsd-ruby (1.4.0)
Installing websocket-driver (0.7.1)
Installing multi_json (1.14.1)
Installing spring (2.1.0)
Installing request_store (1.5.0)
Installing rspec-core (3.9.1)
Installing hashdiff (1.0.1)
Installing factory_bot (5.1.2)
Installing regexp_parser (1.7.0)
Installing selenium-webdriver (3.142.7)
Installing sawyer (0.8.2)
Installing slim (4.0.1)
Installing http-2-next (0.2.6)
Installing rails_stdout_logging (0.0.5)
Installing sassc-rails (2.1.2)
Installing ruby-enum (0.8.0)
Installing rspec-expectations (3.9.1)
Installing pry (0.13.0)
Installing listen (3.2.1)
Installing turbolinks-source (5.2.0)
Installing timers (4.3.0)
Installing rspec-mocks (3.9.1)
Installing crack (0.4.3)
Installing xpath (3.2.0)
Installing octokit (4.18.0)
Installing attr_extras (6.2.3)
Installing webmock (3.8.3)
Installing lograge (0.11.2)
Installing factory_bot_rails (5.1.1)
Installing pry-rails (0.3.9)
Installing sanitize (5.1.0)
Installing puma_worker_killer (0.1.1)
Installing sass-rails (6.0.0)
Installing rack-mini-profiler (2.0.1)
Installing rspec-rails (4.0.0)
Installing secure_headers (6.3.0)
Installing rouge (3.17.0)
Installing shoulda-matchers (4.3.0)
Installing rack-rewrite (1.5.1)
Installing slim-rails (3.2.0)
Installing coffee-rails (5.0.0)
Installing capybara (3.32.0)
Installing rack-timeout (0.6.0)
Installing html-pipeline (2.12.3)
Installing gemoji (3.0.1)
Installing httpx (0.7.0)
Installing turbolinks (5.2.1)
Installing spring-watcher-listen (2.0.1)
Installing barnes (0.0.8)
Installing twemoji (3.1.6)
Installing uglifier (4.2.0)
Installing webdrivers (4.2.0)
Installing memory_profiler (0.9.14)
Installing launchy (2.5.0)
Installing redis (4.1.3)
Installing rails_serve_static_assets (0.0.5)
Installing nio4r (2.5.2)
Installing rails_12factor (0.0.3)
Installing msgpack (1.3.3)
Installing puma (4.3.3)
Installing hiredis (0.6.3)
Installing json (2.3.0)
Installing bootsnap (1.4.6)
Installing commonmarker (0.21.0)
Installing pg (1.2.3)
Installing ffi (1.12.2)
Installing nokogiri (1.10.9)
Installing nokogumbo (2.0.2)
Installing sassc (2.2.1)
Installed 110 gems
real 1m27.697s
user 2m45.017s
sys 0m40.872s |
JuanitoFatas
force-pushed
the
less-clone-for-git-gems
branch
from
eccf3d8 to
7e26027
Compare
| ) | ||
| fake_download_pool.expect(:queue, true) do |argument| | ||
| argument == "actioncable" | ||
| end |
There was a problem hiding this comment.
if this expect got run twice, Minitest will throw an error
MockExpectationError: No more expects available for :queue: ["actionmailbox"]
simulates checkout twice in real scenario.
|
@kaspth Sorry about the disappear dotenv reference. I also tried f8b9e89 with my rails project it works and emits all gem names: Result running
|
Ah, so this is another thing. Interesting! The output is missing because of this condition: Line 101 in d5e4aa3 ... and that's false (i.e., the directory exists) because we've already cloned the repository earlier, in order to resolve the lockfile. So, I think we can drop that condition ( |
When there is no lockfile, we've already cloned the repository earlier to resolve the lockfile. So the check here will return false and some git gems message will not appear.
There was a problem hiding this comment.
Cool! Looks like I blew away the local setup I had for testing this, so I haven't tried again. But from your updated PR description, it seems like it worked. Nice job pulling for this @JuanitoFatas 🙏
When an application installs rails master, it clones rails for 12 times. By storing Rails‘s subgems under the same name "rails" in the pending queue. We can avoid clone rails gems multiple times.
Tested Gemfile
Results tested this PR against 00ca7aa
gel install without Gemfile.lock should print all rails gems
gel install with Gemfile.lock should print all rails gems