Browse Source

cmList: Fix performance regression in append/prepend

Fixes: #25147
Marc Chevrier 2 years ago
parent
commit
a9a34edc82
3 changed files with 36 additions and 26 deletions
  1. 12 4
      Source/cmList.cxx
  2. 20 18
      Source/cmList.h
  3. 4 4
      Tests/CMakeLib/testList.cxx

+ 12 - 4
Source/cmList.cxx

@@ -803,27 +803,35 @@ cmList& cmList::transform(TransformAction action,
   return *this;
 }
 
-std::string& cmList::append(std::string& list, cm::string_view value)
+std::string& cmList::append(std::string& list, std::string&& value)
 {
   if (list.empty()) {
-    list = std::string(value);
+    list = std::move(value);
   } else {
     list += cmStrCat(cmList::element_separator, value);
   }
 
   return list;
 }
+std::string& cmList::append(std::string& list, cm::string_view value)
+{
+  return cmList::append(list, std::string{ value });
+}
 
-std::string& cmList::prepend(std::string& list, cm::string_view value)
+std::string& cmList::prepend(std::string& list, std::string&& value)
 {
   if (list.empty()) {
-    list = std::string(value);
+    list = std::move(value);
   } else {
     list.insert(0, cmStrCat(value, cmList::element_separator));
   }
 
   return list;
 }
+std::string& cmList::prepend(std::string& list, cm::string_view value)
+{
+  return cmList::prepend(list, std::string{ value });
+}
 
 cmList::size_type cmList::ComputeIndex(index_type pos, bool boundCheck) const
 {

+ 20 - 18
Source/cmList.h

@@ -10,7 +10,6 @@
 #include <initializer_list>
 #include <iterator>
 #include <memory>
-#include <numeric>
 #include <stdexcept>
 #include <string>
 #include <utility>
@@ -1086,6 +1085,7 @@ public:
   // but without any intermediate expansion. So the operation is simply a
   // string concatenation with special handling for the CMake list item
   // separator
+  static std::string& append(std::string& list, std::string&& value);
   static std::string& append(std::string& list, cm::string_view value);
   template <typename InputIterator>
   static std::string& append(std::string& list, InputIterator first,
@@ -1095,15 +1095,11 @@ public:
       return list;
     }
 
-    return cmList::append(list,
-                          cm::string_view{ std::accumulate(
-                            std::next(first), last, *first,
-                            [](const std::string& a, const std::string& b) {
-                              return a +
-                                std::string(cmList::element_separator) + b;
-                            }) });
+    return cmList::append(
+      list, cmList::Join(first, last, cmList::element_separator));
   }
 
+  static std::string& prepend(std::string& list, std::string&& value);
   static std::string& prepend(std::string& list, cm::string_view value);
   template <typename InputIterator>
   static std::string& prepend(std::string& list, InputIterator first,
@@ -1113,13 +1109,8 @@ public:
       return list;
     }
 
-    return cmList::prepend(list,
-                           cm::string_view{ std::accumulate(
-                             std::next(first), last, *first,
-                             [](std::string a, const std::string& b) {
-                               return std::move(a) +
-                                 std::string(cmList::element_separator) + b;
-                             }) });
+    return cmList::prepend(
+      list, cmList::Join(first, last, cmList::element_separator));
   }
 
   template <typename Range,
@@ -1209,11 +1200,22 @@ private:
       return std::string{};
     }
 
+    return cmList::Join(std::begin(r), std::end(r), glue);
+  }
+  template <typename InputIterator>
+  static std::string Join(InputIterator first, InputIterator last,
+                          cm::string_view glue)
+  {
+    if (first == last) {
+      return std::string{};
+    }
+
     const auto sep = std::string{ glue };
 
-    std::string joined = cmList::ToString(*std::begin(r));
-    for (auto it = std::next(std::begin(r)); it != std::end(r); ++it) {
-      joined += sep + cmList::ToString(*it);
+    std::string joined = cmList::ToString(*first);
+    for (auto it = std::next(first); it != last; ++it) {
+      joined += sep;
+      joined += cmList::ToString(*it);
     }
 
     return joined;

+ 4 - 4
Tests/CMakeLib/testList.cxx

@@ -858,7 +858,7 @@ bool testStaticModifiers()
   }
   {
     std::string list{ "a;b;c" };
-    cmList::append(list, "");
+    cmList::append(list, ""_s);
 
     if (list != "a;b;c;") {
       result = false;
@@ -893,7 +893,7 @@ bool testStaticModifiers()
   }
   {
     std::string list{ "a;b;c" };
-    cmList::prepend(list, "d;e");
+    cmList::prepend(list, "d;e"_s);
 
     if (list != "d;e;a;b;c") {
       result = false;
@@ -901,7 +901,7 @@ bool testStaticModifiers()
   }
   {
     std::string list;
-    cmList::prepend(list, "d;e");
+    cmList::prepend(list, "d;e"_s);
 
     if (list != "d;e") {
       result = false;
@@ -909,7 +909,7 @@ bool testStaticModifiers()
   }
   {
     std::string list{ "a;b;c" };
-    cmList::prepend(list, "");
+    cmList::prepend(list, ""_s);
 
     if (list != ";a;b;c") {
       result = false;