Skip to content
This repository has been archived by the owner on Nov 27, 2020. It is now read-only.

Speed up travis runtime. #499

Merged
merged 1 commit into from
Jan 6, 2015
Merged

Conversation

botandrose
Copy link
Contributor

Vast majority of time is in bundle install, and the vast majority of
that is installing nokogiri. Parallelize bundle install, and use system
libraries for nokogiri installation.

@botandrose
Copy link
Contributor Author

Strange, how do I get Travis to test this branch? I thought it tested all PRs automatically.

@@ -5,3 +5,9 @@ rvm:
- 1.9.3
- rbx-2
- jruby-19mode

# speed up bundle install, especially nokogiri
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment feels overly specific and probably doesn't need to be here.

@botandrose
Copy link
Contributor Author

I've confirmed that setting BUNDLE_JOBS will give us the parallelization without requiring an override of the install command. Force pushed a new commit that addressed your two issues, and Travis appears to be testing this one! We should see a nice speedup.

@botandrose
Copy link
Contributor Author

Nice speedup, indeed.

master:
Job Duration    Finished    Ruby
 530.1  4 min 16 sec    2 days ago  2.0.0
 530.2  4 min 15 sec    2 days ago  1.9.3
 530.3  6 min 14 sec    2 days ago  rbx-2
 530.4  3 min 26 sec    2 days ago  jruby-19mode
speed_up_travis:
Job Duration    Finished    Ruby
 535.1  2 min 32 sec    8 minutes ago   2.0.0
 535.2  3 min 7 sec 7 minutes ago   1.9.3
 535.3  6 min 16 sec    3 minutes ago   rbx-2
 535.4  4 min 11 sec    3 minutes ago   jruby-19mode

@botandrose
Copy link
Contributor Author

MRIs are a clear win, but rbx seems unaffected, and jruby is slower! I investigated, and those numbers are red herrings. On this branch, rbx-2 failed to fetch a gem over the network on first try, so it needed to do a full rebundle. jruby-19mode failed two flaky specs on first try, so it did a retry of those specs. Neither of these things happened when travis last ran on master. This suggests its faster across the board, flaky specs and network notwithstanding.

@aprescott
Copy link
Contributor

I think it's very unlikely that we'll not see a speedup from parallelising bundle installs (all else equal), so this seems good to me regardless! I actually thought that bundler did this by default using a reasonable job value for the system, but it seems not?

@botandrose
Copy link
Contributor Author

Yeah, I would expect the jobs setting to be inferred, or to have a sensible default, but it is not. One must manually set jobs on each machine. I expect that future versions of bundler will have jobs set to something > 1 by default, but it is currently not the case.

@route
Copy link
Contributor

route commented Sep 5, 2014

What's the state of this?

@botandrose
Copy link
Contributor Author

As far as I know, this is good to merge. The CI was green, but I just force-pushed a rebase to make sure we're up to date. However, looks like master was already failing. Doh.

@@ -5,3 +5,7 @@ rvm:
- 1.9.3
- rbx-2
- jruby-19mode
env:
global:
- BUNDLE_JOBS=4
Copy link

Choose a reason for hiding this comment

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

I'm not sure about this. The latest TravisCI log shows:
$ bundle install --jobs=3 --retry=3

Should it be using 4 jobs, as suggested here, or is the output faulty?

@botandrose
Copy link
Contributor Author

Interesting. I bet Travis has updated their default bundle command to take advantage of concurrency since this PR was submitted. I'll push another commit to remove BUNDLE_JOBS, since Travis appears to curating this setting now.

Otherwise, NOKOGIRI_USE_SYSTEM_LIBRARIES=true appears to be working:

Building nokogiri using system libraries.

Vast majority of time is in bundle install, and the vast majority of
that is installing nokogiri. Parallelize bundle install, and use system
libraries for nokogiri installation.
@route
Copy link
Contributor

route commented Sep 5, 2014

@botandrose Yeah there are three tests in master that fail for now, because they aren't filtered out by capybara_skip. Let's wait till new window system will be merged in master and then merge your PR.

route added a commit that referenced this pull request Jan 6, 2015
@route route merged commit 067011e into teampoltergeist:master Jan 6, 2015
@route
Copy link
Contributor

route commented Jan 6, 2015

Thank you!

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

5 participants