perf: deep packages are swarmed with duplicated libraries in `ament_target_dependencies()`

[VRichardJP]

Here is a cmake trace when building the autoware_behavior_path_planner:

Most Autoware packages rely on ament_auto_* macros to build and export libraries.

This feels a bit like https://github.com/ament/ament_cmake/issues/442, except it's no more ament_libraries_deduplicate's fault (it is rather pretty fast):

By adding some log messages here and there, I have found that this package is swarmed by duplicated libraries:

diff --git a/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake b/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
index 50bb69c..e18e94f 100644
--- a/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
+++ b/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
@@ -110,6 +110,10 @@ function(ament_target_dependencies target)
         # otherwise use the classic CMake variables
         list_append_unique(definitions ${${package_name}_DEFINITIONS})
         list_append_unique(include_dirs ${${package_name}_INCLUDE_DIRS})
+        message(STATUS "####################################")
+        message(STATUS "package: ${package_name}")
+        message(STATUS "libraries: ${${package_name}_LIBRARIES}")
+        message(STATUS "####################################")
         foreach(library ${${package_name}_LIBRARIES})
           if(NOT "${${package_name}_LIBRARY_DIRS}" STREQUAL "")
             if(NOT IS_ABSOLUTE ${library} OR NOT EXISTS ${library})

Here is just the log output from a single dependency of autoware_behavior_path_planner: output.txt

There are 8410 libraries, but only 669 of them are unique.

So the cmake processing is slow simply because ament_target_dependencies loops over way too many duplicated items.

It could be a mistake from Autoware's use of ament_auto_* macros, but it feels strange to find so many duplicates. Any idea?

[VRichardJP]

A quick and dirty list(REMOVE_DUPLICATES) shows there is definitely room for improvement. The cmake processing time dropped from 30 seconds to 8 seconds on this package:

diff --git a/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake b/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
index 50bb69c..3c0e21b 100644
--- a/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
+++ b/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake
@@ -73,6 +73,11 @@ function(ament_target_dependencies target)
     set(libraries "")
     set(link_flags "")
     foreach(package_name ${ARG_UNPARSED_ARGUMENTS})
+      list(REMOVE_DUPLICATES ${package_name}_LIBRARIES)
+      list(REMOVE_DUPLICATES ${package_name}_TARGETS)
+      list(REMOVE_DUPLICATES ${package_name}_INTERFACES)
+      list(REMOVE_DUPLICATES ${package_name}_DEFINITIONS)
+      list(REMOVE_DUPLICATES ${package_name}_INCLUDE_DIRS)
       if(NOT "${${package_name}_FOUND}")
         message(FATAL_ERROR "ament_target_dependencies() the passed package name '${package_name}' was not found before")
       endif()

[VRichardJP]

Even ament_target_dependencies(target SYSTEM builtin_interfaces) brings a lot of duplicated libraries:

-- #############################
-- package_name: builtin_interfaces
-- targets: builtin_interfaces::builtin_interfaces__rosidl_generator_c;builtin_interfaces::builtin_interfaces__rosidl_typesupport_fastrtps_c;builtin_interfaces::builtin_interfaces__rosidl_typesupport_introspection_c;builtin_interfaces::builtin_interfaces__rosidl_typesupport_c;builtin_interfaces::builtin_interfaces__rosidl_generator_cpp;builtin_interfaces::builtin_interfaces__rosidl_typesupport_fastrtps_cpp;builtin_interfaces::builtin_interfaces__rosidl_typesupport_introspection_cpp;builtin_interfaces::builtin_interfaces__rosidl_typesupport_cpp;builtin_interfaces::builtin_interfaces__rosidl_generator_py
-- interfaces: 
-- definitions: 
-- include_dirs: /opt/ros/humble/include/builtin_interfaces;/opt/ros/humble/include/rosidl_runtime_c;/opt/ros/humble/include/rosidl_typesupport_interface;/opt/ros/humble/include/rcutils;/opt/ros/humble/include/rosidl_runtime_cpp;/opt/ros/humble/include/rosidl_typesupport_fastrtps_c;/opt/ros/humble/include/rosidl_typesupport_fastrtps_cpp;/opt/ros/humble/include/rosidl_typesupport_c;/opt/ros/humble/include/rmw;/opt/ros/humble/include/rosidl_typesupport_cpp;/opt/ros/humble/include/rcpputils;/opt/ros/humble/include/rosidl_typesupport_introspection_c;/opt/ros/humble/include/rosidl_typesupport_introspection_cpp
-- link_flags: 
-- libraries: /opt/ros/humble/lib/libbuiltin_interfaces__rosidl_generator_c.so;/opt/ros/humble/lib/libbuiltin_interfaces__rosidl_typesupport_c.so;/opt/ros/humble/lib/libbuiltin_interfaces__rosidl_typesupport_cpp.so;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;rcutils::rcutils;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rosidl_runtime_c::rosidl_runtime_c;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librosidl_typesupport_cpp.so;rosidl_runtime_c::rosidl_runtime_c;rosidl_typesupport_c::rosidl_typesupport_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librosidl_typesupport_cpp.so;rosidl_runtime_c::rosidl_runtime_c;rosidl_typesupport_c::rosidl_typesupport_c;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_c.so;/opt/ros/humble/lib/librmw.so;/opt/ros/humble/lib/librosidl_typesupport_fastrtps_cpp.so;fastcdr;rmw::rmw;rosidl_typesupport_c::rosidl_typesupport_c;/opt/ros/humble/lib/librcutils.so;dl;/opt/ros/humble/lib/librcpputils.so;/opt/ros/humble/lib/librosidl_typesupport_c.so;/opt/ros/humble/lib/librosidl_typesupport_cpp.so;/opt/ros/humble/lib/librosidl_runtime_c.so;rcutils::rcutils;rosidl_typesupport_interface::rosidl_typesupport_interface;rosidl_runtime_c::rosidl_runtime_c;/opt/ros/humble/lib/librosidl_typesupport_introspection_c.so;/opt/ros/humble/lib/librosidl_typesupport_introspection_cpp.so
-- #############################

There are 267 listed libraries, but only 20 are unique. The root cause is the way dependencies are aggregated when a package is exported by the ament_cmake_export_* macros:

https://github.com/ament/ament_cmake/blob/446e3ede3ec268bcc9f76be27b4a2a24ba6a96bb/ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in#L87-L90

For example, if I build a package myPackage which depends on packageA, packageB and packageC, and that each of these packages have dependencies on the same common libraries libX and libY, then myPackage_LIBRARIES may look something like this: myPackage_LIBRARIES="libA;libX;libY;libB;libX;libY;libC;libX;libY". Basically, if a package has N dependencies, each list library may appear up to N times in the *_LIBRARIES variable.

So one way to remove the duplicated libraries is to act on upstream packages export like so:

diff --git a/ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in b/ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in
index ab3d1e3..7628f5a 100644
--- a/ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in
+++ b/ament_cmake_export_dependencies/cmake/ament_cmake_export_dependencies-extras.cmake.in
@@ -89,4 +89,5 @@ if(NOT _exported_dependencies STREQUAL "")
       list(APPEND @PROJECT_NAME@_LIBRARIES "${_libraries}")
     endif()
   endforeach()
+  ament_libraries_deduplicate(@PROJECT_NAME@_LIBRARIES "${@PROJECT_NAME@_LIBRARIES}")
 endif()

However, I am not sure it wouldn't break some packages when linking with GNU ld (because order matters, and there may be circular dependencies). To be "safe", @PROJECT_NAME@_LIBRARIES could be artificially duplicated (1 duplicate is better than 20) to make sure this kind of issue can't happen (or use -Wl,--start-group/-Wl,--end-group or use another linker than GNU ld...)

That being said, I would argue that ament_libraries_deduplicate already breaks circular dependencies in the first place, so I don't think it would change anything here.

[clalancette]

Even ament_target_dependencies(target SYSTEM builtin_interfaces) brings a lot of duplicated libraries:

This is one of the major reasons we are (slowly) trying to move away from ament_target_dependencies; it is slower than it needs to be, and nowadays it is basically duplicate functionality with target_link_libraries. The entire ROS 2 core, for instance, no longer uses ament_target_dependencies.

That said, we are not going to deprecate/remove it anytime soon, so we can consider improving it. But since it is so heavily used, we'd have to be very certain that it won't break things.