colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

Produce minimal size binaries in a simpler fashion #42

Closed AAlon closed 4 years ago

AAlon commented 5 years ago

Hello, This is a proposal for supporting a --minify option (or similar) in colcon build.

Problem Statement

Proposal

Running colcon build --minify would have the following effect:

  1. Implies --cmake-args ' -DCMAKE_BUILD_TYPE=MinSizeRel' (and in turn, -Os)
  2. On Linux/Unix-based systems, seamlessly run strip --strip-unneeded on all resulting binaries

Implementation

I've implemented a proof of concept in the following manner. If the general idea makes sense, I'll be happy to hear your thoughts about a better way of implementing this.

  1. Export build targets from ament_cmake_core/ament_package.cmake with the following addition to the ament_package macro:
    get_directory_property(BUILDSYSTEM_TARGETS_VAR BUILDSYSTEM_TARGETS)
    set(EXPORTED_BUILD_TARGETS "${BUILDSYSTEM_TARGETS_VAR}" CACHE STRING)
  2. colcon-cmake/build.py - scans the build directory for any of the names exported from ament and runs strip after build. Roughly:
    
    # _reconfigure
    if args.minify:
    cmake_args += ['-DCMAKE_BUILD_TYPE=MinSizeRel']

_build

build_targets = get_variable_from_cmake_cache(args.build_base, 'EXPORTED_BUILD_TARGETS') files_in_build_dir = glob.glob(os.path.join(args.build_base, '*')) for each name in files_in_build_dir, if it matches a build target in build_targets, invoke strip



### Deliberately Omitted ###
This proposal does not cover everything. Specifically, non-ament packages or the dependency on `strip` were not dealt with. If we have a green light about the general idea we'll iron out those issues too.

### Example ###
* Default: `colcon build --packages-select rclcpp`
Size of librclcpp.so: **7.6MB**
Size of the entire build directory ./build/rclcpp: **97MB**
* Minified: `colcon build --cmake-clean-cache --packages-select rclcpp --minify`
Size of librclcpp.so: **966K**
Size of the entire build directory ./build/rclcpp: **32M**
For reference, a MinSizeRel build without strip produces a **1.4MB** librclcpp.so.
dirk-thomas commented 5 years ago
  1. Implies --cmake-args ' -DCMAKE_BUILD_TYPE=MinSizeRel' (and in turn, -Os)

This can be achieved by a trivial mixin and doesn't require a new command line option. There already exists one for this specifica similar build type: see https://github.com/colcon/colcon-mixin-repository/blob/d34115187a85fd170311d5e452211ef71c300798/build-type.mixin#L6-L7

  1. On Linux/Unix-based systems, seamlessly run strip --strip-unneeded on all resulting binaries

Can you clarify on what kind of targets you want to run this. Executables and shared libraries only? Are there any alternatives to perform the stripping in a post-processing step, e.g. a compiler / linker option?

AAlon commented 5 years ago
  1. Implies --cmake-args ' -DCMAKE_BUILD_TYPE=MinSizeRel' (and in turn, -Os)

This can be achieved by a trivial mixin and doesn't require a new command line option. There already exists one for this specific build type: see https://github.com/colcon/colcon-mixin-repository/blob/d34115187a85fd170311d5e452211ef71c300798/build-type.mixin#L6-L7

Oh nice. There doesn't seem to be one for MinRelSize though. Opened https://github.com/colcon/colcon-mixin-repository/pull/18

  1. On Linux/Unix-based systems, seamlessly run strip --strip-unneeded on all resulting binaries

Can you clarify on what kind of targets you want to run this. Executables and shared libraries only? Are there any alternatives to perform the stripping in a post-processing step, e.g. a compiler / linker option?

I would want to run it on all executables and shared libraries. In the current implementation which uses CMake's BUILDSYSTEM_TARGETS, we'd be running it against all names provided to add_library(), add_executable(), and add_custom_target() which were found in the workspace.

Compiler and linker options: that would be better but I'm not sure how to pass them down from colcon in a sane way. In CMake, the proper way would be to use add_compile_options and add_link_options, something like: add_compile_options(-Os -s -fdata-sections -ffunction-sections) add_link_options(-Wl,--gc-sections -Wl,--strip-all) Should produce a binary similar in size to using strip. And -flto could perhaps improve it further.

But from colcon you need specify the flags via CMake's command line - and there's no "append" option; If we pass options like CMAKE_CXX_FLAGS, CMAKE_SHARED_LINKER_FLAGS via the command line, they would override existing flags. Any ideas?

dirk-thomas commented 5 years ago

If we pass options like CMAKE_CXX_FLAGS, CMAKE_SHARED_LINKER_FLAGS via the command line, they would override existing flags.

What flags do you expect to be overridden when setting these?

AAlon commented 5 years ago

If we pass options like CMAKE_CXX_FLAGS, CMAKE_SHARED_LINKER_FLAGS via the command line, they would override existing flags.

What flags do you expect to be overridden when setting these?

For size-optimized compile options you'd typically want -Os -s -fdata-sections -ffunction-sections (the -Os would override any previously specified -O<n>). Maybe -flto.

We'd want existing options to remain as is (for example, if the package developer specified -Wall -pedantic), so specifying our options via the command line gets a bit tricky.

dirk-thomas commented 5 years ago

for example, if the package developer specified -Wall -pedantic) ...

Can you point to a specific package where you see the problem?

The recommendation for the options you mention is to use: add_compile_options(). And even if a package sets CMAKE_CXX_FLAGS directly it should always prepend/append values rather than overwrite: e.g. https://github.com/ros2/rcutils/blob/4cefa4ab44b73261fea57fea50ae3be536577c63/CMakeLists.txt#L113

AAlon commented 5 years ago

for example, if the package developer specified -Wall -pedantic) ...

Can you point to a specific package where you see the problem?

The recommendation for the options you mention is to use: add_compile_options(). And even if a package sets CMAKE_CXX_FLAGS directly it should always prepend/append values rather than overwrite: e.g. https://github.com/ros2/rcutils/blob/4cefa4ab44b73261fea57fea50ae3be536577c63/CMakeLists.txt#L113

This is not about any specific package... as I mentioned in the problem statement, this has more to do with ROS as a whole: a developer who uses ROS packages (ROS core included) has no control over the CMakeLists.txt of those packages, and would need some streamlined way of producing stripped, minimal size binaries. The only way to influence compilation options without modifying the package itself is through the build tools, i.e. colcon, hence the current proposal.

dirk-thomas commented 5 years ago

If the CMake code of a package isn't "cooperative" there is no way you are able to choose these kind of settings from the outside. A package could simply overwrite the value provided as CMake arguments.

Therefore I think it is very reasonable to expect that package adhere to a certain recommended way in order for them to allow this kind of customization. Another common use case which is similar to this is cross compilation - where extra arguments are passed in from the outside and each package must not do things which prevent that from being effective.

AAlon commented 5 years ago

If the CMake code of a package isn't "cooperative" there is no way you are able to choose these kind of settings from the outside. A package could simply overwrite the value provided as CMake arguments.

You would be able to choose the default behavior from outside and the package could override it. That's fine but it doesn't exist today.

Therefore I think it is very reasonable to expect that package adhere to a certain recommended way in order for them to allow this kind of customization. Another common use case which is similar to this is cross compilation - where extra arguments are passed in from the outside and each package must not do things which prevent that from being effective.

I don't think you're getting to the bottom of the issue. Say I want to deploy a ROS application to my armhf-based robot which doesn't have a lot of RAM available. I would need to build the entire (or most of) ROS2 from source, as the buildfarm does not produce binaries for armhf. Most packages do not specify any special options to strip the binaries or produce minimal size artifacts, so it will be up to me to do any post-processing on those binaries to reduce their footprint and in turn help reduce the RAM usage. This effort is:

  1. Non-trivial - I would have to research and figure out those steps myself.
  2. Time consuming.
  3. Would be duplicated time and time again by developers in the community.

Therefore, we need to have a way to colcon build minimal size artfiacts.

dirk-thomas commented 5 years ago

If the CMake code of a package isn't "cooperative" there is no way you are able to choose these kind of settings from the outside. A package could simply overwrite the value provided as CMake arguments.

You would be able to choose the default behavior from outside and the package could override it. That's fine but it doesn't exist today.

I think you are misunderstanding my point: if a package does do something "wrong" in its CMake code you will never ever we able to pass CMake flags to it (using either CMAKE_CXX_FLAGS or -D...) and have them actually being applied. Basically because the CMake might completely ignore / overwrite what you are passing. Therefore the base assumption for you goal must be that CMake code in packages is following certain recommendations allow us to do so. If any package isn't doing that we need to update it. Hence my repeated question for specific packages where you see the problem.

I don't think you're getting to the bottom of the issue. Say I want to deploy a ROS application to my armhf-based robot which doesn't have a lot of RAM available. I would need to build the entire (or most of) ROS2 from source

We are on the same page on this :+1:

as the buildfarm does not produce binaries for armhf.

Just as a side note: Dashing will have a archive built for armhf (see https://github.com/ros-infrastructure/rep/blob/master/rep-2000.rst#dashing-diademata-may-2019---may-2021).

Most packages do not specify any special options to strip the binaries or produce minimal size artifacts, so it will be up to me to do

Up to here we are still on the same page. :+1:

any post-processing on those binaries to reduce their footprint and in turn help reduce the RAM usage

Based on the conversation I thought it is feasible to do that during the building / linking rather than in a post-processing step (please ignore for now how the arguments are being passed through). Do you agree to that - I thought your posted compiler and linker flags would achieve just that? Or do you think it is technically not possible to do it without post-processing?

dirk-thomas commented 5 years ago

Based on the conversation I thought it is feasible to do that during the building / linking rather than in a post-processing step (please ignore for now how the arguments are being passed through). Do you agree to that - I thought your posted compiler and linker flags would achieve just that? Or do you think it is technically not possible to do it without post-processing?

@AAlon Friendly ping.

dirk-thomas commented 4 years ago

@AAlon Friendly ping.

dirk-thomas commented 4 years ago

Closing due to no further response.