[libcxx-commits] [libcxx] Optimize __assign_with_sentinel in std::vector (PR #113852)

Peng Liu via libcxx-commits libcxx-commits at lists.llvm.org
Mon Nov 11 11:46:28 PST 2024


https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/113852

>From 2756f0a404d0ac8dd95f579115654ff9939004d5 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Sun, 27 Oct 2024 17:03:28 -0400
Subject: [PATCH 1/3] Improve __assign_with_sentinel in std::vector

---
 libcxx/include/__vector/vector.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 6db202efb279b3..9eabeca51be6ea 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -1017,9 +1017,14 @@ template <class _Tp, class _Allocator>
 template <class _Iterator, class _Sentinel>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
 vector<_Tp, _Allocator>::__assign_with_sentinel(_Iterator __first, _Sentinel __last) {
-  clear();
-  for (; __first != __last; ++__first)
-    emplace_back(*__first);
+  pointer __cur = __begin_;
+  for (; __first != __last && __cur != __end_; ++__cur, ++__first)
+    *__cur = *__first;
+  if (__cur != __end_)
+    __destruct_at_end(__cur);
+  else
+    for (; __first != __last; ++__first)
+      emplace_back(*__first);
 }
 
 template <class _Tp, class _Allocator>

>From c29ee1d9cd5b3e8139f5d18c5ac5020df0e6989a Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 6 Nov 2024 22:54:20 -0500
Subject: [PATCH 2/3] Avoid invoking operator,

---
 libcxx/include/__vector/vector.h              |  2 +-
 libcxx/test/benchmarks/ContainerBenchmarks.h  | 70 +++++++++++++++++++
 libcxx/test/benchmarks/GenerateInput.h        |  8 +++
 .../benchmarks/vector_operations.bench.cpp    |  6 ++
 4 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 9eabeca51be6ea..83c52128f5a316 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -1018,7 +1018,7 @@ template <class _Iterator, class _Sentinel>
 _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI void
 vector<_Tp, _Allocator>::__assign_with_sentinel(_Iterator __first, _Sentinel __last) {
   pointer __cur = __begin_;
-  for (; __first != __last && __cur != __end_; ++__cur, ++__first)
+  for (; __first != __last && __cur != __end_; ++__first, (void)++__cur)
     *__cur = *__first;
   if (__cur != __end_)
     __destruct_at_end(__cur);
diff --git a/libcxx/test/benchmarks/ContainerBenchmarks.h b/libcxx/test/benchmarks/ContainerBenchmarks.h
index 742c848328604c..63d5d9829464e4 100644
--- a/libcxx/test/benchmarks/ContainerBenchmarks.h
+++ b/libcxx/test/benchmarks/ContainerBenchmarks.h
@@ -48,6 +48,76 @@ void BM_Assignment(benchmark::State& st, Container) {
   }
 }
 
+// Wrap any Iterator into an input iterator
+template <typename Iterator>
+class InputIterator {
+  using iter_traits = std::iterator_traits<Iterator>;
+
+public:
+  using iterator_category = std::input_iterator_tag;
+  using value_type        = typename iter_traits::value_type;
+  using difference_type   = typename iter_traits::difference_type;
+  using pointer           = typename iter_traits::pointer;
+  using reference         = typename iter_traits::reference;
+
+  InputIterator(Iterator it) : current_(it) {}
+
+  reference operator*() { return *current_; }
+  InputIterator& operator++() {
+    ++current_;
+    return *this;
+  }
+  InputIterator operator++(int) {
+    InputIterator tmp = *this;
+    ++(*this);
+    return tmp;
+  }
+
+  friend bool operator==(const InputIterator& lhs, const InputIterator& rhs) { return lhs.current_ == rhs.current_; }
+  friend bool operator!=(const InputIterator& lhs, const InputIterator& rhs) { return !(lhs == rhs); }
+
+private:
+  Iterator current_;
+};
+
+template <typename Iterator>
+InputIterator<Iterator> make_input_iterator(Iterator it) {
+  return InputIterator<Iterator>(it);
+}
+
+template <class Container,
+          class GenInputs,
+          typename std::enable_if<std::is_trivial<typename Container::value_type>::value>::type* = nullptr>
+void BM_AssignInputIterIter(benchmark::State& st, Container c, GenInputs gen) {
+  auto in = gen(st.range(1));
+  benchmark::DoNotOptimize(&in);
+  for (auto _ : st) {
+    st.PauseTiming();
+    c.resize(st.range(0));
+    benchmark::DoNotOptimize(&c);
+    st.ResumeTiming();
+    c.assign(make_input_iterator(in.begin()), make_input_iterator(in.end()));
+    benchmark::ClobberMemory();
+  }
+}
+
+template <class Container,
+          class GenInputs,
+          typename std::enable_if<!std::is_trivial<typename Container::value_type>::value>::type* = nullptr>
+void BM_AssignInputIterIter(benchmark::State& st, Container c, GenInputs gen) {
+  auto v  = gen(1, 100);
+  auto in = gen(st.range(1), 32);
+  benchmark::DoNotOptimize(&in);
+  for (auto _ : st) {
+    st.PauseTiming();
+    c.resize(st.range(0), v[0]);
+    benchmark::DoNotOptimize(&c);
+    st.ResumeTiming();
+    c.assign(make_input_iterator(in.begin()), make_input_iterator(in.end()));
+    benchmark::ClobberMemory();
+  }
+}
+
 template <class Container>
 void BM_ConstructSizeValue(benchmark::State& st, Container, typename Container::value_type const& val) {
   const auto size = st.range(0);
diff --git a/libcxx/test/benchmarks/GenerateInput.h b/libcxx/test/benchmarks/GenerateInput.h
index 0f3e9309271bb1..c815be7587c5ca 100644
--- a/libcxx/test/benchmarks/GenerateInput.h
+++ b/libcxx/test/benchmarks/GenerateInput.h
@@ -116,6 +116,14 @@ inline std::vector<std::string> getRandomStringInputs(std::size_t N) {
   return inputs;
 }
 
+inline std::vector<std::string> getRandomStringInputsWithLength(std::size_t N, std::size_t len) {
+  std::vector<std::string> inputs;
+  inputs.reserve(N);
+  for (size_t i = 0; i < N; ++i)
+    inputs.push_back(getRandomString(len));
+  return inputs;
+}
+
 inline std::vector<std::string> getPrefixedRandomStringInputs(std::size_t N) {
   std::vector<std::string> inputs;
   constexpr int kSuffixLength = 32;
diff --git a/libcxx/test/benchmarks/vector_operations.bench.cpp b/libcxx/test/benchmarks/vector_operations.bench.cpp
index ce8ab233fc9817..ae09d3f5ec8831 100644
--- a/libcxx/test/benchmarks/vector_operations.bench.cpp
+++ b/libcxx/test/benchmarks/vector_operations.bench.cpp
@@ -69,4 +69,10 @@ BENCHMARK(bm_grow<std::string>);
 BENCHMARK(bm_grow<std::unique_ptr<int>>);
 BENCHMARK(bm_grow<std::deque<int>>);
 
+BENCHMARK_CAPTURE(BM_AssignInputIterIter, vector_int, std::vector<int>{}, getRandomIntegerInputs<int>)
+    ->Args({TestNumInputs, TestNumInputs});
+
+BENCHMARK_CAPTURE(BM_AssignInputIterIter, vector_string, std::vector<std::string>{}, getRandomStringInputsWithLength)
+    ->Args({TestNumInputs, TestNumInputs});
+
 BENCHMARK_MAIN();

>From 03b8721cb5baa9925937cf7dba308181a9300bf2 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Mon, 11 Nov 2024 14:19:10 -0500
Subject: [PATCH 3/3] Add release note to this optimization in 20.rst

---
 libcxx/docs/ReleaseNotes/20.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index 9039c6f046445b..a9c6f105043d08 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -69,6 +69,10 @@ Improvements and New Features
 - The ``_LIBCPP_ABI_BOUNDED_ITERATORS_IN_STD_ARRAY`` ABI configuration was added, which allows storing valid bounds
   in ``std::array::iterator`` and detecting OOB accesses when the appropriate hardening mode is enabled.
 
+- The `assign(_InputIterator, _InputIterator)` function of `std::vector<_Tp, _Allocator>` has been optimized for
+  non-trivial element types, such as `std::vector<std::string>`, with a performance improvement of up to 2.3x. The
+  performance for trivial types, such as `std::vector<int>`, remains similar or shows slight improvements.
+
 Deprecations and Removals
 -------------------------
 



More information about the libcxx-commits mailing list