-
Notifications
You must be signed in to change notification settings - Fork 128
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
base: rolling
Are you sure you want to change the base?
Conversation
7f6a4da
to
5f3eb65
Compare
* 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]>
5f3eb65
to
3939950
Compare
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. |
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 |
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:rosidl/rosidl_pycommon/rosidl_pycommon/__init__.py
Line 51 in 3491f4f
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 andT
targets, this means that the IDL parsing occursM*T
times. The parsing of the IDL file is relatively slow, done here:rosidl/rosidl_parser/rosidl_parser/parser.py
Lines 85 to 89 in 3491f4f
In pseudo-code, this looks as follows:
This approach
The idea is to reorder the two loops above, such that we have:
To achieve this:
rosidl_generator_c
:cmake/rosidl_generator_c_generate_interfaces.cmake
is moved into a new file,cmake/rosidl_generator_c_generate_interfaces.cmake
.rosidl_write_generator_arguments_extensions
, which is executed before the generator extension group.generate_files()
is split into two parts:generate_files_from_arguments_files()
rosidl_cmake/cmake/rosidl_generate_interfaces.cmake
once all arguments files have been written.generate_files_for_idl_tuple()
This means the templates are only parsed once each, so the template-parsing occurs only
M
times forM
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
rosidl_typesupport_c
rosidl_typesupport_cpp
rosidl_typesupport_fastrtps_c
rosidl_typesupport_fastrtps_cpp
rosidl_generator_py
Notes
rosidl_pycommon
, the functiongenerate_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.