Reduce repeated template parsing to speed up builds

DanMesh commented 1 year ago

Building ROS messages is rather slow, and takes longer the more messages there are. With native builds, it is still acceptable, but when cross-compiling or doing emulated builds it is then very slow and is an unpleasant development experience.

I'd like to start a discussion on how to remedy this, and this PR offers a possible solution.

The problem

When building ROS messages, an IDL template is made for each message definition. These are used to generate files for each of the generators or type supports available, provided by the corresponding packages (e.g. rosidl_generator_c, rosidl_typesupport_introspection_c). The expansion of these templates is done using an arguments file for each target.

Until now, this expansion was done in generate_files() here: https://github.com/ros2/rosidl/blob/3491f4fbda1f9c0184b0de5f5e5029f27bf74f23/rosidl_pycommon/rosidl_pycommon/__init__.py#L51

generate_files() is called once per target, using a single arguments file. It then iterates through each message, parses the IDL template and generates the files from it.

For a message set of M messages and T targets, this means that the IDL parsing occurs M*T times. The parsing of the IDL file is relatively slow, done here: https://github.com/ros2/rosidl/blob/3491f4fbda1f9c0184b0de5f5e5029f27bf74f23/rosidl_parser/rosidl_parser/parser.py#L85-L89

In pseudo-code, this looks as follows:

for t in targets:
    arg_file = arg_file_for_target(t)
    for idl_file in message_idl_files:
        template = parse(idl_file)
        generate_files_from_template(template, arg_file)

This approach

The idea is to reorder the two loops above, such that we have:

for idl_file in message_idl_files:
    template = parse(idl_file)
    for t in targets:
        arg_file = arg_file_for_target(t)
        generate_files_from_template(template, arg_file)

To achieve this:

Writing of the arguments files is separated from the building of each target. For example, for package rosidl_generator_c:
- The argument-writing part of cmake/rosidl_generator_c_generate_interfaces.cmake is moved into a new file, cmake/rosidl_generator_c_generate_interfaces.cmake.
- This script is registered as an ament extension in the group rosidl_write_generator_arguments_extensions, which is executed before the generator extension group.
generate_files() is split into two parts:
1. generate_files_from_arguments_files()
  - Executed from rosidl_cmake/cmake/rosidl_generate_interfaces.cmake once all arguments files have been written.
  - For each IDL template, it constructs a list of "generator configs" based on each arguments file.
2. generate_files_for_idl_tuple()
  - Takes an IDL file and its list of generator configs.
  - Parses the IDL template once, then iterates through each generator config.
  - For each, expands the template according to the given arguments.

This means the templates are only parsed once each, so the template-parsing occurs only M times for M messages.

Results

Native builds of https://github.com/PX4/px4_msgs (190 messages) on an amd64 machine initially took 166 seconds, and with these changes take 126 seconds.

With a bit more work, it should be possible to reduce this further (see Notes below).

Other packages to update and corresponding PRs

https://github.com/ros2/rosidl_typesupport/pull/148
- rosidl_typesupport_c
- rosidl_typesupport_cpp
https://github.com/ros2/rosidl_typesupport_fastrtps/pull/107
- rosidl_typesupport_fastrtps_c
- rosidl_typesupport_fastrtps_cpp
https://github.com/ros2/rosidl_python (PR in progress)
- rosidl_generator_py

Notes

This is an initial attempt at improving the build system - suggestions and ideas are very welcome!
In addition to the other packages listed above, there may be others that need to be (or could be updated).
- If more packages are updated to this structure, this would further reduce the build time. Otherwise the non-updated packages are still building the old way, duplicating IDL-parsing effort.
In rosidl_pycommon, the function generate_files() cannot be removed yet because most packages still want to use it in a "CLI" call. This is something I have not looked into and would be open to ideas.

clalancette commented 1 year ago

I think the idea is fantastic; slow build times of messages has been one of the stumbling blocks in ROS 2 for quite a long time. So thanks for starting the conversation. Your analysis of the problem is also very well done.

I haven't looked at this in any detail, but my biggest concerns here are how this slots in with the rest of the "pluggable" nature of the rosidl pipeline, and how it interacts with the "command-line" generation of rosidl (which is used by non-CMake generators, like bazel).

I'll try to spend some time later this week on reviewing this and seeing how it fits in with the rest of the system.

gonzodepedro commented 8 months ago

I think the idea is fantastic; slow build times of messages has been one of the stumbling blocks in ROS 2 for quite a long time. So thanks for starting the conversation. Your analysis of the problem is also very well done.

I haven't looked at this in any detail, but my biggest concerns here are how this slots in with the rest of the "pluggable" nature of the rosidl pipeline, and how it interacts with the "command-line" generation of rosidl (which is used by non-CMake generators, like bazel).

I'll try to spend some time later this week on reviewing this and seeing how it fits in with the rest of the system.

About your first concern, this extends the functionality currently in place. So a "legacy" plugin will still work and a new plugin can benefit from it. Command-line generation should still work or made to work. Can you point me to code that uses non-CMake generators to check?

tfoote commented 8 months ago

One place where message generation outside of CMake is done is in the bazel_ros2_rules package in the Drake-ROS repo: https://github.com/RobotLocomotion/drake-ros/blob/main/bazel_ros2_rules/ros2/rosidl.bzl

If you can build a ROS workspace with your changes, the ros2_example_bazel_installed workspace is a good one to validate against I think: https://github.com/RobotLocomotion/drake-ros/blob/4c17af07b5211424af654bbca4cb290cdb6c4085/ros2_example_bazel_installed/WORKSPACE#L86

Thanks @sloretz for the reference

ros2 / rosidl