Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Parallel installation #2481

Merged
merged 13 commits into from Jul 3, 2013
Merged

Parallel installation #2481

merged 13 commits into from Jul 3, 2013

Conversation

eagletmt
Copy link
Contributor

I parallelized gem installation. It mainly improves initial bundle install speed.

I added a new option -j which specifies the number of jobs to run in parallel.
The parallelization mechanism is normally multi-process using fork, but it switches to multi-thread on Windows because it doesn't have fork. Multi-thread version is slower than multi-process version due to locking about Dir.chdir.

At one project, which have over 200 gems to be installed, initial bundle -j4 is faster than bundle by about four minutes.

bundle --path sequential  183.03s user 45.13s system 38% cpu 9:55.48 total
bundle --path parallel -j4  234.85s user 50.14s system 77% cpu 6:05.52 total

module Bundler
WINDOWS = RbConfig::CONFIG["host_os"] =~ %r!(msdos|mswin|djgpp|mingw)!
FREEBSD = RbConfig::CONFIG["host_os"] =~ /bsd/
NULL = WINDOWS ? "NUL" : "/dev/null"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File::NULL ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File::NULL isn't available on Ruby 1.8.x.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Bundler::NULL, which already does the platform checking. This patch looks great, and I hope to turn on parallel installation by default in a future release.

On Wed, May 22, 2013 at 11:24 PM, Kenta Murata notifications@github.com
wrote:

@@ -0,0 +1,5 @@
+module Bundler

  • WINDOWS = RbConfig::CONFIG["host_os"] =~ %r!(msdos|mswin|djgpp|mingw)!
  • FREEBSD = RbConfig::CONFIG["host_os"] =~ /bsd/
  • NULL = WINDOWS ? "NUL" : "/dev/null"
    File::NULL isn't available on Ruby 1.8.x.

Reply to this email directly or view it on GitHub:
https://github.com/bundler/bundler/pull/2481/files#r4354668

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using File:NULL if available is more portable than the current Bundler::NULL:

NULL = defined?(File::NULL) ? File::NULL : WINDOWS ? "NUL" : "/dev/null"

@eagletmt
Copy link
Contributor Author

eagletmt commented Jun 5, 2013

I made an improvement to the behavior when some gem's installation fails.
In that case, all worker processes are immediately killed by SIGINT.

@indirect
Copy link
Member

indirect commented Jun 5, 2013

Excellent. Are partially installed gems cleaned up? Since the Bundler default is to install into GEM_HOME, I would like to avoid leaving partially installed gems for Rubygems to find later.

On Wed, Jun 5, 2013 at 12:44 AM, eagletmt notifications@github.com
wrote:

I made an improvement to the behavior when some gem's installation fails.

In that case, all worker processes are immediately killed by SIGINT.

Reply to this email directly or view it on GitHub:
#2481 (comment)

@eagletmt
Copy link
Contributor Author

eagletmt commented Jun 6, 2013

The gems/$GEM_NAME-$GEM_VERSION directory isn't cleaned up. This is the same as when the user stops installation by Ctrl-C or the gem installation fails due to build failure of C extensions.
The gemspec file is installed at the last of installation process. gems/$GEM_NAME-$GEM_VERSION directory is just ignored if the corresponding gemspec file inside specification directory is missing, so leaving partially installed gems doesn't matter, is it?

@indirect
Copy link
Member

indirect commented Jun 6, 2013

Sounds like that should be fine. :)

On Wed, Jun 5, 2013 at 6:54 PM, eagletmt notifications@github.com wrote:

The gems/$GEM_NAME-$GEM_VERSION directory isn't cleaned up. This is the same as when the user stops installation by Ctrl-C or the gem installation fails due to build failure of C extensions.

The gemspec file is installed at the last of installation process. gems/$GEM_NAME-$GEM_VERSION directory is just ignored if the corresponding gemspec file inside specification directory is missing, so leaving partially installed gems doesn't matter, is it?

Reply to this email directly or view it on GitHub:
#2481 (comment)

@schneems
Copy link
Contributor

Last comment 14 days ago. @eagletmt have you run into any blockers? Where are we with this PR?

@indirect
Copy link
Member

@schneems I haven't had time to review and test this yet :( But it passes on travis, which is an extremely promising sign.

You want to try this out while building a slug for a big app? That would be a super useful data point.

@eagletmt
Copy link
Contributor Author

My PR has no effect unless -j option is given. So passing on travis ensures only that fact.
I probably should add tests for parallel installatin, but I have no idea what to test and how to test...

indirect added a commit that referenced this pull request Jul 3, 2013
@indirect indirect merged commit 3fb9d05 into rubygems:master Jul 3, 2013
@schneems
Copy link
Contributor

schneems commented Jul 4, 2013

🤘

@davetoxa
Copy link

it's awesome!!! 👍 👍 👍

@padi
Copy link

padi commented Sep 2, 2013

👍 Sweet!

@jlecour
Copy link

jlecour commented Sep 4, 2013

Hi. This is wonderful.

Is it possible to make this the default via a config file and not have to type the flag each time?

Thanks

@indirect
Copy link
Member

indirect commented Sep 4, 2013

In the master branch, this option is remembered, and can also be set via bundle config jobs N or BUNDLE_JOBS=N.

On Sep 4, 2013, at 2:17 AM, Jérémy Lecour notifications@github.com wrote:

Hi. This is wonderful.

Is it possible to make this the default via a config file and not have to type the flag each time?

Thanks


Reply to this email directly or view it on GitHub.

@gurix
Copy link

gurix commented Sep 6, 2013

👍 nice idea

@r3nya
Copy link

r3nya commented Sep 29, 2013

Awesome! 👍

@brndnblck
Copy link
Contributor

This is awesome. 👍

@meatherly
Copy link

Could we also add this to bundle update as well?

@indirect
Copy link
Member

Sounds good, anyone want to send a pull request adding this to update?

@mrkn
Copy link
Contributor

mrkn commented Oct 30, 2013

It can be realized to adding the option entry of --job for the update command.
I've done it in #2692. Please check it.

@masterkain
Copy link

Watching this

@indirect
Copy link
Member

indirect commented Nov 1, 2013

You don’t need to comment to watch things. Please don’t.

On Oct 31, 2013, at 5:14 PM, Claudio Poli notifications@github.com wrote:

Watching this


Reply to this email directly or view it on GitHub.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet