Fix review comments

winner245 · winner245 · commit 438e43dd9f15 · 2025-05-22T17:50:16.000-04:00
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
@@ -72,9 +72,14 @@ Improvements and New Features
   in C++23 and later.
 =======
 - The ``std::ranges::for_each`` and ``std::ranges::for_each_n`` algorithms have been optimized for segmented iterators,
+<<<<<<< HEAD
   resulting in performance improvements of up to 21.3x for ``std::deque::iterator`` segmented inputs and 24.9x for
   ``join_view`` of ``vector<vector<T>>``.
 >>>>>>> 50ac206d4a13 (Apply optimization for join_view segmented iterators)
+=======
+  resulting in performance improvements of up to 21.3x for ``std::deque::iterator`` and 24.9x for ``join_view`` of
+  ``vector<vector<char>>``.
+>>>>>>> 590136ba0d9f (Fix review comments)
 
 - The ``std::for_each_n`` algorithm has been optimized for segmented iterators, resulting in a performance improvement of
   up to 17.7x for ``std::deque<short>`` iterators, and up to 13.9x for ``std::join_view<vector<vector<short>>>`` iterators.
diff --git a/libcxx/include/__algorithm/for_each.h b/libcxx/include/__algorithm/for_each.h
@@ -12,6 +12,8 @@
 
 #include <__algorithm/for_each_segment.h>
 #include <__config>
+#include <__functional/identity.h>
+#include <__functional/invoke.h>
 #include <__iterator/segmented_iterator.h>
 #include <__type_traits/enable_if.h>
 
@@ -21,29 +23,33 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-template <class _InputIterator, class _Sent, class _Func>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __for_each(_InputIterator __first, _Sent __last, _Func& __f) {
+template <class _InputIterator, class _Sent, class _Func, class _Proj>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _InputIterator
+__for_each(_InputIterator __first, _Sent __last, _Func& __f, _Proj& __proj) {
   for (; __first != __last; ++__first)
-    __f(*__first);
+    std::invoke(__f, std::invoke(__proj, *__first));
+  return __first;
 }
 
 #ifndef _LIBCPP_CXX03_LANG
 template <class _SegmentedIterator,
           class _Function,
+          class _Proj,
           __enable_if_t<__is_segmented_iterator<_SegmentedIterator>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-__for_each(_SegmentedIterator __first, _SegmentedIterator __last, _Function& __func) {
+__for_each(_SegmentedIterator __first, _SegmentedIterator __last, _Function& __func, _Proj& __proj) {
   using __local_iterator_t = typename __segmented_iterator_traits<_SegmentedIterator>::__local_iterator;
   std::__for_each_segment(__first, __last, [&](__local_iterator_t __lfirst, __local_iterator_t __llast) {
-    std::__for_each(__lfirst, __llast, __func);
+    std::__for_each(__lfirst, __llast, __func, __proj);
   });
 }
 #endif // !_LIBCPP_CXX03_LANG
 
 template <class _InputIterator, class _Function>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _Function
 for_each(_InputIterator __first, _InputIterator __last, _Function __f) {
-  std::__for_each(__first, __last, __f);
+  __identity __proj;
+  std::__for_each(__first, __last, __f, __proj);
   return __f;
 }
 
diff --git a/libcxx/include/__algorithm/for_each_n.h b/libcxx/include/__algorithm/for_each_n.h
@@ -13,8 +13,9 @@
 #include <__algorithm/for_each.h>
 #include <__algorithm/for_each_n_segment.h>
 #include <__config>
+#include <__functional/identity.h>
+#include <__functional/invoke.h>
 #include <__iterator/iterator_traits.h>
-#include <__iterator/next.h>
 #include <__iterator/segmented_iterator.h>
 #include <__type_traits/disjunction.h>
 #include <__type_traits/enable_if.h>
@@ -34,16 +35,17 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 template <class _InputIterator,
           class _Size,
           class _Func,
+          class _Proj,
           __enable_if_t<!__has_random_access_iterator_category<_InputIterator>::value &&
                             _Or< _Not<__is_segmented_iterator<_InputIterator> >,
                                  _Not<__has_random_access_local_iterator<_InputIterator> > >::value,
                         int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _InputIterator
-__for_each_n(_InputIterator __first, _Size __orig_n, _Func& __f) {
+__for_each_n(_InputIterator __first, _Size __orig_n, _Func& __f, _Proj& __proj) {
   typedef decltype(std::__convert_to_integral(__orig_n)) _IntegralSize;
   _IntegralSize __n = __orig_n;
   while (__n > 0) {
-    __f(*__first);
+    std::invoke(__f, std::invoke(__proj, *__first));
     ++__first;
     --__n;
   }
@@ -53,29 +55,31 @@ __for_each_n(_InputIterator __first, _Size __orig_n, _Func& __f) {
 template <class _RandIter,
           class _Size,
           class _Func,
+          class _Proj,
           __enable_if_t<__has_random_access_iterator_category<_RandIter>::value, int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _RandIter
 __for_each_n(_RandIter __first, _Size __orig_n, _Func& __f) {
   typename std::iterator_traits<_RandIter>::difference_type __n = __orig_n;
   auto __last                                                   = __first + __n;
-  std::__for_each(__first, __last, __f);
+  std::__for_each(__first, __last, __f, __proj);
   return std::move(__last);
 }
 
 #ifndef _LIBCPP_CXX03_LANG
 template <class _SegmentedIterator,
           class _Size,
           class _Func,
+          class _Proj,
           __enable_if_t<!__has_random_access_iterator_category<_SegmentedIterator>::value &&
                             __is_segmented_iterator<_SegmentedIterator>::value &&
                             __has_random_access_iterator_category<
                                 typename __segmented_iterator_traits<_SegmentedIterator>::__local_iterator>::value,
                         int> = 0>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _SegmentedIterator
-__for_each_n(_SegmentedIterator __first, _Size __orig_n, _Func& __f) {
+__for_each_n(_SegmentedIterator __first, _Size __orig_n, _Func& __f, _Proj& __proj) {
   using __local_iterator_t = typename __segmented_iterator_traits<_SegmentedIterator>::__local_iterator;
   return std::__for_each_n_segment(__first, __orig_n, [&](__local_iterator_t __lfirst, __local_iterator_t __llast) {
-    std::__for_each(__lfirst, __llast, __f);
+    std::__for_each(__lfirst, __llast, __f, __proj);
   });
 }
 #endif // !_LIBCPP_CXX03_LANG
@@ -85,7 +89,8 @@ __for_each_n(_SegmentedIterator __first, _Size __orig_n, _Func& __f) {
 template <class _InputIterator, class _Size, class _Function>
 inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _InputIterator
 for_each_n(_InputIterator __first, _Size __orig_n, _Function __f) {
-  return std::__for_each_n(__first, __orig_n, __f);
+  __identity __proj;
+  return std::__for_each_n(__first, __orig_n, __f, __proj);
 }
 
 #endif // _LIBCPP_STD_VER >= 17
diff --git a/libcxx/include/__algorithm/for_each_n_segment.h b/libcxx/include/__algorithm/for_each_n_segment.h
@@ -10,7 +10,13 @@
 #define _LIBCPP___ALGORITHM_FOR_EACH_N_SEGMENT_H
 
 #include <__config>
+<<<<<<< HEAD
 #include <__iterator/iterator_traits.h>
+=======
+#include <__iterator/distance.h>
+#include <__iterator/iterator_traits.h>
+#include <__iterator/next.h>
+>>>>>>> 4a86118918e8 (Fix review comments)
 #include <__iterator/segmented_iterator.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
diff --git a/libcxx/include/__algorithm/ranges_for_each.h b/libcxx/include/__algorithm/ranges_for_each.h
@@ -44,19 +44,13 @@ struct __for_each {
   template <class _Iter, class _Sent, class _Proj, class _Func>
   _LIBCPP_HIDE_FROM_ABI constexpr static for_each_result<_Iter, _Func>
   __for_each_impl(_Iter __first, _Sent __last, _Func& __func, _Proj& __proj) {
-    if constexpr (std::assignable_from<_Iter&, _Sent>) {
-      _Iter __end = std::move(__last);
-      std::for_each(__first, __end, [&](auto&& __val) { std::invoke(__func, std::invoke(__proj, __val)); });
-      return {std::move(__end), std::move(__func)};
-    } else if constexpr (sized_sentinel_for<_Sent, _Iter>) {
-      auto __end = std::for_each_n(__first, __last - __first, [&](auto&& __val) {
-        std::invoke(__func, std::invoke(__proj, __val));
-      });
+    if constexpr (!std::assignable_from<_Iter&, _Sent> && sized_sentinel_for<_Sent, _Iter>) {
+      auto __n   = __last - __first;
+      auto __end = std::__for_each_n(std::move(__first), __n, __func, __proj);
       return {std::move(__end), std::move(__func)};
     } else {
-      for (; __first != __last; ++__first)
-        std::invoke(__func, std::invoke(__proj, *__first));
-      return {std::move(__first), std::move(__func)};
+      auto __end = std::__for_each(std::move(__first), std::move(__last), __func, __proj);
+      return {std::move(__end), std::move(__func)};
     }
   }
 
diff --git a/libcxx/include/__algorithm/ranges_for_each_n.h b/libcxx/include/__algorithm/ranges_for_each_n.h
@@ -17,7 +17,6 @@
 #include <__iterator/concepts.h>
 #include <__iterator/incrementable_traits.h>
 #include <__iterator/iterator_traits.h>
-#include <__iterator/next.h>
 #include <__iterator/projected.h>
 #include <__ranges/concepts.h>
 #include <__utility/move.h>
@@ -42,17 +41,8 @@ struct __for_each_n {
   template <input_iterator _Iter, class _Proj = identity, indirectly_unary_invocable<projected<_Iter, _Proj>> _Func>
   _LIBCPP_HIDE_FROM_ABI constexpr for_each_n_result<_Iter, _Func>
   operator()(_Iter __first, iter_difference_t<_Iter> __count, _Func __func, _Proj __proj = {}) const {
-    if constexpr (forward_iterator<_Iter>) {
-      auto __f    = [&](auto&& __val) { std::invoke(__func, std::invoke(__proj, __val)); };
-      auto __last = std::for_each_n(__first, __count, __f);
-      return {std::move(__last), std::move(__func)};
-    } else {
-      while (__count-- > 0) {
-        std::invoke(__func, std::invoke(__proj, *__first));
-        ++__first;
-      }
-      return {std::move(__first), std::move(__func)};
-    }
+    auto __last = std::__for_each_n(std::move(__first), __count, __func, __proj);
+    return {std::move(__last), std::move(__func)};
   }
 };
 
diff --git a/libcxx/test/benchmarks/algorithms/nonmodifying/for_each.bench.cpp b/libcxx/test/benchmarks/algorithms/nonmodifying/for_each.bench.cpp
@@ -12,6 +12,7 @@
 #include <cstddef>
 #include <deque>
 #include <list>
+#include <ranges>
 #include <string>
 #include <vector>
 
@@ -48,20 +49,6 @@ int main(int argc, char** argv) {
           ->Arg(1 << 16)
           ->Arg(1 << 18);
     };
-    bm.operator()<std::vector<char>>("std::for_each(vector<char>)", std_for_each);
-    bm.operator()<std::deque<char>>("std::for_each(deque<char>)", std_for_each);
-    bm.operator()<std::list<char>>("std::for_each(list<char>)", std_for_each);
-    bm.operator()<std::vector<char>>("rng::for_each(vector<char>)", std::ranges::for_each);
-    bm.operator()<std::deque<char>>("rng::for_each(deque<char>)", std::ranges::for_each);
-    bm.operator()<std::list<char>>("rng::for_each(list<char>)", std::ranges::for_each);
-
-    bm.operator()<std::vector<short>>("std::for_each(vector<short>)", std_for_each);
-    bm.operator()<std::deque<short>>("std::for_each(deque<short>)", std_for_each);
-    bm.operator()<std::list<short>>("std::for_each(list<short>)", std_for_each);
-    bm.operator()<std::vector<short>>("rng::for_each(vector<short>)", std::ranges::for_each);
-    bm.operator()<std::deque<short>>("rng::for_each(deque<short>)", std::ranges::for_each);
-    bm.operator()<std::list<short>>("rng::for_each(list<short>)", std::ranges::for_each);
-
     bm.operator()<std::vector<int>>("std::for_each(vector<int>)", std_for_each);
     bm.operator()<std::deque<int>>("std::for_each(deque<int>)", std_for_each);
     bm.operator()<std::list<int>>("std::for_each(list<int>)", std_for_each);
@@ -70,6 +57,47 @@ int main(int argc, char** argv) {
     bm.operator()<std::list<int>>("rng::for_each(list<int>)", std::ranges::for_each);
   }
 
+  // {std,ranges}::for_each for join_view
+  {
+    auto bm = []<class Container>(std::string name, auto for_each) {
+      using C1       = typename Container::value_type;
+      using ElemType = typename C1::value_type;
+
+      benchmark::RegisterBenchmark(
+          name,
+          [for_each](auto& st) {
+            std::size_t const size     = st.range(0);
+            std::size_t const seg_size = 256;
+            std::size_t const segments = (size + seg_size - 1) / seg_size;
+            Container c(segments);
+            for (std::size_t i = 0, n = size; i < segments; ++i, n -= seg_size) {
+              c[i].resize(std::min(seg_size, n), ElemType(1));
+            }
+
+            auto view  = c | std::views::join;
+            auto first = view.begin();
+            auto last  = view.end();
+
+            for ([[maybe_unused]] auto _ : st) {
+              benchmark::DoNotOptimize(c);
+              auto result = for_each(first, last, [](ElemType& x) { x = std::clamp<ElemType>(x, 10, 100); });
+              benchmark::DoNotOptimize(result);
+            }
+          })
+          ->Arg(8)
+          ->Arg(32)
+          ->Arg(50) // non power-of-two
+          ->Arg(1024)
+          ->Arg(4096)
+          ->Arg(8192)
+          ->Arg(1 << 14)
+          ->Arg(1 << 16)
+          ->Arg(1 << 18);
+    };
+    bm.operator()<std::vector<std::vector<int>>>("std::for_each(join_view(vector<vector<int>>))", std_for_each);
+    bm.operator()<std::vector<std::vector<int>>>("rng::for_each(join_view(vector<vector<int>>)", std::ranges::for_each);
+  }
+
   benchmark::Initialize(&argc, argv);
   benchmark::RunSpecifiedBenchmarks();
   benchmark::Shutdown();
diff --git a/libcxx/test/benchmarks/algorithms/nonmodifying/for_each_join_view.bench.cpp b/libcxx/test/benchmarks/algorithms/nonmodifying/for_each_join_view.bench.cpp
diff --git a/libcxx/test/benchmarks/algorithms/nonmodifying/for_each_n.bench.cpp b/libcxx/test/benchmarks/algorithms/nonmodifying/for_each_n.bench.cpp
