[libcxx-commits] [libcxx] Optimize __assign_with_sentinel in std::vector (PR #113852)
Peng Liu via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Nov 12 08:03:25 PST 2024
https://github.com/winner245 updated https://github.com/llvm/llvm-project/pull/113852
>From a5c98e29e05e44e039fe44bec8fffaf0ec00d157 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/4] 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 457c4058d475262a7e9b2e7464f1e956e0c74cf4 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/4] 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 66ed5decee9f4c84abe3aeb4f80ad65cd09f8a9c 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/4] 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 87f94b9cb794888052b19ad2e8630f9c79a56693 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/4] 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();
More information about the libcxx-commits
mailing list