perf: deep packages are swarmed with duplicated libraries in ament_target_dependencies()
#541
Labels
Comments
|
A quick and dirty 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()
|
|
Even 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 For example, if I build a package 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", That being said, I would argue that |
This is one of the major reasons we are (slowly) trying to move away from 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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #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:
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_dependenciesloops 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?The text was updated successfully, but these errors were encountered: