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

Fix #160 Enable optimizations #357

Closed

Conversation

blopker
Copy link
Contributor

@blopker blopker commented Dec 1, 2018

@blopker blopker changed the title Feature/enable optimizations Fix #160 Eenable optimizations Dec 1, 2018
@blopker blopker changed the title Fix #160 Eenable optimizations Fix #160 Enable optimizations Dec 1, 2018
@blopker
Copy link
Contributor Author

blopker commented Dec 2, 2018

The commits still need to be squashed, but the PR is ready for review now.

@JayH5
Copy link
Contributor

JayH5 commented Dec 3, 2018

@blopker thanks for giving this a try. Those build times are really long, though 😞.

Maybe you could make a separate PR just to do the FFI/Expat stuff for Python 2.7? It seems like a good idea to fix that and I don't think we should let the --enable-optimizations discussion get in the way of that.

@blopker blopker mentioned this pull request Dec 4, 2018
@blopker
Copy link
Contributor Author

blopker commented Dec 4, 2018

Here you go: #358

The build times are longer, but how often are builds run? It looks like around monthly. As an added benefit, with the optimization flag we found an issue with the current images. If there's another issue at some point, running the tests may catch it.

@blopker
Copy link
Contributor Author

blopker commented Dec 4, 2018

I looked around a little and it turns out Travis CI has granted some open source projects more concurrent builders. WordPress has 15: https://make.wordpress.org/core/2017/05/18/increased-concurrent-jobs-on-travis-ci-builds/

Here travis-ci/travis-ci#4668 they mention sending an email to support@travis-ci.com to ask about raising the limit. Might be worth a try.

@tianon
Copy link
Member

tianon commented Dec 4, 2018 via email

@blopker
Copy link
Contributor Author

blopker commented Dec 5, 2018

Oh, that's too bad. I didn't know there was another build system. I usually just use Travis to push images once built: https://docs.travis-ci.com/user/docker/

May be worth asking support for more workers anyway.

I do want to note that I just rolled out a Python 2.7 with optimizations to a production system at work (not Docker though) and we saw a visible minimum 10% performance gain across all endpoints. Endpoints with heavy database access saw the least gain, of course.

If this change doesn't go through I'll definitely set up optimized images for internal use, but I'd love to share it with the world :)

@gaby
Copy link

gaby commented Feb 4, 2019

Any updates regarding this PR? It would be great addition to have optimized Images!

@yosifkit
Copy link
Member

Closing since we cannot handle the ~16x on build times.

Since these optimizations are only based on running the test suite it would make sense that they could be all pre-created and released with the source, rather than having distributions recreate what is, in essence, a static optimization.

@yosifkit yosifkit closed this Mar 22, 2019
@gpshead
Copy link

gpshead commented Jul 8, 2019

See the comment on the issue, you don't need to use the default overly-long PROFILE_TASK setting to see a large benefit.

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.

Consider adding '--enable-optimizations' configure argument
6 participants