Skip to content

Commit

Permalink
catkin pkg config template: performance fix for pack/unpack/append
Browse files Browse the repository at this point in the history
These algorithms may be used O(M^2) times where M is the level of
rospackage nesting, and each pack/unpack/append algorithm complexity
are O(N^2)/O(N)/O(N^2) where N is libraries count. The fix allows
to reduce them to O(1) complexity if we only consider amount of cmake
statements executed.

Notes regarding REMOVE_DUPLICATES in list_append_unique and
list_append_deduplicate are incorrect, in case of:

- list_append_unique: REMOVE_DUPLICATES uses set/unordered_set to
  remember encountered items, all output items are written in order,
  so the algorithm is "stable", it doesn't shuffle elements

- list_append_deduplicate: REMOVE_DUPLICATES is improper here, as it
  removes duplicates from the added element list, not the added-to element
  list, not due to it's "unstability" as note suggests

3.0.2:  https://github.com/Kitware/CMake/blob/v3.0.2/Source/cmListCommand.cxx#L480-L493
3.19.6: https://github.com/Kitware/CMake/blob/v3.19.6/Source/cmAlgorithms.h#L120-L130
  • Loading branch information
aurzenligl committed Mar 12, 2021
1 parent a9672d7 commit 03b9647
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 69 deletions.
26 changes: 2 additions & 24 deletions cmake/catkin_libraries.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -71,24 +71,7 @@ endfunction()
#
function(catkin_pack_libraries_with_build_configuration VAR)
set(result "")
set(_argn ${ARGN})
list(LENGTH _argn _count)
set(_index 0)
while(${_index} LESS ${_count})
list(GET _argn ${_index} lib)
if("${lib}" MATCHES "^(debug|optimized|general)$")
math(EXPR _index "${_index} + 1")
if(${_index} EQUAL ${_count})
message(FATAL_ERROR "catkin_pack_libraries_with_build_configuration() the list of libraries '${_argn}' ends with '${lib}' which is a build configuration keyword and must be followed by a library")
endif()
list(GET _argn ${_index} library)
list(APPEND result "${lib}${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}${library}")
else()
list(APPEND result "${lib}")
endif()
math(EXPR _index "${_index} + 1")
endwhile()
#debug_message(10 "catkin_pack_libraries_with_build_configuration(${VAR} ${_argn}) ${result}")
string(REGEX REPLACE "(^|;)(debug|optimized|general);([^;]+)(;|$)" "\\1\\2${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}\\3\\4" result "${ARGN}")
set(${VAR} "${result}" PARENT_SCOPE)
endfunction()

Expand All @@ -105,12 +88,7 @@ endfunction()
#
function(catkin_unpack_libraries_with_build_configuration VAR)
set(result "")
foreach(lib ${ARGN})
string(REGEX REPLACE "^(debug|optimized|general)${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}(.+)$" "\\1;\\2" lib "${lib}")
list(APPEND result "${lib}")
endforeach()
#set(_argn ${ARGN})
#debug_message(10 "catkin_unpack_libraries_with_build_configuration(${VAR} ${_argn}) ${result}")
string(REGEX REPLACE "(debug|optimized|general)${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}([^;]+)" "\\1;\\2" result "${ARGN}")
set(${VAR} "${result}" PARENT_SCOPE)
endfunction()

Expand Down
5 changes: 0 additions & 5 deletions cmake/list_append_deduplicate.cmake
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
#
# Append elements to a list and remove existing duplicates from the list.
#
# .. note:: Using CMake's ``list(APPEND ..)`` and
# ``list(REMOVE_DUPLICATES ..)`` is not sufficient since its
# implementation uses a set internally which makes the operation
# unstable.
#
macro(list_append_deduplicate listname)
if(NOT "${ARGN}" STREQUAL "")
if(${listname})
Expand Down
15 changes: 4 additions & 11 deletions cmake/list_append_unique.cmake
Original file line number Diff line number Diff line change
@@ -1,16 +1,9 @@
#
# Append elements to a list if they are not already in the list.
#
# .. note:: Using CMake's ``list(APPEND ..)`` and
# ``list(REMOVE_DUPLICATES ..)`` is not sufficient since its
# implementation uses a set internally which makes the operation
# unstable.
#
macro(list_append_unique listname)
foreach(_item ${ARGN})
list(FIND ${listname} ${_item} _index)
if(_index EQUAL -1)
list(APPEND ${listname} ${_item})
endif()
endforeach()
if(NOT "${ARGN}" STREQUAL "")
list(APPEND ${listname} ${ARGN})
list(REMOVE_DUPLICATES ${listname})
endif()
endmacro()
35 changes: 6 additions & 29 deletions cmake/templates/pkgConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,47 +16,24 @@ endmacro()
# copied from catkin/cmake/list_append_unique.cmake to keep pkgConfig
# self contained
macro(_list_append_unique listname)
foreach(_item ${ARGN})
list(FIND ${listname} ${_item} _index)
if(_index EQUAL -1)
list(APPEND ${listname} ${_item})
endif()
endforeach()
if(NOT "${ARGN}" STREQUAL "")
list(APPEND ${listname} ${ARGN})
list(REMOVE_DUPLICATES ${listname})
endif()
endmacro()

# pack a list of libraries with optional build configuration keywords
# copied from catkin/cmake/catkin_libraries.cmake to keep pkgConfig
# self contained
macro(_pack_libraries_with_build_configuration VAR)
set(${VAR} "")
set(_argn ${ARGN})
list(LENGTH _argn _count)
set(_index 0)
while(${_index} LESS ${_count})
list(GET _argn ${_index} lib)
if("${lib}" MATCHES "^(debug|optimized|general)$")
math(EXPR _index "${_index} + 1")
if(${_index} EQUAL ${_count})
message(FATAL_ERROR "_pack_libraries_with_build_configuration() the list of libraries '${ARGN}' ends with '${lib}' which is a build configuration keyword and must be followed by a library")
endif()
list(GET _argn ${_index} library)
list(APPEND ${VAR} "${lib}${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}${library}")
else()
list(APPEND ${VAR} "${lib}")
endif()
math(EXPR _index "${_index} + 1")
endwhile()
string(REGEX REPLACE "(^|;)(debug|optimized|general);([^;]+)(;|$)" "\\1\\2${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}\\3\\4" ${VAR} "${ARGN}")
endmacro()

# unpack a list of libraries with optional build configuration keyword prefixes
# copied from catkin/cmake/catkin_libraries.cmake to keep pkgConfig
# self contained
macro(_unpack_libraries_with_build_configuration VAR)
set(${VAR} "")
foreach(lib ${ARGN})
string(REGEX REPLACE "^(debug|optimized|general)${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}(.+)$" "\\1;\\2" lib "${lib}")
list(APPEND ${VAR} "${lib}")
endforeach()
string(REGEX REPLACE "(debug|optimized|general)${CATKIN_BUILD_CONFIGURATION_KEYWORD_SEPARATOR}([^;]+)" "\\1;\\2" ${VAR} "${ARGN}")
endmacro()


Expand Down

0 comments on commit 03b9647

Please sign in to comment.