classification
Title: --enable-optimizations makes common build commands always need to compile from scratch
Type: enhancement Stage: commit review
Components: Build Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: asvetlov, brett.cannon, cstratak, gregory.p.smith, inada.naoki, torsava, vstinner, xiang.zhang
Priority: normal Keywords:

Created on 2017-01-11 16:15 by xiang.zhang, last changed 2017-12-20 20:50 by asvetlov.

Pull Requests
URL Status Linked Edit
PR 1478 merged torsava, 2017-05-05 09:44
PR 1518 merged torsava, 2017-05-09 13:46
PR 1520 merged torsava, 2017-05-09 14:56
PR 1522 merged torsava, 2017-05-09 15:14
Messages (18)
msg285233 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-01-11 16:15
In 3.6 we get --enable-optimizations. One thing I find annoyed with this flag is that it makes some build commands compile from scratch since it always clears the environment first. For example, with the commands listed at the top of Makefile:

./configure
make 
make test
make install

It compiles 3 times and considering the optimized build needs more time every time, it lasts long.

I am not sure this is a problem and feel free to close it.
msg285259 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-01-11 18:42
Since optimizations uses PGO which should do a fresh run, that's why there's the clearing of the state.

I'm personally not bothered by it as you really should only being doing an optimized build once when you do the first install and then you're done. But if or anyone else can come up with a way to not clear out the results while not accidentally using stale PGO-collected stats or builds then I don't see why we can't speed it up.
msg286226 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2017-01-25 04:26
That's why I want to enable only --with-lto
msg290076 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-03-24 03:53
Another complaint from #29889.
msg293100 - (view) Author: Tomas Orsava (torsava) * Date: 2017-05-05 11:09
I've opened a PR#1478 that I believe fixes the issue.
msg293112 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-05 15:35
New changeset a1054c3b0037d4c2a5492e79fc193f36245366c7 by Victor Stinner (torsava) in branch 'master':
bpo-29243: Fix Makefile with respect to --enable-optimizations (#1478)
https://github.com/python/cpython/commit/a1054c3b0037d4c2a5492e79fc193f36245366c7
msg293113 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-05 15:39
I closed my issue #29641 as a duplicate of this one.
msg293315 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-09 14:05
New changeset 03b8a378dfa46372b96790f82c85e9b72518f1bf by Victor Stinner (torsava) in branch '3.6':
[3.6] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (#1518)
https://github.com/python/cpython/commit/03b8a378dfa46372b96790f82c85e9b72518f1bf
msg293319 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-09 15:12
New changeset 8489409bbfabb2ddc30ed55c9f4d679a3710ebe4 by Victor Stinner (torsava) in branch '3.5':
[3.5] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (#1520)
https://github.com/python/cpython/commit/8489409bbfabb2ddc30ed55c9f4d679a3710ebe4
msg293334 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-09 16:04
New changeset a473a73d0cb42c534a3047bbf781b3c592fc33ca by Victor Stinner (torsava) in branch '2.7':
[2.7] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (#1522)
https://github.com/python/cpython/commit/a473a73d0cb42c534a3047bbf781b3c592fc33ca
msg293377 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-05-10 04:13
So now we have to run make if we want to trigger the profile procedure since other make commands like make test won't trigger the procedure by itself, right?
msg293385 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-10 06:52
Xiang Zhang added the comment:
> So now we have to run make if we want to trigger the profile procedure since other make commands like make test won't trigger the procedure by itself, right?

I don't think that the patch changes the behaviour of "make": it only
changes the behaviour of "make install" and "make test" (and a few
others).

To be honest, I don't know if Makefile knows when a rebuild is needed
and how it works. To run benchmarks, I always rebuild Python from
scratch, even from an empty directory!
msg293387 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-05-10 07:22
> I don't think that the patch changes the behaviour of "make": it only
> changes the behaviour of "make install" and "make test" (and a few
> others).

I don't mean "make" is changed. I mean the latter. Before, a single "make test" or "make install" will do all things for you, including the profile procedure. Now they seems not. You have to run "make" before them to trigger the profile procedure.

Actually I am quite okay with this change. But don't know others.
msg293388 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2017-05-10 08:06
Yes, this appears true.

I'm okay with it because I've always considered the make, test, and install as different command invocations.  The package building automation I am aware of tends to do the same.  Our buildbots keep the steps separate.

But we need document this requirement somewhere.

I think if we wanted test and install to accurately figure out if they need to redo the profile build and profiling run, we'd need make to detect that _any_ of its possible profile build+run input files are more recent than the last profiling run.  ugh.
msg293397 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-05-10 10:29
Oh wait, I expected that "make test" would build Python with PGO, but it doesn't anymore. Would it be possible to change the Makefile to make sure that "make python" (on Linux, "make python.exe" on macOS) would build Python using PGO?

I tried to hack something, but I'm lost in dependencies... "make profile-opt" runs "$(MAKE) build_all_generate_profile" which runs "$(MAKE) build_all CFLAGS_NODIST=..." and build_all depends on "$(BUILDPYTHON)".

I'm not sure that it's possible to make everything automatic because of the high number of targets and dependencies.

To be honest, I'm not excited by ./configure --enable-optimizations. I was happy with an explicit "make profile-opt".
msg293414 - (view) Author: Tomas Orsava (torsava) * Date: 2017-05-10 12:54
> Oh wait, I expected that "make test" would build Python with PGO, but it
> doesn't anymore. Would it be possible to change the Makefile to make sure
> that "make python" (on Linux, "make python.exe" on macOS) would build Python
> using PGO?
>
> I tried to hack something, but I'm lost in dependencies... "make profile-opt"
> runs "$(MAKE) build_all_generate_profile" which runs "$(MAKE) build_all
> CFLAGS_NODIST=..." and build_all depends on "$(BUILDPYTHON)".
>
> I'm not sure that it's possible to make everything automatic because of the
> high number of targets and dependencies.

In addition, profile-opt would have to rewritten to test whether there's
already an interpreter built with PGO. And if there is, regenerate it only if
the main target is `all` or `profile-opt`, but not when someone runs `make
test`, `make install`, etc.

It's maybe too radical, but we could change it so that if you run `make test`, `make install` and the like and there's no interpreter built already to test or install, the user is told to run `make` first.
msg293437 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2017-05-10 16:58
To be honest, I'm not excited by ./configure --enable-optimizations. I was happy with an explicit "make profile-opt".

The configure flag was added because not everything can be enabled just in the Makefile and so that people have a single flag to set to turn on any and all optimizations that work consistently across OSs (it used to be LTO+PGO until we discovered how universally broken LTO seems to be).
msg308811 - (view) Author: Andrew Svetlov (asvetlov) * (Python committer) Date: 2017-12-20 20:50
Is the issue done? Can it be closed?
History
Date User Action Args
2017-12-20 20:50:35asvetlovsetnosy: + asvetlov
messages: + msg308811
2017-05-10 16:58:29brett.cannonsetmessages: + msg293437
2017-05-10 12:54:26torsavasetmessages: + msg293414
2017-05-10 10:29:36vstinnersetmessages: + msg293397
2017-05-10 08:06:35gregory.p.smithsetstage: commit review
messages: + msg293388
versions: + Python 2.7, Python 3.5
2017-05-10 07:22:03xiang.zhangsetmessages: + msg293387
2017-05-10 06:52:00vstinnersetmessages: + msg293385
2017-05-10 04:13:42xiang.zhangsetmessages: + msg293377
2017-05-09 16:04:32vstinnersetmessages: + msg293334
2017-05-09 15:14:08torsavasetpull_requests: + pull_request1621
2017-05-09 15:12:40vstinnersetmessages: + msg293319
2017-05-09 14:56:08torsavasetpull_requests: + pull_request1620
2017-05-09 14:05:23vstinnersetmessages: + msg293315
2017-05-09 13:46:08torsavasetpull_requests: + pull_request1618
2017-05-05 15:39:52vstinnersetmessages: + msg293113
2017-05-05 15:35:52vstinnersetnosy: + vstinner
messages: + msg293112
2017-05-05 11:09:17torsavasetnosy: + torsava
messages: + msg293100
2017-05-05 09:44:52torsavasetpull_requests: + pull_request1577
2017-05-03 15:12:04cstrataksetnosy: + cstratak
2017-05-02 13:34:12vstinnerlinkissue29641 superseder
2017-03-24 03:53:40xiang.zhangsetmessages: + msg290076
2017-01-25 06:19:02gregory.p.smithsetnosy: + gregory.p.smith
2017-01-25 04:26:47inada.naokisetnosy: + inada.naoki
messages: + msg286226
2017-01-11 18:42:00brett.cannonsettype: enhancement

messages: + msg285259
nosy: + brett.cannon
2017-01-11 16:15:14xiang.zhangcreate