On Sun, Dec 12, 2021 at 4:37 AM Daniel Latypov dlatypov@google.com wrote:
On Sat, Dec 11, 2021 at 12:49 AM David Gow davidgow@google.com wrote:
The --jobs parameter for kunit_tool currently defaults to 8 CPUs, regardless of the number available. For systems with significantly more (or less), this is not as efficient. Instead, default --jobs to the number of CPUs present in the system: while there are as many superstitions as to exactly what the ideal jobs:CPU ratio is, this seems sufficiently sensible to me.
Signed-off-by: David Gow davidgow@google.com
Reminder: the unit tests depend on this hard-coded value. $ ag '\b8\b' tools/testing/kunit/kunit_tool_test.py 422: self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, '.kunit', None) 529: self.linux_source_mock.build_kernel.assert_called_once_with(False, 8, build_dir, None)
Gah: that's what I get for rushing this through at the end of the day.
v2 will have this no-longer hardcoded, but call a get_default_jobs() function which calculates the default.
tools/testing/kunit/kunit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 68e6f461c758..2cb6c7db5683 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -310,7 +310,7 @@ def add_build_opts(parser) -> None: parser.add_argument('--jobs', help='As in the make command, "Specifies the number of ' 'jobs (commands) to run simultaneously."',
type=int, default=8, metavar='jobs')
type=int, default=os.cpu_count(), metavar='jobs')Just looking for edge cases: https://docs.python.org/3/library/os.html#os.cpu_count says
Returns None if undetermined
and
This number is not equivalent to the number of CPUs the current process can use. The number of usable CPUs can be obtained with len(os.sched_getaffinity(0))
I assume the None caveat is mainly for other operating systems and doubt it'll impact any users. The second point is a bit more interesting, but still niche. Up to you if you want to use that instead.
Super unscientific comparison (n=1) running all on CPU #0
$ taskset 0x1 ./tools/testing/kunit/kunit.py run --jobs=1 Elapsed time: ... 155.978s building ...
--jobs=2 (some people swear by the 2x ratio) Elapsed time: ... 158.891s building ...
--jobs=8 (Old behavior) ... Elapsed time: ... 171.448s building
--jobs=32 Elapsed time: ... 170.765s building ...
So the overhead of j being "too high" isn't that bad and it doesn't seem to matter much either way.
I'll go with len(os.sched_getaffinity(0)) in v2: it is technically closer to what the intent is here, even if it's not a problem in general. Putting it in a helper function will make the len(os.sched_getaffinity(0)) call less unwieldy, IMHO, too.
def add_exec_opts(parser) -> None: parser.add_argument('--timeout', -- 2.34.1.173.g76aa8bc2d0-goog
Cheers, -- David