-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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
bpo-28643: Record profile-opt build progress with stamp files #4223
Conversation
The profile-opt makefile target is expensive to build. Since the makefile does not contain complete dependency information for this target, much extra work can get done if the build is interrupted and re-started. Even running "make" a second time will result in a huge amount of redundant work. As a minimal fix (rather than removing recursive "make" and adding a proper dependency graph), split the profile-opt target into three parts: - build with profile generation enabled (profile-gen-stamp) - run task to generate profile information (profile-run-stamp) - build optimized Python using above information (profile-opt) Use the "stamp" files profile-gen-stamp and profile-run-stamp to record completion of the first two steps. The first will be cleaned by the "clobber" target, the second by "profile-removal". Other minor changes: - remove the "build_all_use_profile" target. I don't expect callers of the makefile to use this target so that should be safe. - remove "$(MAKE) clean" at the start of the profile-gen-stamp target. I assume that a profiled build normally starts from a clean source or build tree and so cleaning should not be required. Having the clean there causes an interruption of the profile-gen result in everthing getting built from scratch. - remove execution of "profile-removal" at end of "profile-opt". I don't see any reason to not to keep the profile information, given the cost to generate it. Removing the stamp files will force a re-build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, thank you for doing this. It should make repeated profile-opt builds easier to deal with.
there is a chance that someone could build with an outdated profile if they haven't run make clobber or make distclean - but that is rarely an issue. Important profile-opt builds likely to be included in a release anywhere should always be done with either a fresh build tree or at least after a make distclean.
Makefile.pre.in
Outdated
$(MAKE) profile-removal | ||
# This is an expensive target to build and it does not have proper | ||
# makefile dependancy information. So, we create a "stamp" file | ||
# to record that it has been preformed and does not have to get |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd edit this to read
# to record its completion to avoid rerunning it unnecessarily.
Makefile.pre.in
Outdated
@@ -443,6 +443,9 @@ all: @DEF_MAKE_ALL_RULE@ | |||
build_all: $(BUILDPYTHON) oldsharedmods sharedmods gdbhooks Programs/_testembed python-config | |||
|
|||
# Compile with profile generation | |||
# | |||
# Run "make clean" and remove stamp file to force re-build of the profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should read "make clobber"
Add a profile-clean-stamp target. We must make sure that Python is built from a clean tree with -fprofile-generate before running the profile task.
…#4223) * bpo-28643: Record profile-opt build progress with stamp files The profile-opt makefile target is expensive to build. Since the makefile does not contain complete dependency information for this target, much extra work can get done if the build is interrupted and re-started. Even running "make" a second time will result in a huge amount of redundant work. As a minimal fix (rather than removing recursive "make" and adding a proper dependency graph), split the profile-opt target into parts: - ensure tree is clean (profile-clean-stamp) - build with profile generation enabled (profile-gen-stamp) - run task to generate profile information (profile-run-stamp) - build optimized Python using above information (profile-opt) We use "stamp" files to record completion of the steps. Running "make clean" will not remove the profile-run-stamp file. Other minor changes: - remove the "build_all_use_profile" target. I don't expect callers of the makefile to use this target so that should be safe. - remove execution of "profile-removal" at end of "profile-opt". I don't see any reason to not to keep the profile information, given the cost to generate it. Removing the "profile-run-stamp" file will force re-generation of it.
The profile-opt makefile target is expensive to build. Since the
makefile does not contain complete dependency information for this
target, much extra work can get done if the build is interrupted and
re-started. Even running "make" a second time will result in a huge
amount of redundant work.
As a minimal fix (rather than removing recursive "make" and adding a
proper dependency graph), split the profile-opt target into three parts:
Use the "stamp" files profile-gen-stamp and profile-run-stamp to
record completion of the first two steps. The first will be cleaned by
the "clobber" target, the second by "profile-removal".
Other minor changes:
remove the "build_all_use_profile" target. I don't expect callers
of the makefile to use this target so that should be safe.
remove "$(MAKE) clean" at the start of the profile-gen-stamp target.
I assume that a profiled build normally starts from a clean source
or build tree and so cleaning should not be required. Having the
clean there causes an interruption of the profile-gen result in
everthing getting built from scratch.
remove execution of "profile-removal" at end of "profile-opt". I
don't see any reason to not to keep the profile information, given
the cost to generate it. Removing the stamp files will force a
re-build.
https://bugs.python.org/issue28643