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

Reduce repeated template parsing to speed up builds #769

Draft
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

DanMesh
Copy link

@DanMesh DanMesh commented Sep 26, 2023

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:

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:

def get_ast_from_idl_string(idl_string):
global _parser
if _parser is None:
_parser = Lark(grammar, start='specification', maybe_placeholders=False)
return _parser.parse(idl_string)

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

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.

@DanMesh DanMesh force-pushed the streamline-file-generation branch from 7f6a4da to 5f3eb65 Compare September 27, 2023 08:05
* Separate writing arguments files from the builds of each generator or type support target.
* Write all arguments files first, before generating files.
* Generate all files for each IDL template at once, avoiding repeated template parsing.

Signed-off-by: Daniel Mesham <[email protected]>
@clalancette
Copy link
Contributor

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
Copy link

gonzodepedro commented Mar 1, 2024

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
Copy link
Contributor

tfoote commented Mar 1, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants