[libcxx-commits] [libcxx] Optimize input iterator overload of `std::vector::assign(first, last)` (PR #113852)
Peng Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Nov 17 12:51:26 PST 2024
https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/113852
>From 9299a1b49faa8499c50a62887652942ed91d0a3c 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/7] 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 d2d707d8c913c0..a87ac8a57e7882 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 6c8dc053bd4954ee9828d0720f28321caff08fbc 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/7] 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 a87ac8a57e7882..2c295123cb99c4 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 36058a0ad201c41a7a32f6c1f89a05356835204f 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/7] 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
-------------------------
>From 4bc1223410f4a4998f6991da944745dca10b95e8 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Tue, 12 Nov 2024 10:59:11 -0500
Subject: [PATCH 4/7] Restructure benchmark tests
---
libcxx/test/benchmarks/ContainerBenchmarks.h | 56 +++----------------
libcxx/test/benchmarks/GenerateInput.h | 13 ++++-
.../benchmarks/vector_operations.bench.cpp | 4 ++
3 files changed, 23 insertions(+), 50 deletions(-)
diff --git a/libcxx/test/benchmarks/ContainerBenchmarks.h b/libcxx/test/benchmarks/ContainerBenchmarks.h
index 63d5d9829464e4..111c2485ef33b6 100644
--- a/libcxx/test/benchmarks/ContainerBenchmarks.h
+++ b/libcxx/test/benchmarks/ContainerBenchmarks.h
@@ -12,8 +12,9 @@
#include <cassert>
-#include "Utilities.h"
#include "benchmark/benchmark.h"
+#include "Utilities.h"
+#include "test_iterators.h"
namespace ContainerBenchmarks {
@@ -48,55 +49,16 @@ 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));
+ c.resize(st.range(0));
benchmark::DoNotOptimize(&in);
+ benchmark::DoNotOptimize(&c);
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()));
+ c.assign(cpp17_input_iterator(in.begin()), cpp17_input_iterator(in.end()));
benchmark::ClobberMemory();
}
}
@@ -106,14 +68,12 @@ template <class Container,
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);
+ c.resize(st.range(0), v[0]);
auto in = gen(st.range(1), 32);
benchmark::DoNotOptimize(&in);
+ benchmark::DoNotOptimize(&c);
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()));
+ c.assign(cpp17_input_iterator(in.begin()), cpp17_input_iterator(in.end()));
benchmark::ClobberMemory();
}
}
diff --git a/libcxx/test/benchmarks/GenerateInput.h b/libcxx/test/benchmarks/GenerateInput.h
index c815be7587c5ca..c5695ef2c64943 100644
--- a/libcxx/test/benchmarks/GenerateInput.h
+++ b/libcxx/test/benchmarks/GenerateInput.h
@@ -116,14 +116,23 @@ 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) {
+inline std::vector<std::string> getRandomStringInputsWithLength(std::size_t N, std::size_t len) { // N-by-len
std::vector<std::string> inputs;
inputs.reserve(N);
- for (size_t i = 0; i < N; ++i)
+ for (std::size_t i = 0; i < N; ++i)
inputs.push_back(getRandomString(len));
return inputs;
}
+template <class IntT>
+inline std::vector<std::vector<IntT>> getRandomIntegerInputsWithLength(std::size_t N, std::size_t len) { // N-by-len
+ std::vector<std::vector<IntT>> inputs;
+ inputs.reserve(N);
+ for (std::size_t i = 0; i < N; ++i)
+ inputs.push_back(getRandomIntegerInputs<IntT>(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 ae09d3f5ec8831..0bae771b0904fc 100644
--- a/libcxx/test/benchmarks/vector_operations.bench.cpp
+++ b/libcxx/test/benchmarks/vector_operations.bench.cpp
@@ -75,4 +75,8 @@ BENCHMARK_CAPTURE(BM_AssignInputIterIter, vector_int, std::vector<int>{}, getRan
BENCHMARK_CAPTURE(BM_AssignInputIterIter, vector_string, std::vector<std::string>{}, getRandomStringInputsWithLength)
->Args({TestNumInputs, TestNumInputs});
+BENCHMARK_CAPTURE(
+ BM_AssignInputIterIter, vector_vector_int, std::vector<std::vector<int>>{}, getRandomIntegerInputsWithLength<int>)
+ ->Args({TestNumInputs, TestNumInputs});
+
BENCHMARK_MAIN();
>From 03bfb19ec7500c6cc25f19195b0a39257e1d81d5 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Tue, 12 Nov 2024 11:26:33 -0500
Subject: [PATCH 5/7] Run clang-format
---
libcxx/include/__vector/vector.h | 5 +++--
libcxx/test/benchmarks/ContainerBenchmarks.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/libcxx/include/__vector/vector.h b/libcxx/include/__vector/vector.h
index 2c295123cb99c4..7d82815633b2fc 100644
--- a/libcxx/include/__vector/vector.h
+++ b/libcxx/include/__vector/vector.h
@@ -1020,11 +1020,12 @@ vector<_Tp, _Allocator>::__assign_with_sentinel(_Iterator __first, _Sentinel __l
pointer __cur = __begin_;
for (; __first != __last && __cur != __end_; ++__first, (void)++__cur)
*__cur = *__first;
- if (__cur != __end_)
+ if (__cur != __end_) {
__destruct_at_end(__cur);
- else
+ } else {
for (; __first != __last; ++__first)
emplace_back(*__first);
+ }
}
template <class _Tp, class _Allocator>
diff --git a/libcxx/test/benchmarks/ContainerBenchmarks.h b/libcxx/test/benchmarks/ContainerBenchmarks.h
index 111c2485ef33b6..3ec4d47faeefdf 100644
--- a/libcxx/test/benchmarks/ContainerBenchmarks.h
+++ b/libcxx/test/benchmarks/ContainerBenchmarks.h
@@ -67,7 +67,7 @@ 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 v = gen(1, 100);
c.resize(st.range(0), v[0]);
auto in = gen(st.range(1), 32);
benchmark::DoNotOptimize(&in);
>From 4db31b94e1444d408cd5b76bf0f78918ad6889fd Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Wed, 13 Nov 2024 22:42:00 -0500
Subject: [PATCH 6/7] Update release note
---
libcxx/docs/ReleaseNotes/20.rst | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index a9c6f105043d08..4f4bcbd98a8f29 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -69,9 +69,8 @@ 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.
+- The input iterator overload of `assign(_InputIterator, _InputIterator)` in `std::vector<_Tp, _Allocator>` has been
+ optimized, resulting in a performance improvement of up to 3.7x.
Deprecations and Removals
-------------------------
>From 6112450dac7b487b6366d0672e258a8ab878dbe5 Mon Sep 17 00:00:00 2001
From: Peng Liu <winner245 at hotmail.com>
Date: Fri, 15 Nov 2024 14:27:42 -0500
Subject: [PATCH 7/7] Refactor tests
---
libcxx/docs/ReleaseNotes/20.rst | 5 +++--
libcxx/test/benchmarks/ContainerBenchmarks.h | 22 +++----------------
.../benchmarks/vector_operations.bench.cpp | 9 +++++---
3 files changed, 12 insertions(+), 24 deletions(-)
diff --git a/libcxx/docs/ReleaseNotes/20.rst b/libcxx/docs/ReleaseNotes/20.rst
index 4f4bcbd98a8f29..366d07178a20af 100644
--- a/libcxx/docs/ReleaseNotes/20.rst
+++ b/libcxx/docs/ReleaseNotes/20.rst
@@ -69,8 +69,9 @@ 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 input iterator overload of `assign(_InputIterator, _InputIterator)` in `std::vector<_Tp, _Allocator>` has been
- optimized, resulting in a performance improvement of up to 3.7x.
+- The input iterator overload of `assign(_InputIterator, _InputIterator)` in `std::vector<_Tp, _Allocator>` has been
+ optimized, resulting in a performance improvement of up to 2x for trivial element types (e.g., `std::vector<int>`),
+ and up to 3.4x for non-trivial element types (e.g., `std::vector<std::vector<int>>`).
Deprecations and Removals
-------------------------
diff --git a/libcxx/test/benchmarks/ContainerBenchmarks.h b/libcxx/test/benchmarks/ContainerBenchmarks.h
index 3ec4d47faeefdf..265412d413bd7c 100644
--- a/libcxx/test/benchmarks/ContainerBenchmarks.h
+++ b/libcxx/test/benchmarks/ContainerBenchmarks.h
@@ -49,27 +49,11 @@ void BM_Assignment(benchmark::State& st, Container) {
}
}
-template <class Container,
- class GenInputs,
- typename std::enable_if<std::is_trivial<typename Container::value_type>::value>::type* = nullptr>
+template <std::size_t... sz, typename Container, typename GenInputs>
void BM_AssignInputIterIter(benchmark::State& st, Container c, GenInputs gen) {
- auto in = gen(st.range(1));
- c.resize(st.range(0));
- benchmark::DoNotOptimize(&in);
- benchmark::DoNotOptimize(&c);
- for (auto _ : st) {
- c.assign(cpp17_input_iterator(in.begin()), cpp17_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 v = gen(1, sz...);
c.resize(st.range(0), v[0]);
- auto in = gen(st.range(1), 32);
+ auto in = gen(st.range(1), sz...);
benchmark::DoNotOptimize(&in);
benchmark::DoNotOptimize(&c);
for (auto _ : st) {
diff --git a/libcxx/test/benchmarks/vector_operations.bench.cpp b/libcxx/test/benchmarks/vector_operations.bench.cpp
index 0bae771b0904fc..b7e25759f33438 100644
--- a/libcxx/test/benchmarks/vector_operations.bench.cpp
+++ b/libcxx/test/benchmarks/vector_operations.bench.cpp
@@ -72,11 +72,14 @@ 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)
+BENCHMARK_CAPTURE(
+ BM_AssignInputIterIter<32>, vector_string, std::vector<std::string>{}, getRandomStringInputsWithLength)
->Args({TestNumInputs, TestNumInputs});
-BENCHMARK_CAPTURE(
- BM_AssignInputIterIter, vector_vector_int, std::vector<std::vector<int>>{}, getRandomIntegerInputsWithLength<int>)
+BENCHMARK_CAPTURE(BM_AssignInputIterIter<100>,
+ vector_vector_int,
+ std::vector<std::vector<int>>{},
+ getRandomIntegerInputsWithLength<int>)
->Args({TestNumInputs, TestNumInputs});
BENCHMARK_MAIN();
More information about the libcxx-commits
mailing list