build: Improve consistency of parallelism flags among make->python dependencies #20065

Activity

1. chrismilleruk commented on Apr 16, 2018

   

   chrismilleruk

   on Apr 16, 2018

   ContributorAuthor

   More actions

   @rvagg commented:

   You can -j X with make but it doesn't propagate to tools/test.py, it only does the build in parallel. The call to tools/test.py is given a -J, it doesn't pass X on. Here's what test.py does when it's run with -J:

   If JOBS is set in the current environment then use that as the parallel count
   If there is no sensible JOBS env var then use the Python multiprocessing.cpu_count() call which can be inaccurate on some VMs or containers (like os.cpus().length .. SmartOS is one of these).
   So, what we tend to do in CI is be explicit about it, with something like:

   if [ -z ${JOBS+x} ]; then
     JOBS=$(getconf _NPROCESSORS_ONLN) #reported cores, kind of like `os.cpus().length`
   fi
   JOBS=$JOBS make test-ci -j $JOBS
   

   And in some configurations we explicitly set a JOBS, like on SmartOS and our 96-core ARMv8 hosts that shouldn't be running with a parallellism of 96.

   My wish is that -j X was definitive for both building and testing, but I don't believe you can fetch the -j value from inside a Makefile and you'd have to do some ppid hackery to look it up from the tools/test.py subprocess.
2. chrismilleruk commented on Apr 16, 2018

   

   chrismilleruk

   on Apr 16, 2018

   Last edited by chrismilleruk

   ContributorAuthor

   More actions

   I noticed three different methods for calling test.py from Makefile

   (1) Called with the -J flag (4 times)
   e.g. $(PYTHON) tools/test.py --mode=release parallel -J

   (2) Called with PARALLEL_ARGS (3 times)
   e.g. $(PYTHON) tools/test.py $(PARALLEL_ARGS) ...

   (3) Called without -J or PARALLEL_ARGS (17 times)
   e.g. $(PYTHON) tools/test.py --mode=debug,release

   The -J flag is defined here:

     if options.J:
       # inherit JOBS from environment if provided. some virtualised systems
       # tends to exaggerate the number of available cpus/cores.
       cores = os.environ.get('JOBS')
       options.j = int(cores) if cores is not None else multiprocessing.cpu_count()

   PARALLEL_ARGS is defined here:

   ifdef JOBS
     PARALLEL_ARGS = -j $(JOBS)
   endif
   

   These appear to be more or less the same except that -J falls back to a sensible default (number of cores).

   So I have two key questions:
   (1) Should -J and PARALLEL_ARGS be merged / normalised or is there a good reason to keep both?
   (2) Should the non-parallel calls be made parallel (for consistency and/or performance)?
3. rvagg commented on Apr 18, 2018

   

   rvagg

   on Apr 18, 2018

   Member

   More actions

   Yeah, I think it could be normalised. Maybe we go with PARALLEL_ARGS and default it to (a) -j$JOBS then (b) -J. It looks like tools/lint-js.js takes both, like tools/test.py so that should be safe I think. I'm not seeing a good reason why we wouldn't just pass on PARALLEL_ARGS to all instances of test.py but perhaps I'm overlooking something.
4. chrismilleruk

   added a commit that references this issue on Apr 18, 2018

   build: normalise test.py calls to use PARALLEL_ARGS

   ...

   1343f4f
5. BridgeAR

   added a commit that references this issue on Apr 23, 2018

   build: normalise test.py calls to use PARALLEL_ARGS

   ...

   8786889
6. BridgeAR

   closed this as completedin 8786889on Apr 23, 2018
7. jasnell

   added a commit that references this issue on Apr 23, 2018

   build: normalise test.py calls to use PARALLEL_ARGS

   ...

   a4cba2d