#30345 closed defect (fixed)

build/make/Makefile.in: Filter out "-j" from sub-make invocations to avoid excessive parallel load

Reported by: mkoeppe Owned by:
Priority: blocker Milestone: sage-9.2
Component: build Keywords:
Cc: mjo, gh-kliem, jhpalmieri Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 701b4ce (Commits, GitHub, GitLab) Commit: 701b4ceec489024aa76617a276cf7a9cec4adeb3
Dependencies: Stopgaps:

Status badges

Description

Fixup from #30153, where recursive invocations of $(MAKE) for SPKG-no-deps targets were added.

The recursive invocation of $(MAKE) seems to lead to builds with extremely high parallel load when MAKE="make -j8" as recommended in Sage build documentation. This may be part of why lately many builds on GH Actions are failing.

Change History (16)

comment:1 Changed 23 months ago by mkoeppe

Example, from https://github.com/mkoeppe/sage/runs/977866778:

make[1]: Entering directory '/sage/build/make'
make -j8 yasm-no-deps
make -j8 gf2x-no-deps
make -j8 boost_cropped-no-deps
make -j8 zlib-no-deps
make -j8 xz-no-deps
make -j8 ncurses-no-deps
make -j8 bzip2-no-deps
make -j8 libffi-no-deps
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Entering directory '/sage/build/make'
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: warning: -jN forced in submake: disabling jobserver mode.
make[2]: Entering directory '/sage/build/make'
make[2]: warning: -jN forced in submake: disabling jobserver mode.

Looks like 64 parallel jobs to me...

comment:2 Changed 23 months ago by mkoeppe

  • Authors set to Matthias Koeppe

comment:3 Changed 23 months ago by mkoeppe

  • Branch set to u/mkoeppe/build_make_makefile_in__filter_out___j__from_sub_make_invocations_to_avoid_excessive_parallel_load

comment:4 Changed 23 months ago by mkoeppe

  • Commit set to 7240f3dbd1a515790100724b654c65fe01adceba
  • Status changed from new to needs_review

New commits:

7240f3dbuild/make/Makefile.in: Filter out "-j" from sub-make invocations to avoid excessive parallel load

comment:5 Changed 23 months ago by mkoeppe

  • Status changed from needs_review to needs_work

Now I see

make --no-print-directory planarity-no-deps
make[2]: warning: jobserver unavailable: using -j1.  Add `+' to parent make rule.
sage-logger -p 'SAGE_CHECK=warn sage-spkg -y -o   planarity-3.0.0.5.p0' 

e.g. at https://github.com/mkoeppe/sage/runs/978617323

comment:6 Changed 23 months ago by git

  • Commit changed from 7240f3dbd1a515790100724b654c65fe01adceba to 701b4ceec489024aa76617a276cf7a9cec4adeb3

Branch pushed to git repo; I updated commit sha1. New commits:

701b4cebuild/make/Makefile.in: Add + before recursive make invocations

comment:7 Changed 23 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:8 follow-up: Changed 23 months ago by dimpase

what is the meaning of this syntax:

MAKE_REC = $(MAKE:-j%=)

in Makefile.in ?

comment:9 follow-up: Changed 23 months ago by mjo

This is going to wind up doing the wrong thing in a lot of places but I'm not sure how to fix it. Using the top-level makefile as a convenient way to run high-level commands means that we have two different interpretations of the -j argument,

  1. How many high-level commands do you want to run at the same time?
  2. How many parallel build processes do you want to run when the high-level command being executed is to install a package?

Right now we don't distinguish them. For example, I want to build each spkg with 4 threads, not try to build 4 spkgs at the same time. Maybe a new sage-specific variable like SAGE_BUILD_JOBS could be used to pass the right -j argument to sub-make while the top-level make would always be invoked with -j1. Just an idea.

In any case... can some of these recursive calls be eliminated? E.g. this one...

# The 2 preliminary build phases: base and toolchain.                           
base-toolchain: _clean-broken-gcc base
        $(MAKE) toolchain

can probably be eliminated by making _clean-broken-gcc base a prerequisite for toolchain. Then base-toolchain can simply depend on both, if it needs to be kept around at all. But we could probably make other stuff depend on base toolchain instead of base-toolchain afterwards.

comment:10 in reply to: ↑ 8 Changed 23 months ago by mkoeppe

Replying to dimpase:

what is the meaning of this syntax:

MAKE_REC = $(MAKE:-j%=)

in Makefile.in ?

This is an instance of pattern substitution $(VARIABLE:FROM=TO), https://www.gnu.org/software/make/manual/html_node/Substitution-Refs.html

comment:11 in reply to: ↑ 9 Changed 23 months ago by mkoeppe

Let me just quickly say that the present ticket is a hotfix for breakage caused by #30153. Let's please keep the scope limited so we can it into the next beta.

For the big picture discussion regarding this whole mess of why we even recommend MAKE="make -j8" make build instead of the more common make -j8 build, let's continue on #21610

comment:12 Changed 23 months ago by mkoeppe

  • Priority changed from critical to blocker

comment:13 Changed 23 months ago by dimpase

  • Reviewers set to Dima Pasechnik
  • Status changed from needs_review to positive_review

it does the job, let us get it in.

As to recommendation for setting MAKE, it could be due to an outdated make, as discussed on #21610

comment:14 Changed 23 months ago by mkoeppe

Thank you!

comment:15 Changed 23 months ago by mkoeppe

Follow-up: #30369

comment:16 Changed 23 months ago by vbraun

  • Branch changed from u/mkoeppe/build_make_makefile_in__filter_out___j__from_sub_make_invocations_to_avoid_excessive_parallel_load to 701b4ceec489024aa76617a276cf7a9cec4adeb3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.