Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow a "-j" or "--jobs" argument to "docker-php-ext-install" to speed up extension building #159

Merged
merged 2 commits into from
Dec 9, 2015

Conversation

tianon
Copy link
Member

@tianon tianon commented Dec 9, 2015

Fixes #144

@tianon
Copy link
Member Author

tianon commented Dec 9, 2015

Thanks for the prod, @soullivaneuh 👍

@tianon
Copy link
Member Author

tianon commented Dec 9, 2015

For testing this, I did time docker-php-ext-install mysqli pdo pdo_mysql vs time docker-php-ext-install -j8 mysqli pdo pdo_mysql, and got 13s vs 6s. 😄

@yosifkit
Copy link
Member

yosifkit commented Dec 9, 2015

LGTM

yosifkit added a commit that referenced this pull request Dec 9, 2015
Allow a "-j" or "--jobs" argument to "docker-php-ext-install" to speed up extension building
@yosifkit yosifkit merged commit 8361e62 into docker-library:master Dec 9, 2015
@yosifkit yosifkit deleted the jobs branch December 9, 2015 20:37
@soullivaneuh
Copy link

Thank you! But I don't think this is useful for install and clean. This take effect only for compilation AFAIK.

When this patch will be available from DockerHub? I'm pretty new with this system, I don't know how it works. :-)

@tianon
Copy link
Member Author

tianon commented Dec 9, 2015 via email

@soullivaneuh
Copy link

this won't hit the Hub until a PR is made over in
https://github.com/docker-library/official-images

So you're telling that I should submit the same ph on docker-library/official-images? And then, this will be automatically available?

Thanks.

yosifkit added a commit to infosiftr/stackbrew that referenced this pull request Dec 10, 2015
- `mariadb` bump to `5.5.46`
- `php` add `--jobs` to dokcer-php-ext-install docker-library/php#159
- `tomcat` bump `7.0.67`
yosifkit added a commit to infosiftr/stackbrew that referenced this pull request Dec 10, 2015
- `mariadb` bump to `5.5.46`
- `php` add `--jobs` to docker-php-ext-install docker-library/php#159
- `tomcat` bump `7.0.67`
yosifkit added a commit to infosiftr/stackbrew that referenced this pull request Dec 10, 2015
- `mariadb` bump to `5.5.46`
- `php` add `--jobs` to docker-php-ext-install, see docker-library/php#159
- `tomcat` bump `7.0.67`
@yosifkit
Copy link
Member

Thanks for being willing but you don't have to make the PR. I got this docker-library/official-images#1264. 😄

@soullivaneuh
Copy link

Great, thanks! 👍

pierreozoux pushed a commit to pierreozoux/official-images that referenced this pull request Jan 7, 2016
- `mariadb` bump to `5.5.46`
- `php` add `--jobs` to docker-php-ext-install, see docker-library/php#159
- `tomcat` bump `7.0.67`
RichardScothern pushed a commit to RichardScothern/official-images that referenced this pull request Jun 14, 2016
- `mariadb` bump to `5.5.46`
- `php` add `--jobs` to docker-php-ext-install, see docker-library/php#159
- `tomcat` bump `7.0.67`
opts="$(getopt -o 'h?j:' --long 'help,jobs:' -- "$@" || { usage >&2 && false; })"
eval set -- "$opts"

j=1
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should default to $(nproc).

Copy link
Member

Choose a reason for hiding this comment

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

docker-library/drupal#154 (comment):

It defaults to make's default (we don't explicitly -j1 anywhere), mostly to be considerate of php users' machines -- for the official images though, we want to make full use of our build systems as much as possible (especially chasing the dragon of potentially faster builds), which is why we typically use -j "$(nproc)" in ours.

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

Successfully merging this pull request may close these issues.

4 participants