config:build_jobs is capped by the number of cores available

alalazo · alalazo · commit 1c5fb9b13555 · 2019-05-27T15:19:20.000+02:00
config:build_jobs controls the number of parallel jobs to spawn during
builds, but cannot ever exceed the number of cores on the machine.

The default is set to 16 or the number of available cores, whatever
is lowest.
diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml
@@ -99,10 +99,12 @@ config:
   locks: true
 
 
-  # The default number of jobs to use when running `make` in parallel.
-  # If set to 4, for example, `spack install` will run `make -j4`.
-  # If not set, all available cores are used by default.
-  # build_jobs: 4
+  # The tentative number of jobs to use when running `make` in parallel,
+  # always limited by the number of cores available. For instance:
+  # - If set to 16 on a 4 cores machine `spack install` will run `make -j4`
+  # - If set to 16 on a 18 cores machine `spack install` will run `make -j16`
+  # If not set, Spack will use all available cores up to 16.
+  # build_jobs: 16
 
 
   # If set to true, Spack will use ccache to cache C compiles.
diff --git a/lib/spack/spack/cmd/common/arguments.py b/lib/spack/spack/cmd/common/arguments.py
@@ -5,6 +5,7 @@
 
 
 import argparse
+import multiprocessing
 
 import spack.cmd
 import spack.config
@@ -86,6 +87,7 @@ def __call__(self, parser, namespace, jobs, option_string):
                   '[expected a positive integer, got "{1}"]'
             raise ValueError(msg.format(option_string, jobs))
 
+        jobs = min(jobs, multiprocessing.cpu_count())
         spack.config.set('config:build_jobs', jobs, scope='command_line')
 
         setattr(namespace, 'jobs', jobs)
@@ -94,7 +96,8 @@ def __call__(self, parser, namespace, jobs, option_string):
     def default(self):
         # This default is coded as a property so that look-up
         # of this value is done only on demand
-        return spack.config.get('config:build_jobs')
+        return min(spack.config.get('config:build_jobs'),
+                   multiprocessing.cpu_count())
 
     @default.setter
     def default(self, value):
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
@@ -100,7 +100,7 @@
         'verify_ssl': True,
         'checksum': True,
         'dirty': False,
-        'build_jobs': multiprocessing.cpu_count(),
+        'build_jobs': min(16, multiprocessing.cpu_count()),
     }
 }
 
diff --git a/lib/spack/spack/test/cmd/common/arguments.py b/lib/spack/spack/test/cmd/common/arguments.py
@@ -20,14 +20,38 @@ def parser():
     yield p
     # Cleanup the command line scope if it was set during tests
     if 'command_line' in spack.config.config.scopes:
-        spack.config.config.remove_scope('command_line')
+        spack.config.config.scopes['command_line'].clear()
 
 
-@pytest.mark.parametrize('cli_args,expected', [
+@pytest.fixture(params=[1, 2, 4, 8, 16, 32])
+def ncores(monkeypatch, request):
+    """Mocks having a machine with n cores for the purpose of
+    computing config:build_jobs.
+    """
+    def _cpu_count():
+        return request.param
+
+    # Patch multiprocessing.cpu_count() to return the value we need
+    monkeypatch.setattr(multiprocessing, 'cpu_count', _cpu_count)
+    # Patch the configuration parts that have been cached already
+    monkeypatch.setitem(spack.config.config_defaults['config'],
+                        'build_jobs', min(16, request.param))
+    monkeypatch.setitem(
+        spack.config.config.scopes, '_builtin',
+        spack.config.InternalConfigScope(
+            '_builtin', spack.config.config_defaults
+        ))
+    return request.param
+
+
+@pytest.mark.parametrize('cli_args,requested', [
     (['-j', '24'], 24),
-    ([], multiprocessing.cpu_count())
+    # Here we report the default if we have enough cores, as the cap
+    # on the available number of cores will be taken care of in the test
+    ([], 16)
 ])
-def test_setting_parallel_jobs(parser, cli_args, expected):
+def test_setting_parallel_jobs(parser, cli_args, ncores, requested):
+    expected = min(requested, ncores)
     namespace = parser.parse_args(cli_args)
     assert namespace.jobs == expected
     assert spack.config.get('config:build_jobs') == expected

Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,7 @@`
`100`	`100`	`'verify_ssl': True,`
`101`	`101`	`'checksum': True,`
`102`	`102`	`'dirty': False,`
`103`		`- 'build_jobs': multiprocessing.cpu_count(),`
	`103`	`+ 'build_jobs': min(16, multiprocessing.cpu_count()),`
`104`	`104`	`}`
`105`	`105`	`}`
`106`	`106`