[libc++] Avoid materializing input ranges prepended to deque#199969
Open
BITree2004 wants to merge 2 commits into
Open
[libc++] Avoid materializing input ranges prepended to deque#199969BITree2004 wants to merge 2 commits into
BITree2004 wants to merge 2 commits into
Conversation
|
Hello @BITree2004 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
Author
|
I have read the LLVM AI Tool Use Policy, Code-Review Policy, and Developer Policy. I didn't use any AI tools in preparing this pull request |
llvmorg-github-actions
Bot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: fffuuu (BITree2004) ChangesFixes #199788 Avoid materializing unsized input ranges when inserting at the beginning of std::deque. For input iterator insertion at begin() and C++23 insert_range/prepend_range, deque now prepends elements directly and reverses the inserted prefix to preserve range order. The existing split_buffer path remains the fallback when value_type is not swappable. Testing:
Full diff: https://github.com/llvm/llvm-project/pull/199969.diff 4 Files Affected:
diff --git a/libcxx/include/deque b/libcxx/include/deque
index c8c6889f1a165..a8716920134ba 100644
--- a/libcxx/include/deque
+++ b/libcxx/include/deque
@@ -194,6 +194,7 @@ template <class T, class Allocator, class Predicate>
# include <__algorithm/ranges_copy_n.h>
# include <__algorithm/remove.h>
# include <__algorithm/remove_if.h>
+# include <__algorithm/reverse.h>
# include <__assert>
# include <__config>
# include <__debug_utils/sanitizers.h>
@@ -224,6 +225,7 @@ template <class T, class Allocator, class Predicate>
# include <__type_traits/conditional.h>
# include <__type_traits/container_traits.h>
# include <__type_traits/enable_if.h>
+# include <__type_traits/integral_constant.h>
# include <__type_traits/is_allocator.h>
# include <__type_traits/is_convertible.h>
# include <__type_traits/is_nothrow_assignable.h>
@@ -871,6 +873,8 @@ public:
return __insert_with_size(__position, ranges::begin(__range), __n);
} else {
+ if (__position == cbegin())
+ return __prepend_with_sentinel(ranges::begin(__range), ranges::end(__range));
return __insert_with_sentinel(__position, ranges::begin(__range), ranges::end(__range));
}
}
@@ -1231,6 +1235,15 @@ private:
template <class _Iterator, class _Sentinel>
_LIBCPP_HIDE_FROM_ABI iterator __insert_with_sentinel(const_iterator __p, _Iterator __f, _Sentinel __l);
+# ifndef _LIBCPP_CXX03_LANG
+ template <class _Iterator, class _Sentinel>
+ _LIBCPP_HIDE_FROM_ABI iterator __prepend_with_sentinel(_Iterator __f, _Sentinel __l);
+ template <class _Iterator, class _Sentinel>
+ _LIBCPP_HIDE_FROM_ABI iterator __prepend_with_sentinel(_Iterator __f, _Sentinel __l, true_type);
+ template <class _Iterator, class _Sentinel>
+ _LIBCPP_HIDE_FROM_ABI iterator __prepend_with_sentinel(_Iterator __f, _Sentinel __l, false_type);
+# endif
+
template <class _Iterator>
_LIBCPP_HIDE_FROM_ABI iterator __insert_with_size(const_iterator __p, _Iterator __f, size_type __n);
@@ -1784,6 +1797,11 @@ template <class _Tp, class _Allocator>
template <class _InputIter, __enable_if_t<__has_exactly_input_iterator_category<_InputIter>::value, int> >
typename deque<_Tp, _Allocator>::iterator
deque<_Tp, _Allocator>::insert(const_iterator __p, _InputIter __f, _InputIter __l) {
+# ifndef _LIBCPP_CXX03_LANG
+ if (__p == cbegin())
+ return __prepend_with_sentinel(__f, __l);
+# endif
+
return __insert_with_sentinel(__p, __f, __l);
}
@@ -1797,6 +1815,45 @@ deque<_Tp, _Allocator>::__insert_with_sentinel(const_iterator __p, _Iterator __f
return insert(__p, move_iterator<__bi>(__buf.begin()), move_iterator<__bi>(__buf.end()));
}
+# ifndef _LIBCPP_CXX03_LANG
+template <class _Tp, class _Allocator>
+template <class _Iterator, class _Sentinel>
+_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
+deque<_Tp, _Allocator>::__prepend_with_sentinel(_Iterator __f, _Sentinel __l) {
+ return __prepend_with_sentinel(
+ std::move(__f), std::move(__l), integral_constant<bool, __is_swappable_v<value_type> >());
+}
+
+template <class _Tp, class _Allocator>
+template <class _Iterator, class _Sentinel>
+_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
+deque<_Tp, _Allocator>::__prepend_with_sentinel(_Iterator __f, _Sentinel __l, true_type) {
+ size_type __inserted = 0;
+ auto __guard = std::__make_exception_guard([&] {
+ while (__inserted > 0) {
+ pop_front();
+ --__inserted;
+ }
+ });
+
+ for (; __f != __l; ++__f) {
+ emplace_front(*__f);
+ ++__inserted;
+ }
+ std::reverse(begin(), begin() + __inserted);
+
+ __guard.__complete();
+ return begin();
+}
+
+template <class _Tp, class _Allocator>
+template <class _Iterator, class _Sentinel>
+_LIBCPP_HIDE_FROM_ABI typename deque<_Tp, _Allocator>::iterator
+deque<_Tp, _Allocator>::__prepend_with_sentinel(_Iterator __f, _Sentinel __l, false_type) {
+ return __insert_with_sentinel(cbegin(), std::move(__f), std::move(__l));
+}
+# endif
+
template <class _Tp, class _Allocator>
template <class _ForwardIterator, __enable_if_t<__has_exactly_forward_iterator_category<_ForwardIterator>::value, int> >
typename deque<_Tp, _Allocator>::iterator
diff --git a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
index 92346c38563fb..4373cb44dc917 100644
--- a/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
+++ b/libcxx/test/benchmarks/containers/sequence/sequence_container_benchmarks.h
@@ -447,6 +447,28 @@ void sequence_container_benchmarks(std::string container) {
state.ResumeTiming();
}
});
+
+ for (auto gen : generators)
+ bench("prepend_range(input-iter) (full container)" + tostr(gen), [gen](auto& state) {
+ auto const size = state.range(0);
+ std::vector<ValueType> in;
+ std::generate_n(std::back_inserter(in), size, gen);
+ DoNotOptimizeData(in);
+
+ Container c(in.begin(), in.end());
+ DoNotOptimizeData(c);
+ for (auto _ : state) {
+ using Iter = cpp20_input_iterator<ValueType*>;
+ auto first = Iter(in.data());
+ auto last = sentinel_wrapper<Iter>(Iter(in.data() + in.size()));
+ c.prepend_range(std::ranges::subrange(std::move(first), std::move(last)));
+ DoNotOptimizeData(c);
+
+ state.PauseTiming();
+ c.erase(c.begin(), std::next(c.begin(), size));
+ state.ResumeTiming();
+ }
+ });
}
/////////////////////////
diff --git a/libcxx/test/libcxx/containers/sequences/deque/input_range_insert_at_begin.pass.cpp b/libcxx/test/libcxx/containers/sequences/deque/input_range_insert_at_begin.pass.cpp
new file mode 100644
index 0000000000000..7d5794faa551a
--- /dev/null
+++ b/libcxx/test/libcxx/containers/sequences/deque/input_range_insert_at_begin.pass.cpp
@@ -0,0 +1,94 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Check that inserting an unsized input range at the start of a deque uses
+// existing front spare capacity instead of materializing the range first.
+
+// UNSUPPORTED: c++03
+
+#include <cassert>
+#include <deque>
+
+#include "count_new.h"
+#include "test_iterators.h"
+#include "test_macros.h"
+
+#if TEST_STD_VER >= 23
+# include <ranges>
+#endif
+
+std::deque<int> make_deque_with_front_spare() {
+ std::deque<int> c;
+ for (int i = 0; i != 8; ++i)
+ c.push_back(100 + i);
+ for (int i = 0; i != 4; ++i)
+ c.pop_front();
+ return c;
+}
+
+void assert_expected(const std::deque<int>& c) {
+ int expected[] = {0, 1, 2, 3, 104, 105, 106, 107};
+ assert(c.size() == 8);
+ for (int i = 0; i != 8; ++i)
+ assert(c[i] == expected[i]);
+}
+
+void test_insert_iter_iter() {
+ std::deque<int> c = make_deque_with_front_spare();
+ int input[] = {0, 1, 2, 3};
+ typedef cpp17_input_iterator<int*> Iter;
+
+ {
+ DisableAllocationGuard guard;
+ auto it = c.insert(c.begin(), Iter(input), Iter(input + 4));
+ assert(it == c.begin());
+ }
+
+ assert_expected(c);
+}
+
+#if TEST_STD_VER >= 23
+void test_insert_range() {
+ std::deque<int> c = make_deque_with_front_spare();
+ int input[] = {0, 1, 2, 3};
+ using Iter = cpp20_input_iterator<int*>;
+ auto in = std::ranges::subrange(Iter(input), sentinel_wrapper<Iter>(Iter(input + 4)));
+
+ {
+ DisableAllocationGuard guard;
+ auto it = c.insert_range(c.begin(), in);
+ assert(it == c.begin());
+ }
+
+ assert_expected(c);
+}
+
+void test_prepend_range() {
+ std::deque<int> c = make_deque_with_front_spare();
+ int input[] = {0, 1, 2, 3};
+ using Iter = cpp20_input_iterator<int*>;
+ auto in = std::ranges::subrange(Iter(input), sentinel_wrapper<Iter>(Iter(input + 4)));
+
+ {
+ DisableAllocationGuard guard;
+ c.prepend_range(in);
+ }
+
+ assert_expected(c);
+}
+#endif
+
+int main(int, char**) {
+ test_insert_iter_iter();
+#if TEST_STD_VER >= 23
+ test_insert_range();
+ test_prepend_range();
+#endif
+
+ return 0;
+}
diff --git a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
index 5ff572c79e512..cfd5135bad50c 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.modifiers/prepend_range.pass.cpp
@@ -17,6 +17,82 @@
#include "../../insert_range_sequence_containers.h"
#include "test_macros.h"
+#if !defined(TEST_HAS_NO_EXCEPTIONS)
+struct ThrowingSwap {
+ static bool throwing_enabled;
+ static int swaps;
+
+ int value = 0;
+
+ ThrowingSwap() = default;
+ ThrowingSwap(int v) : value(v) {}
+
+ friend void swap(ThrowingSwap& lhs, ThrowingSwap& rhs) {
+ int tmp = lhs.value;
+ lhs.value = rhs.value;
+ rhs.value = tmp;
+ if (throwing_enabled && ++swaps == 2)
+ throw 1;
+ }
+
+ friend bool operator==(const ThrowingSwap& lhs, const ThrowingSwap& rhs) { return lhs.value == rhs.value; }
+};
+
+bool ThrowingSwap::throwing_enabled = false;
+int ThrowingSwap::swaps = 0;
+#endif
+
+void test_input_range_strong_exception_safety() {
+#if !defined(TEST_HAS_NO_EXCEPTIONS)
+ using T = ThrowingCopy<3>;
+
+ T::throwing_enabled = false;
+ std::deque<T> c;
+ for (int i = 0; i != 8; ++i)
+ c.emplace_back(100 + i);
+ for (int i = 0; i != 4; ++i)
+ c.pop_front();
+ std::deque<T> expected = c;
+
+ T input[] = {T(0), T(1), T(2), T(3)};
+ using Iter = cpp20_input_iterator<T*>;
+ auto in = std::ranges::subrange(Iter(input), sentinel_wrapper<Iter>(Iter(input + 4)));
+
+ T::reset();
+ T::throwing_enabled = true;
+ try {
+ c.prepend_range(in);
+ assert(false);
+ } catch (int) {
+ assert(c == expected);
+ }
+ T::throwing_enabled = false;
+#endif
+}
+
+void test_input_range_reverse_exception_safety() {
+#if !defined(TEST_HAS_NO_EXCEPTIONS)
+ std::deque<ThrowingSwap> c = {100, 101, 102, 103, 104, 105, 106, 107};
+ for (int i = 0; i != 4; ++i)
+ c.pop_front();
+ std::deque<ThrowingSwap> expected = c;
+
+ ThrowingSwap input[] = {0, 1, 2, 3};
+ using Iter = cpp20_input_iterator<ThrowingSwap*>;
+ auto in = std::ranges::subrange(Iter(input), sentinel_wrapper<Iter>(Iter(input + 4)));
+
+ ThrowingSwap::swaps = 0;
+ ThrowingSwap::throwing_enabled = true;
+ try {
+ c.prepend_range(in);
+ assert(false);
+ } catch (int) {
+ assert(c == expected);
+ }
+ ThrowingSwap::throwing_enabled = false;
+#endif
+}
+
// Tested cases:
// - different kinds of insertions (prepending an {empty/one-element/mid-sized/long range} into an
// {empty/one-element/full} container);
@@ -36,6 +112,8 @@ int main(int, char**) {
test_prepend_range_exception_safety_throwing_copy<std::deque>();
test_prepend_range_exception_safety_throwing_allocator<std::deque, int>();
+ test_input_range_strong_exception_safety();
+ test_input_range_reverse_exception_safety();
return 0;
}
|
| template <class _Iterator, class _Sentinel> | ||
| _LIBCPP_HIDE_FROM_ABI iterator __insert_with_sentinel(const_iterator __p, _Iterator __f, _Sentinel __l); | ||
|
|
||
| # ifndef _LIBCPP_CXX03_LANG |
Contributor
There was a problem hiding this comment.
Do we need to avoid the changes in C++03 mode?
Author
There was a problem hiding this comment.
Thanks for catching this. I was too conservative with the C++03 guard
C++03 doesn’t have the C++23 range APIs, but the iterator-pair insert overload is affected by the same split_buffer materialization. I will make the new path available there as well, using push_front instead of emplace_front in C++03 mode. I will keep the frozen C++03 headers unchanged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #199788
Avoid materializing unsized input ranges when inserting at the beginning of std::deque.
For input iterator insertion at begin() and C++23 insert_range/prepend_range, deque now prepends elements directly and reverses the inserted prefix to preserve range order. The existing split_buffer path remains the fallback when value_type is not swappable.
Testing: