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.

Sign up for GitHub

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

Jump to bottom

--enable-optimizations makes common build commands always need to compile from scratch #73429

Open
zhangyangyu opened this issue Jan 11, 2017 · 18 comments
Open

--enable-optimizations makes common build commands always need to compile from scratch #73429

zhangyangyu opened this issue Jan 11, 2017 · 18 comments
Labels
3.7 (EOL) end of life build The build process and cross-build type-feature A feature request or enhancement

Comments

@zhangyangyu
Copy link
Member

BPO 29243
Nosy @brettcannon, @gpshead, @vstinner, @asvetlov, @methane, @zhangyangyu, @stratakis, @torsava
PRs
  • bpo-29243: Fix Makefile with respect to --enable-optimizations #1478
  • [3.6] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) #1518
  • [3.5] bpo-29243: Fix Makefile with respect to --enable-optimizations … #1520
  • [2.7] bpo-29243: Fix Makefile with respect to --enable-optimizations … #1522
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = None
created_at = <Date 2017-01-11.16:15:14.785>
labels = ['type-feature', '3.7', 'build']
title = '--enable-optimizations makes common build commands always need to compile from scratch'
updated_at = <Date 2017-12-20.20:50:35.331>
user = 'https://github.com/zhangyangyu'

    bugs.python.org fields:

    activity = <Date 2017-12-20.20:50:35.331>
actor = 'asvetlov'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Build']
creation = <Date 2017-01-11.16:15:14.785>
creator = 'xiang.zhang'
dependencies = []
files = []
hgrepos = []
issue_num = 29243
keywords = []
message_count = 18.0
messages = ['285233', '285259', '286226', '290076', '293100', '293112', '293113', '293315', '293319', '293334', '293377', '293385', '293387', '293388', '293397', '293414', '293437', '308811']
nosy_count = 8.0
nosy_names = ['brett.cannon', 'gregory.p.smith', 'vstinner', 'asvetlov', 'methane', 'xiang.zhang', 'cstratak', 'torsava']
pr_nums = ['1478', '1518', '1520', '1522']
priority = 'normal'
resolution = None
stage = 'commit review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue29243'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']
    @zhangyangyu
    Copy link
    Member Author

    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.

    @zhangyangyu zhangyangyu added 3.7 (EOL) end of life build The build process and cross-build labels Jan 11, 2017
    @brettcannon
    Copy link
    Member

    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.

    @brettcannon brettcannon added the type-feature A feature request or enhancement label Jan 11, 2017
    @methane
    Copy link
    Member

    methane commented Jan 25, 2017

    That's why I want to enable only --with-lto

    @zhangyangyu
    Copy link
    Member Author

    Another complaint from bpo-29889.

    @torsava
    Copy link
    Mannequin

    torsava mannequin commented May 5, 2017

    I've opened a PR#1478 that I believe fixes the issue.

    @vstinner
    Copy link
    Member

    vstinner commented May 5, 2017

    New changeset a1054c3 by Victor Stinner (torsava) in branch 'master':
    bpo-29243: Fix Makefile with respect to --enable-optimizations (bpo-1478)
    a1054c3

    @vstinner
    Copy link
    Member

    vstinner commented May 5, 2017

    I closed my issue bpo-29641 as a duplicate of this one.

    @vstinner
    Copy link
    Member

    vstinner commented May 9, 2017

    New changeset 03b8a37 by Victor Stinner (torsava) in branch '3.6':
    [3.6] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (bpo-1518)
    03b8a37

    @vstinner
    Copy link
    Member

    vstinner commented May 9, 2017

    New changeset 8489409 by Victor Stinner (torsava) in branch '3.5':
    [3.5] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (bpo-1520)
    8489409

    @vstinner
    Copy link
    Member

    vstinner commented May 9, 2017

    New changeset a473a73 by Victor Stinner (torsava) in branch '2.7':
    [2.7] bpo-29243: Fix Makefile with respect to --enable-optimizations (GH-1478) (bpo-1522)
    a473a73

    @zhangyangyu
    Copy link
    Member Author

    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?

    @vstinner
    Copy link
    Member

    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!

    @zhangyangyu
    Copy link
    Member Author

    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.

    @gpshead
    Copy link
    Member

    gpshead commented May 10, 2017

    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.

    @vstinner
    Copy link
    Member

    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".

    @torsava
    Copy link
    Mannequin

    torsava mannequin commented May 10, 2017

    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.

    @brettcannon
    Copy link
    Member

    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).

    @asvetlov
    Copy link
    Contributor

    Is the issue done? Can it be closed?

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life build The build process and cross-build type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    6 participants
    @brettcannon @gpshead @vstinner @methane @asvetlov @zhangyangyu