Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for controlling the number of jobs used by some cmake tasks #107

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
21 changes: 21 additions & 0 deletions colcon_cmake/task/cmake/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,27 @@ def get_generator(path, cmake_args=None):
return generator


def is_jobs_base_generator(path, cmake_args=None):
hidmic marked this conversation as resolved.
Show resolved Hide resolved
"""
Check if the used CMake generator supports jobs base arguments (-jN).

:param str path: The path of the directory contain the CMake cache file
:param list cmake_args: The CMake command line arguments
:rtype: bool
"""
known_jobs_base_multi_configuration_generators = (
'Ninja Multi-Config',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally made this change on top of PR 106 then pulled this out. You can view this as a defensive or pre-emptive change. It should have little to no impact until then as the ninja multi-config won't really work yet.

)
if not is_multi_configuration_generator(path, cmake_args):
# Historically we assume any non-multi-config generator is jobs based.
return True
generator = get_generator(path, cmake_args)
for multi in known_jobs_base_multi_configuration_generators:
if multi in generator:
return True
return False


def is_multi_configuration_generator(path, cmake_args=None):
"""
Check if the used CMake generator is a multi configuration generator.
Expand Down
75 changes: 47 additions & 28 deletions colcon_cmake/task/cmake/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from colcon_cmake.task.cmake import get_variable_from_cmake_cache
from colcon_cmake.task.cmake import get_visual_studio_version
from colcon_cmake.task.cmake import has_target
from colcon_cmake.task.cmake import is_jobs_base_generator
from colcon_cmake.task.cmake import is_multi_configuration_generator
from colcon_core.environment import create_environment_scripts
from colcon_core.logging import colcon_logger
Expand Down Expand Up @@ -63,6 +64,12 @@ def add_arguments(self, *, parser): # noqa: D102
'--cmake-force-configure',
action='store_true',
help='Force CMake configure step')
parser.add_argument(
'--cmake-jobs',
type=int,
help='Number of jobs to use for supported generators (e.g., Ninja '
'Makefiles). Negative values subtract from the maximum '
'available, so --jobs=-1 uses all bar 1 available threads.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'available, so --jobs=-1 uses all bar 1 available threads.')
'available, so --jobs=-1 uses all but 1 available threads.')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a typo


async def build( # noqa: D102
self, *, additional_hooks=None, skip_hook_creation=False,
Expand Down Expand Up @@ -221,6 +228,8 @@ async def _build(self, args, env, *, additional_targets=None):
if additional_targets:
targets += additional_targets

jobs_base_generator = is_jobs_base_generator(
args.build_base, args.cmake_args)
multi_configuration_generator = is_multi_configuration_generator(
args.build_base, args.cmake_args)
if multi_configuration_generator:
Expand All @@ -240,8 +249,8 @@ async def _build(self, args, env, *, additional_targets=None):
cmd += ['--clean-first']
if multi_configuration_generator:
cmd += ['--config', self._get_configuration(args)]
else:
job_args = self._get_make_arguments(env)
if jobs_base_generator:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the removal of the else and replacing it with this separate condition check relates to ninja multi-config support, but should have no impact in isolation.

job_args = self._get_make_jobs_arguments(args, env)
if job_args:
cmd += ['--'] + job_args
completed = await run(
Expand Down Expand Up @@ -281,7 +290,7 @@ def _get_msbuild_environment(self, args, env):
env['CL'] = ' '.join(cl_split)
return env

def _get_make_arguments(self, env):
def _get_make_jobs_arguments(self, args, env):
hidmic marked this conversation as resolved.
Show resolved Hide resolved
"""
Get the make arguments to limit the number of simultaneously run jobs.

Expand All @@ -291,28 +300,37 @@ def _get_make_arguments(self, env):
:returns: list of make arguments
:rtype: list of strings
"""
# check MAKEFLAGS for -j/--jobs/-l/--load-average arguments
makeflags = env.get('MAKEFLAGS', '')
regex = (
r'(?:^|\s)'
r'(-?(?:j|l)(?:\s*[0-9]+|\s|$))'
r'|'
r'(?:^|\s)'
r'((?:--)?(?:jobs|load-average)(?:(?:=|\s+)[0-9]+|(?:\s|$)))'
)
matches = re.findall(regex, makeflags) or []
matches = [m[0] or m[1] for m in matches]
if matches:
# do not extend make arguments, let MAKEFLAGS set things
return []
# Use the number of CPU cores
jobs = os.cpu_count()
with suppress(AttributeError):
# consider restricted set of CPUs if applicable
jobs = min(jobs, len(os.sched_getaffinity(0)))
if jobs is None:
# the number of cores can't be determined
return []
generator = get_generator(args.build_base)
if 'Makefiles' in generator and args.cmake_jobs is None:
# check MAKEFLAGS for -j/--jobs/-l/--load-average arguments
# Note: Ninja does not support environment variables.
makeflags = env.get('MAKEFLAGS', '')
regex = (
r'(?:^|\s)'
r'(-?(?:j|l)(?:\s*[0-9]+|\s|$))'
r'|'
r'(?:^|\s)'
r'((?:--)?(?:jobs|load-average)(?:(?:=|\s+)[0-9]+|(?:\s|$)))'
)
matches = re.findall(regex, makeflags) or []
matches = [m[0] or m[1] for m in matches]
if matches:
# do not extend make arguments, let MAKEFLAGS set things
return []
# Use command line specified jobs if positive.
jobs_args = args.cmake_jobs if args.cmake_jobs is not None else 0
jobs = 0
if jobs_args <= 0:
hidmic marked this conversation as resolved.
Show resolved Hide resolved
# Base off the number of CPU cores if jobs arg non-positive.
jobs = os.cpu_count()
with suppress(AttributeError):
# consider restricted set of CPUs if applicable
jobs = min(jobs, len(os.sched_getaffinity(0)))
if jobs is None:
# the number of cores can't be determined
return []
# Finalize jobs as as CPU count deducting the limit specified.
jobs = max(jobs + jobs_args, 1)
return [
'-j{jobs}'.format_map(locals()),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rename of the function is based on this result. It can only return jobs. Will revert if appropriate.

'-l{jobs}'.format_map(locals()),
Expand All @@ -325,7 +343,8 @@ async def _install(self, args, env):
raise RuntimeError("Could not find 'cmake' executable")
cmd = [CMAKE_EXECUTABLE]
cmake_ver = get_cmake_version()
allow_job_args = True
allow_job_args = is_jobs_base_generator(
args.build_base, args.cmake_args)
if cmake_ver and cmake_ver >= parse_version('3.15.0'):
# CMake 3.15+ supports invoking `cmake --install`
cmd += ['--install', args.build_base]
Expand All @@ -343,8 +362,8 @@ async def _install(self, args, env):
args.build_base, args.cmake_args)
if multi_configuration_generator:
cmd += ['--config', self._get_configuration(args)]
elif allow_job_args:
job_args = self._get_make_arguments(env)
if allow_job_args:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This elif to if change also supports ninja multi-config.

job_args = self._get_make_jobs_arguments(args, env)
if job_args:
cmd += ['--'] + job_args
return await run(
Expand Down