| Message ID | 20241105215644.1884070-2-ross.burton@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] python3-cython: remove obsolete SETUPTOOLS_INSTALL_ARGS | expand |
On Tue, Nov 5, 2024 at 01:56 PM, Ross Burton wrote:
I find that patch incredibly risky, as there is no really opt-out.
As the main beneficiary is pandas (which is not in core), which not add it there as a patch?
Also other suggest that all that can be achieved with --global-option="build_ext" --global-option="-j5"
set by PEP517_BUILD_OPTS, which would find the much better options here.
Something like
PARALLEL_PEP517_BUILD ?= " --global-option="build_ext" --global-option="-j<job count>""
PEP517_BUILD_OPTS:append = " ${PARALLEL_PEP517_BUILD}"
On 6 Nov 2024, at 18:30, Konrad Weihmann via lists.openembedded.org <kweihmann=outlook.com@lists.openembedded.org> wrote: > > On Tue, Nov 5, 2024 at 01:56 PM, Ross Burton wrote: > > I find that patch incredibly risky, as there is no really opt-out. > As the main beneficiary is pandas (which is not in core), which not add it there as a patch? You can opt out by setting PARALLEL_MAKE to -j1. Pandas is where I noticed, but I expect it will make a difference for any package that builds extensions with setuptools. > Also other suggest that all that can be achieved with --global-option="build_ext" --global-option="-j5" > set by PEP517_BUILD_OPTS, which would find the much better options here. > > Something like > > PARALLEL_PEP517_BUILD ?= " --global-option="build_ext" --global-option="-j<job count>"" > PEP517_BUILD_OPTS:append = " ${PARALLEL_PEP517_BUILD}” That’s possibly a neater solution, yes. I tried using global-option but couldn’t get it to work right. Ross
On 07.11.24 22:40, Ross Burton wrote: > On 6 Nov 2024, at 18:30, Konrad Weihmann via lists.openembedded.org <kweihmann=outlook.com@lists.openembedded.org> wrote: >> >> On Tue, Nov 5, 2024 at 01:56 PM, Ross Burton wrote: >> >> I find that patch incredibly risky, as there is no really opt-out. >> As the main beneficiary is pandas (which is not in core), which not add it there as a patch? > > You can opt out by setting PARALLEL_MAKE to -j1. Pandas is where I noticed, but I expect it will make a difference for any package that builds extensions with setuptools. > >> Also other suggest that all that can be achieved with --global-option="build_ext" --global-option="-j5" >> set by PEP517_BUILD_OPTS, which would find the much better options here. >> >> Something like >> >> PARALLEL_PEP517_BUILD ?= " --global-option="build_ext" --global-option="-j<job count>"" >> PEP517_BUILD_OPTS:append = " ${PARALLEL_PEP517_BUILD}” > > That’s possibly a neater solution, yes. I tried using global-option but couldn’t get it to work right. > > Ross > Here's a monkey patch to expose the missing option diff --git a/setuptools/command/bdist_wheel.py b/setuptools/command/bdist_wheel.py index 8cf91538f..b692a7aa3 100644 --- a/setuptools/command/bdist_wheel.py +++ b/setuptools/command/bdist_wheel.py @@ -234,6 +234,11 @@ class bdist_wheel(Command): "result of calling the PEP517 'prepare_metadata_for_build_wheel' " "method)", ), + ( + "parallel=", + "j", + "number of parallel build jobs", + ), ] boolean_options = ["keep-temp", "skip-build", "relative", "universal"] @@ -259,6 +264,7 @@ class bdist_wheel(Command): self.build_number: str | None = None self.py_limited_api: str | Literal[False] = False self.plat_name_supplied = False + self.parallel = None def finalize_options(self) -> None: if not self.bdist_dir: @@ -398,6 +404,7 @@ class bdist_wheel(Command): build_ext = self.reinitialize_command("build_ext") build_ext.inplace = False + build_ext.parallel = self.parallel if not self.skip_build: self.run_command("build") feel free to propose that to upstream, as I think it's the better option in the long run. And yeah I have to agree for pandas it's like a day and night difference, on 20+ cores it's almost fun to compile it now :-) Cheers Konrad
On 07.11.24 22:40, Ross Burton wrote: > On 6 Nov 2024, at 18:30, Konrad Weihmann via lists.openembedded.org <kweihmann=outlook.com@lists.openembedded.org> wrote: >> >> On Tue, Nov 5, 2024 at 01:56 PM, Ross Burton wrote: >> >> I find that patch incredibly risky, as there is no really opt-out. >> As the main beneficiary is pandas (which is not in core), which not add it there as a patch? > > You can opt out by setting PARALLEL_MAKE to -j1. Pandas is where I noticed, but I expect it will make a difference for any package that builds extensions with setuptools. > >> Also other suggest that all that can be achieved with --global-option="build_ext" --global-option="-j5" >> set by PEP517_BUILD_OPTS, which would find the much better options here. >> >> Something like >> >> PARALLEL_PEP517_BUILD ?= " --global-option="build_ext" --global-option="-j<job count>"" >> PEP517_BUILD_OPTS:append = " ${PARALLEL_PEP517_BUILD}” > > That’s possibly a neater solution, yes. I tried using global-option but couldn’t get it to work right. > > Ross > Maybe as an addition, why I would like this feature to added in a very transparent and easy to undo way: https://github.com/pypa/setuptools/issues/3119 - I'm a bit worried that in the wider ecosystem things will fall apart easily, if parallel builds are the default. And as comment for the v2 of the patch: I believe --bdist-dir does set the build dir to a path of your choice already
On 8 Nov 2024, at 07:45, Konrad Weihmann <kweihmann@outlook.com> wrote: > And as comment for the v2 of the patch: > I believe --bdist-dir does set the build dir to a path of your choice already It sets the directory for the binary distribution not the intermediate build tree. Ross
diff --git a/meta/classes-recipe/python_setuptools_build_meta.bbclass b/meta/classes-recipe/python_setuptools_build_meta.bbclass index 4c84d1e8d0b..f4695b08245 100644 --- a/meta/classes-recipe/python_setuptools_build_meta.bbclass +++ b/meta/classes-recipe/python_setuptools_build_meta.bbclass @@ -7,3 +7,12 @@ inherit setuptools3-base python_pep517 DEPENDS += "python3-setuptools-native python3-wheel-native" + +do_compile:prepend() { + # Write an extra config file to build in parallel + export DIST_EXTRA_CONFIG=${WORKDIR}/setuptools-extra.cfg + cat <<EOF >$DIST_EXTRA_CONFIG +[build_ext] +parallel = ${@oe.utils.parallel_make(d)} +EOF +} diff --git a/meta/classes-recipe/setuptools3.bbclass b/meta/classes-recipe/setuptools3.bbclass index 64a78e9a367..f9b892ef832 100644 --- a/meta/classes-recipe/setuptools3.bbclass +++ b/meta/classes-recipe/setuptools3.bbclass @@ -31,6 +31,13 @@ setuptools3_do_configure() { } setuptools3_do_compile() { + # Write an extra config file to build in parallel + export DIST_EXTRA_CONFIG=${WORKDIR}/setuptools-extra.cfg + cat <<EOF >$DIST_EXTRA_CONFIG +[build_ext] +parallel = ${@oe.utils.parallel_make(d)} +EOF + cd ${SETUPTOOLS_SETUP_PATH} NO_FETCH_BUILD=1 \ STAGING_INCDIR=${STAGING_INCDIR} \
Explicitly tell the build_ext task to do parallel builds, as it defaults to not. In my testing this halves the build time of python3-pandas. Signed-off-by: Ross Burton <ross.burton@arm.com> --- meta/classes-recipe/python_setuptools_build_meta.bbclass | 9 +++++++++ meta/classes-recipe/setuptools3.bbclass | 7 +++++++ 2 files changed, 16 insertions(+)