ament_auto macros are too slow?
#442
Comments
|
I had a lot of fun with cmake today! To answer my own question: it is "not that difficult" to use modern targets instead of the old style Here is what I have done:
$ sudo dpkg -r --force-depends ros-humble-ament-cmake-auto
diff --git a/ament_cmake/COLCON_IGNORE b/ament_cmake/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_auto/cmake/ament_auto_add_library.cmake b/ament_cmake_auto/cmake/ament_auto_add_library.cmake
index f01ac9d..f999b83 100644
--- a/ament_cmake_auto/cmake/ament_auto_add_library.cmake
+++ b/ament_cmake_auto/cmake/ament_auto_add_library.cmake
@@ -67,8 +67,11 @@ macro(ament_auto_add_library target)
# add include directory of this package if it exists
if(EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/include")
+ # target_include_directories("${target}" PUBLIC
+ # "${CMAKE_CURRENT_SOURCE_DIR}/include")
target_include_directories("${target}" PUBLIC
- "${CMAKE_CURRENT_SOURCE_DIR}/include")
+ $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
+ $<INSTALL_INTERFACE:include>)
endif()
# link against other libraries of this package
if(NOT ${PROJECT_NAME}_LIBRARIES STREQUAL "" AND
diff --git a/ament_cmake_auto/cmake/ament_auto_find_build_dependencies.cmake b/ament_cmake_auto/cmake/ament_auto_find_build_dependencies.cmake
index 956a09f..e88f61c 100644
--- a/ament_cmake_auto/cmake/ament_auto_find_build_dependencies.cmake
+++ b/ament_cmake_auto/cmake/ament_auto_find_build_dependencies.cmake
@@ -67,10 +67,17 @@ macro(ament_auto_find_build_dependencies)
find_package(${_dep} QUIET ${_REQUIRED_KEYWORD})
if(${_dep}_FOUND)
list(APPEND ${PROJECT_NAME}_FOUND_BUILD_DEPENDS ${_dep})
-
- list(APPEND ${PROJECT_NAME}_FOUND_DEFINITIONS ${${_dep}_DEFINITIONS})
- list(APPEND ${PROJECT_NAME}_FOUND_INCLUDE_DIRS ${${_dep}_INCLUDE_DIRS})
- list(APPEND ${PROJECT_NAME}_FOUND_LIBRARIES ${${_dep}_LIBRARIES})
+
+ # if a package provides modern CMake interface targets use them
+ # exclusively assuming the classic CMake variables only exist for
+ # backward compatibility
+ if(NOT "${${_dep}_TARGETS}" STREQUAL "")
+ list(APPEND ${PROJECT_NAME}_FOUND_TARGETS ${${_dep}_TARGETS})
+ else()
+ list(APPEND ${PROJECT_NAME}_FOUND_DEFINITIONS ${${_dep}_DEFINITIONS})
+ list(APPEND ${PROJECT_NAME}_FOUND_INCLUDE_DIRS ${${_dep}_INCLUDE_DIRS})
+ list(APPEND ${PROJECT_NAME}_FOUND_LIBRARIES ${${_dep}_LIBRARIES})
+ endif()
endif()
endforeach()
diff --git a/ament_cmake_auto/cmake/ament_auto_package.cmake b/ament_cmake_auto/cmake/ament_auto_package.cmake
index 3c6c7bb..a69ce90 100644
--- a/ament_cmake_auto/cmake/ament_auto_package.cmake
+++ b/ament_cmake_auto/cmake/ament_auto_package.cmake
@@ -66,14 +66,16 @@ macro(ament_auto_package)
endif()
# export and install all libraries
- if(NOT ${PROJECT_NAME}_LIBRARIES STREQUAL "")
+ if(NOT "${${PROJECT_NAME}_LIBRARIES}" STREQUAL "")
ament_export_libraries(${${PROJECT_NAME}_LIBRARIES})
install(
TARGETS ${${PROJECT_NAME}_LIBRARIES}
+ EXPORT ${PROJECT_NAME}Targets
ARCHIVE DESTINATION lib
LIBRARY DESTINATION lib
RUNTIME DESTINATION bin
)
+ ament_export_targets(${PROJECT_NAME}Targets)
endif()
# install all executables
diff --git a/ament_cmake_core/COLCON_IGNORE b/ament_cmake_core/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_definitions/COLCON_IGNORE b/ament_cmake_export_definitions/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_dependencies/COLCON_IGNORE b/ament_cmake_export_dependencies/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_include_directories/COLCON_IGNORE b/ament_cmake_export_include_directories/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_interfaces/COLCON_IGNORE b/ament_cmake_export_interfaces/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_libraries/COLCON_IGNORE b/ament_cmake_export_libraries/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_link_flags/COLCON_IGNORE b/ament_cmake_export_link_flags/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_export_targets/COLCON_IGNORE b/ament_cmake_export_targets/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_gen_version_h/COLCON_IGNORE b/ament_cmake_gen_version_h/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_gmock/COLCON_IGNORE b/ament_cmake_gmock/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_google_benchmark/COLCON_IGNORE b/ament_cmake_google_benchmark/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_gtest/COLCON_IGNORE b/ament_cmake_gtest/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_include_directories/COLCON_IGNORE b/ament_cmake_include_directories/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_libraries/COLCON_IGNORE b/ament_cmake_libraries/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_nose/COLCON_IGNORE b/ament_cmake_nose/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_pytest/COLCON_IGNORE b/ament_cmake_pytest/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_python/COLCON_IGNORE b/ament_cmake_python/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_target_dependencies/COLCON_IGNORE b/ament_cmake_target_dependencies/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_test/COLCON_IGNORE b/ament_cmake_test/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29
diff --git a/ament_cmake_version/COLCON_IGNORE b/ament_cmake_version/COLCON_IGNORE
new file mode 100644
index 0000000..e69de29Basically, I make sure I ignore all packages but
=== src/core/autoware.core (git) ===
=== src/core/autoware_adapi_msgs (git) ===
=== src/core/autoware_common (git) ===
=== src/core/autoware_msgs (git) ===
=== src/core/external/autoware_auto_msgs (git) ===
=== src/launcher/autoware_launch (git) ===
=== src/param/autoware_individual_params (git) ===
=== src/sensor_component/external/sensor_component_description (git) ===
=== src/sensor_component/external/tamagawa_imu_driver (git) ===
=== src/sensor_component/external/velodyne_vls (git) ===
diff --git a/velodyne_driver/CMakeLists.txt b/velodyne_driver/CMakeLists.txt
index 955acb9..a12071b 100644
--- a/velodyne_driver/CMakeLists.txt
+++ b/velodyne_driver/CMakeLists.txt
@@ -24,6 +24,11 @@ include_directories(include)
add_subdirectory(src/lib)
# add_subdirectory(src/driver)
+# NOTE(VRichardJP) workaround for not using ament_auto_add_library
+# This triggers target export behavior in ament_auto_package()
+# see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+list(APPEND ${PROJECT_NAME}_LIBRARIES velodyne_input)
+
ament_auto_add_library(velodyne_driver SHARED
src/driver/driver.cc
src/driver/driver.h
=== src/sensor_kit/external/awsim_sensor_kit_launch (git) ===
=== src/sensor_kit/sample_sensor_kit_launch (git) ===
=== src/tmp/ament_cmake (git) ===
=== src/universe/autoware.universe (git) ===
diff --git a/map/map_height_fitter/CMakeLists.txt b/map/map_height_fitter/CMakeLists.txt
index ddab6db192..83c5f19c7b 100644
--- a/map/map_height_fitter/CMakeLists.txt
+++ b/map/map_height_fitter/CMakeLists.txt
@@ -2,9 +2,12 @@ cmake_minimum_required(VERSION 3.14)
project(map_height_fitter)
find_package(autoware_cmake REQUIRED)
-find_package(PCL REQUIRED COMPONENTS common)
autoware_package()
+find_package(PCL REQUIRED COMPONENTS common)
+# NOTE(VRichardJP) fix unmet Boost::* dependencies in downstream packages
+ament_export_dependencies(PCL)
+
ament_auto_add_library(map_height_fitter SHARED
src/map_height_fitter.cpp
)
diff --git a/perception/image_projection_based_fusion/CMakeLists.txt b/perception/image_projection_based_fusion/CMakeLists.txt
index 29cc087ed3..7c86551309 100644
--- a/perception/image_projection_based_fusion/CMakeLists.txt
+++ b/perception/image_projection_based_fusion/CMakeLists.txt
@@ -159,6 +159,10 @@ if(TRT_AVAIL AND CUDA_AVAIL AND CUDNN_AVAIL)
cuda_add_library(pointpainting_cuda_lib SHARED
src/pointpainting_fusion/preprocess_kernel.cu
)
+ # NOTE(VRichardJP) workaround for not using ament_auto_add_library
+ # This triggers target export behavior in ament_auto_package()
+ # see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+ list(APPEND ${PROJECT_NAME}_LIBRARIES pointpainting_cuda_lib)
target_link_libraries(pointpainting_lib
${OpenCV_LIBRARIES}
diff --git a/perception/lidar_centerpoint/CMakeLists.txt b/perception/lidar_centerpoint/CMakeLists.txt
index f2935ae98a..8cafed4d51 100644
--- a/perception/lidar_centerpoint/CMakeLists.txt
+++ b/perception/lidar_centerpoint/CMakeLists.txt
@@ -144,6 +144,10 @@ if(TRT_AVAIL AND CUDA_AVAIL AND CUDNN_AVAIL)
lib/network/scatter_kernel.cu
lib/preprocess/preprocess_kernel.cu
)
+ # NOTE(VRichardJP) workaround for not using ament_auto_add_library
+ # This triggers target export behavior in ament_auto_package()
+ # see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+ list(APPEND ${PROJECT_NAME}_LIBRARIES centerpoint_cuda_lib)
target_link_libraries(centerpoint_lib
${NVINFER}
diff --git a/perception/tensorrt_yolo/CMakeLists.txt b/perception/tensorrt_yolo/CMakeLists.txt
index f97a65834f..443880e219 100755
--- a/perception/tensorrt_yolo/CMakeLists.txt
+++ b/perception/tensorrt_yolo/CMakeLists.txt
@@ -136,23 +136,36 @@ if(TRT_AVAIL AND CUDA_AVAIL AND CUDNN_AVAIL)
lib/src/plugins/mish.cu
lib/src/plugins/mish_plugin.cpp
)
+ # NOTE(VRichardJP) workaround for not using ament_auto_add_library
+ # This triggers target export behavior in ament_auto_package()
+ # see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+ list(APPEND ${PROJECT_NAME}_LIBRARIES mish_plugin)
cuda_add_library(yolo_layer_plugin SHARED
lib/src/plugins/yolo_layer.cu
lib/src/plugins/yolo_layer_plugin.cpp
)
+ # NOTE(VRichardJP) workaround for not using ament_auto_add_library
+ # This triggers target export behavior in ament_auto_package()
+ # see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+ list(APPEND ${PROJECT_NAME}_LIBRARIES yolo_layer_plugin)
cuda_add_library(nms_plugin SHARED
lib/src/plugins/nms.cu
lib/src/plugins/nms_plugin.cpp
)
+ # NOTE(VRichardJP) workaround for not using ament_auto_add_library
+ # This triggers target export behavior in ament_auto_package()
+ # see https://github.com/VRichardJP/ament_cmake/blob/9c0e148ac79bb38f9ab16365813a685f7e93c2dc/ament_cmake_auto/cmake/ament_auto_package.cmake#L69
+ list(APPEND ${PROJECT_NAME}_LIBRARIES nms_plugin)
ament_auto_add_library(yolo SHARED
lib/src/trt_yolo.cpp
)
target_include_directories(yolo PUBLIC
- lib/include
+ $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lib/include>
+ $<INSTALL_INTERFACE:lib/include>
)
target_link_libraries(yolo
diff --git a/perception/traffic_light_ssd_fine_detector/CMakeLists.txt b/perception/traffic_light_ssd_fine_detector/CMakeLists.txt
index 8e91e36fdd..42097546a3 100644
--- a/perception/traffic_light_ssd_fine_detector/CMakeLists.txt
+++ b/perception/traffic_light_ssd_fine_detector/CMakeLists.txt
@@ -124,7 +124,8 @@ if(TRT_AVAIL AND CUDA_AVAIL AND CUDNN_AVAIL)
)
target_include_directories(ssd PUBLIC
- lib/include
+ $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/lib/include>
+ $<INSTALL_INTERFACE:lib/include>
)
target_link_libraries(ssd
diff --git a/planning/costmap_generator/CMakeLists.txt b/planning/costmap_generator/CMakeLists.txt
index 501e0c27df..30b632cd40 100644
--- a/planning/costmap_generator/CMakeLists.txt
+++ b/planning/costmap_generator/CMakeLists.txt
@@ -7,6 +7,9 @@ autoware_package()
find_package(PCL REQUIRED COMPONENTS common io)
find_package(FLANN REQUIRED)
+ament_export_dependencies(PCL)
+ament_export_dependencies(FLANN)
+
include_directories(
include
SYSTEM
@@ -27,6 +30,7 @@ target_link_libraries(costmap_generator_lib
if(${PCL_VERSION} GREATER_EQUAL 1.12.1)
find_package(Qhull REQUIRED)
+ ament_export_dependencies(Qhull)
target_link_libraries(costmap_generator_lib
QHULL::QHULL
)
=== src/universe/external/morai_msgs (git) ===
=== src/universe/external/muSSP (git) ===
diff --git a/CMakeLists.txt b/CMakeLists.txt
index eb445e6..b0399f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -43,7 +43,12 @@ set(MUSSP_HEADERS
ament_auto_add_library(mussp SHARED
${MUSSP_SOURCES}
- ${MUSSP_HEADERS}
+ # ${MUSSP_HEADERS}
+)
+
+# NOTE(VRichardJP) so that downstream nodes can find headers when using mussp::mussp target
+target_include_directories(mussp PUBLIC
+ $<INSTALL_INTERFACE:include>
)
ament_export_include_directories(include)
=== src/universe/external/ndt_omp (git) ===
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 13af01e..34fe437 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -97,6 +97,7 @@ find_package(ament_cmake_auto REQUIRED)
ament_auto_find_build_dependencies()
find_package(PCL 1.7 REQUIRED)
+ament_export_dependencies(PCL)
include_directories(${PCL_INCLUDE_DIRS})
link_directories(${PCL_LIBRARY_DIRS})
add_definitions(${PCL_DEFINITIONS})
@@ -106,6 +107,7 @@ message(STATUS "PCL_LIBRARY_DIRS:" ${PCL_LIBRARY_DIRS})
message(STATUS "PCL_DEFINITIONS:" ${PCL_DEFINITIONS})
find_package(OpenMP)
+ament_export_dependencies(OpenMP)
###########
## Build ##
=== src/universe/external/pointcloud_to_laserscan (git) ===
=== src/universe/external/tier4_ad_api_adaptor (git) ===
=== src/universe/external/tier4_autoware_msgs (git) ===
=== src/vehicle/external/pacmod_interface (git) ===
=== src/vehicle/sample_vehicle_launch (git) ===
$ colcon build --symlink-install --event-handlers=console_cohesion+ --cmake-args -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_TESTING=False -DCMAKE_CXX_FLAGS='-fdiagnostics-color' -GNinja --profiling-format=google-trace --profiling-output=cmake_profile.jsonI have not benchmarked a build from scratch yet, but I am pretty happy at what I see so far:
The
As you can see with the flamegraph, my changes do not adress the recursive Anyway, there is some work both on ament and user side to make it work, but I think it is definitely worth it! I guess such breaking chance has no change to land on humble branch, but could it be added as new feature for the next ROS2 release? For example, there could be a new option |
Well the bad news is that feature freeze for Iron was yesterday, so this is unlikely to get to a point that it's ready for that. The good news is that we have a lot of time to get it merged into rolling and tested before the J-turtle release next year. |
|
To complete my previous benchmarks, I have benchmarked a full autoware build (
Overall the performance gain is not as big as for single package build. I think there are a few reasons for that:
|
3 tasks
This was referenced May 5, 2023
3 tasks
|
@mjcarroll |
|
Yes, please go ahead and target |
This was referenced May 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Diclaimer: I am nothing but a user of Autoware, I do not represent the Autoware Project nor the Autoware Foundation in any way.
Let me first bring a bit of context:
The Autoware project is a collection of ROS2 packages for autonomous vehicles. As of today, a "base" Autoware source tree contains around 250 packages. Building Autoware from scratch takes quite some time (e.g. ~30 minutes on my dev machine), but this is not all because there are a lot of files to compile. Depending on the package, cmake invocation represents from 20% to 40% of the build time. Autoware heavily use the
ament_automacros (because they are great!), but I suspect some are slower than they could.Most Autoware packages
CMakeLists.txtlook like this:autoware_package()(a common wrapper toament_auto_find_build_dependencies())ament_auto_add_library(...)ament_auto_package()The
route_handlerpackage is a great example:This package is "not that deep" in Autoware (i.e. "not so many dependencies"). On my machine, the package is built in 53 seconds, of which 22.5 seconds (40%) are spent in cmake. With the 2 flags
-profiling-format=google-trace --profiling-output=cmake_profile.json, I can get more information about where cmake spends most of its time:As you can see, there are 2 big chunks:
ament_auto_find_build_dependenciesandament_auto_add_library.ament_auto_find_build_dependencieshas to callfind_packagerecursively on all dependencies, so I am not surprised it takes quite some time. Howeverament_auto_add_libraryis surprisingly long. In particular, the macroament_libraries_pack_build_configurationloops over many (too many?) duplicated arguments:Even if each loop iteration is rather "fast" (1~2 ms), it becomes significant when there are 5299 listed dependencies.
This package is far from being the worst. For example, the
behavior_path_planneris one of the deepest package in Autoware. On my machine, the package is built in 385 seconds, of which 112 seconds (30%) are from cmake. This is what the cmake flamegraph looks like on this package:Again, there are 2 big chunks:
ament_auto_find_build_dependenciesandament_auto_add_library. Theament_auto_find_build_dependenciespart (31s) is slightly longer than for theroute_handlerpackage, but it is not surprising becausebehavior_path_plannerhas many more dependencies. However theament_auto_add_librarypart absolutely skyrocketed and now takes 81 seconds.And so are the arguments supplied to
ament_auto_find_build_dependencies:I am no cmake expert, but I have the feeling this is way too much time for what the
ament_auto_add_librarymacro is supposed to do. For example, if I rewrite the packagesCMakeLists.txtand callfind_packageon each dependency instead of usingament_auto_find_build_dependencies, the package does not build any faster. So there is basically no cost for using theament_auto_find_build_dependenciesmacro (great!). However, if I replace theament_auto_add_librarymacro with hand-writtenadd_library(),target_include_directories()andtarget_link_libraries(), the package build time suddenly drops. The 3 builtin cmake macros simply take no time. Of courseament_auto_add_librarydoes a bit more than that and no one would ever want to do it by hands, but I still find it is "too much" time for the task.Is it something that could be addressed with the current design? or would it be necessary to rewrite some of the
ament_auto_*macros, for example to use modern cmake target import/export? (would target import be faster?)The text was updated successfully, but these errors were encountered: