[libcxx-commits] [PATCH] D124122: [libc++] Optimize std::rotate

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik updated this revision to Diff 424741.
philnik added a comment.

I noticed that it wasn't actually enabled for non-trivial types, so I only restricted it to types larger than 32 bytes, although I'm not sure we want to keep this in, since it's a large part of the code while while only being enabled for a very small amount of types and only making a relatively small difference performance wise. If we can find a good heuristic for enabling it for non-tivial types I would be happier to keep it in. (Although I don't think we can have a good heuristic for this kind of stuff)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124122/new/

https://reviews.llvm.org/D124122

Files:
  libcxx/benchmarks/algorithms.bench.cpp
  libcxx/include/__algorithm/rotate.h


Index: libcxx/include/__algorithm/rotate.h
===================================================================
--- libcxx/include/__algorithm/rotate.h
+++ libcxx/include/__algorithm/rotate.h
@@ -83,7 +83,6 @@
     }
     return __r;
 }
-
 template<typename _Integral>
 inline _LIBCPP_INLINE_VISIBILITY
 _LIBCPP_CONSTEXPR_AFTER_CXX14 _Integral
@@ -172,7 +171,7 @@
          random_access_iterator_tag)
 {
     typedef typename iterator_traits<_RandomAccessIterator>::value_type value_type;
-    if (is_trivially_move_assignable<value_type>::value)
+    if _LIBCPP_CONSTEXPR_AFTER_CXX14 (is_trivially_move_assignable<value_type>::value && sizeof(value_type) > 32)
     {
         if (_VSTD::next(__first) == __middle)
             return _VSTD::__rotate_left(__first, __last);
Index: libcxx/benchmarks/algorithms.bench.cpp
===================================================================
--- libcxx/benchmarks/algorithms.bench.cpp
+++ libcxx/benchmarks/algorithms.bench.cpp
@@ -14,14 +14,19 @@
 
 namespace {
 
-enum class ValueType { Uint32, Uint64, Pair, Tuple, String, Float };
-struct AllValueTypes : EnumValuesAsTuple<AllValueTypes, ValueType, 6> {
+enum class ValueType { Uint32, Uint64, Pair, Tuple, String, Float, ExpensiveToMove };
+struct AllValueTypes : EnumValuesAsTuple<AllValueTypes, ValueType, 7> {
   static constexpr const char* Names[] = {"uint32", "uint64", "pair<uint32, uint32>", "tuple<uint32, uint64, uint32>",
-                                          "string", "float"};
+                                          "string", "float", "ExpensiveToMove"};
+};
+
+struct ExpensiveToMove {
+  int a[256];
+  std::strong_ordering operator<=>(const ExpensiveToMove&) const = default;
 };
 
 using Types = std::tuple< uint32_t, uint64_t, std::pair<uint32_t, uint32_t>, std::tuple<uint32_t, uint64_t, uint32_t>,
-                          std::string, float >;
+                          std::string, float, ExpensiveToMove >;
 
 template <class V>
 using Value = std::tuple_element_t<(int)V::value, Types>;
@@ -143,6 +148,20 @@
   }
 }
 
+void fillValues(std::vector<ExpensiveToMove>& V, size_t N, Order O) {
+  if (O == Order::SingleElement) {
+    V.resize(N, ExpensiveToMove{});
+  } else {
+    while (V.size() != N) {
+      ExpensiveToMove e;
+      for (size_t i = 0; i != 256; ++i) {
+        e.a[i] = V.size() + i;
+      }
+      V.push_back(e);
+    }
+  }
+}
+
 template <class T>
 void sortValues(T& V, Order O) {
   switch (O) {
@@ -367,6 +386,22 @@
   }
 };
 
+template <class ValueType, class Order>
+struct Rotate {
+  size_t Quantity;
+  mutable std::mt19937_64 rng;
+
+  void run(benchmark::State& state) const {
+    runOpOnCopies<ValueType>(state, Quantity, Order(), BatchSize::CountBatch, [&](auto& Copy) {
+      benchmark::DoNotOptimize(std::rotate(Copy.begin(), Copy.begin() + (rng() % Copy.size()), Copy.end()));
+    });
+  }
+
+  std::string name() const {
+    return "BM_Rotate" + ValueType::name() + Order::name() + "_" + std::to_string(Quantity);
+  }
+};
+
 } // namespace
 
 int main(int argc, char** argv) {
@@ -392,5 +427,6 @@
   makeCartesianProductBenchmark<PushHeap, AllValueTypes, AllOrders>(Quantities);
   makeCartesianProductBenchmark<PopHeap, AllValueTypes>(Quantities);
   makeCartesianProductBenchmark<MinMaxElement, AllValueTypes, AllOrders>(Quantities);
+  makeCartesianProductBenchmark<Rotate, AllValueTypes, AllOrders>(Quantities);
   benchmark::RunSpecifiedBenchmarks();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124122.424741.patch
Type: text/x-patch
Size: 3444 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220423/c68f85bc/attachment.bin>