[libcxx-commits] [libcxx] [libc++] Make drop_view::begin constant time (#72883) (PR #72929)
Hongyu Ouyang via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Dec 21 13:32:34 PST 2023
https://github.com/casavaca updated https://github.com/llvm/llvm-project/pull/72929
>From 79ad83718077d1510e36e36f83080d9ea5cc4567 Mon Sep 17 00:00:00 2001
From: Hongyu Ouyang <HongyuOuyang at proton.me>
Date: Mon, 20 Nov 2023 23:20:36 -0800
Subject: [PATCH 1/3] [libc++] optimization on ranges::drop_view::begin
(#72883)
The optimization is specifically for non common range,
as pointed out from the github issue:
> ... the implementation only needs to return the value of ranges::next
and does not need to obtain the value through ranges::advance, which will have
O(n) complexity in the case of random-access-sized but non-common range.
---
.../range.adaptors/range.drop/begin.pass.cpp | 38 +++++++++++++++++++
.../ranges/range.adaptors/range.drop/types.h | 32 ++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
index cff088453d226e..138667dc5abd0d 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
@@ -81,6 +81,44 @@ constexpr bool test() {
static_assert(!BeginInvocable<const ForwardView>);
+ {
+ // non-common non-simple view,
+ // The wording of the standard is:
+ // Returns: ranges::next(ranges::begin(base_), count_, ranges::end(base_))
+ // Note that "Returns" is used here, meaning that we don't have to do it this way.
+ // In fact, this will use ranges::advance that has O(n) on non-common range.
+ // but [range.range] requires "amortized constant time" for ranges::begin and ranges::end
+ // Here, we test that begin() is indeed constant time, by creating a customized
+ // sentinel and counting how many times the sentinel eq function is called.
+ // It should be 0 times, but since this test (or any test under libcxx/test/std) is
+ // also used by other implementations, we relax the condition to that
+ // sentinel_cmp_calls is a constant number.
+ int sentinel_cmp_calls_1 = 0;
+ int sentinel_cmp_calls_2 = 0;
+ using NonCommonView = MaybeSimpleNonCommonView<false>;
+ static_assert(std::ranges::random_access_range<NonCommonView>);
+ static_assert(std::ranges::sized_range<NonCommonView>);
+ std::ranges::drop_view dropView9_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
+ std::ranges::drop_view dropView9_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
+ assert(dropView9_1.begin() == globalBuff + 4);
+ assert(dropView9_2.begin() == globalBuff + 6);
+ assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
+ }
+
+ {
+ // non-common simple view, same as above.
+ int sentinel_cmp_calls_1 = 0;
+ int sentinel_cmp_calls_2 = 0;
+ using NonCommonView = MaybeSimpleNonCommonView<true>;
+ static_assert(std::ranges::random_access_range<NonCommonView>);
+ static_assert(std::ranges::sized_range<NonCommonView>);
+ std::ranges::drop_view dropView10_1(NonCommonView{{}, 0, &sentinel_cmp_calls_1}, 4);
+ std::ranges::drop_view dropView10_2(NonCommonView{{}, 0, &sentinel_cmp_calls_2}, 6);
+ assert(dropView10_1.begin() == globalBuff + 4);
+ assert(dropView10_2.begin() == globalBuff + 6);
+ assert(sentinel_cmp_calls_1 == sentinel_cmp_calls_2);
+ }
+
{
static_assert(std::ranges::random_access_range<const SimpleView>);
static_assert(std::ranges::sized_range<const SimpleView>);
diff --git a/libcxx/test/std/ranges/range.adaptors/range.drop/types.h b/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
index 32bbddc05ed973..1fc3f05bf5eaab 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
+++ b/libcxx/test/std/ranges/range.adaptors/range.drop/types.h
@@ -14,6 +14,38 @@
int globalBuff[8];
+template <class T>
+struct sentinel {
+ T* ptr_;
+ int* num_of_sentinel_cmp_calls;
+
+public:
+ friend constexpr bool operator==(sentinel const s, T* const ptr) noexcept {
+ ++(*s.num_of_sentinel_cmp_calls);
+ return {s.ptr_ == ptr};
+ }
+ friend constexpr bool operator==(T* const ptr, sentinel const s) noexcept {
+ ++(*s.num_of_sentinel_cmp_calls);
+ return {s.ptr_ == ptr};
+ }
+ friend constexpr bool operator!=(sentinel const s, T* const ptr) noexcept { return !(s == ptr); }
+ friend constexpr bool operator!=(T* const ptr, sentinel const s) noexcept { return !(s == ptr); }
+};
+
+template <bool IsSimple>
+struct MaybeSimpleNonCommonView : std::ranges::view_base {
+ int start_;
+ int* num_of_sentinel_cmp_calls;
+ constexpr std::size_t size() const { return 8; }
+ constexpr int* begin() { return globalBuff + start_; }
+ constexpr std::conditional_t<IsSimple, int*, const int*> begin() const { return globalBuff + start_; }
+ constexpr sentinel<int> end() { return sentinel<int>{globalBuff + size(), num_of_sentinel_cmp_calls}; }
+ constexpr auto end() const {
+ return std::conditional_t<IsSimple, sentinel<int>, sentinel<const int>>{
+ globalBuff + size(), num_of_sentinel_cmp_calls};
+ }
+};
+
struct MoveOnlyView : std::ranges::view_base {
int start_;
constexpr explicit MoveOnlyView(int start = 0) : start_(start) {}
>From e2ec7a439a12530f343e5c56cb39ff9be079d1af Mon Sep 17 00:00:00 2001
From: Hongyu Ouyang <HongyuOuyang at proton.me>
Date: Thu, 14 Dec 2023 14:13:47 -0800
Subject: [PATCH 2/3] Update comment (remove zero width space)
---
libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
index 138667dc5abd0d..8c28769acf7fbf 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.drop/begin.pass.cpp
@@ -84,7 +84,7 @@ constexpr bool test() {
{
// non-common non-simple view,
// The wording of the standard is:
- // Returns: ranges::next(ranges::begin(base_), count_, ranges::end(base_))
+ // Returns: ranges::next(ranges::begin(base_), count_, ranges::end(base_))
// Note that "Returns" is used here, meaning that we don't have to do it this way.
// In fact, this will use ranges::advance that has O(n) on non-common range.
// but [range.range] requires "amortized constant time" for ranges::begin and ranges::end
>From 9bdf7bb9806ae26257de68bc47ed2c2a7fde5481 Mon Sep 17 00:00:00 2001
From: Hongyu Ouyang <HongyuOuyang at proton.me>
Date: Thu, 21 Dec 2023 13:29:35 -0800
Subject: [PATCH 3/3] rebase
---
libcxx/include/__ranges/drop_view.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index 2b89c6877a7896..83bb598b0a0c91 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -90,6 +90,10 @@ class drop_view : public view_interface<drop_view<_View>> {
_LIBCPP_HIDE_FROM_ABI constexpr auto begin()
requires(!(__simple_view<_View> && random_access_range<const _View> && sized_range<const _View>))
{
+ if constexpr (random_access_range<_View> && sized_range<_View>) {
+ const auto __dist = std::min(ranges::distance(__base_), __count_);
+ return ranges::begin(__base_) + __dist;
+ }
if constexpr (_UseCache)
if (__cached_begin_.__has_value())
return *__cached_begin_;
@@ -103,7 +107,8 @@ class drop_view : public view_interface<drop_view<_View>> {
_LIBCPP_HIDE_FROM_ABI constexpr auto begin() const
requires random_access_range<const _View> && sized_range<const _View>
{
- return ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
+ const auto __dist = std::min(ranges::distance(__base_), __count_);
+ return ranges::begin(__base_) + __dist;
}
_LIBCPP_HIDE_FROM_ABI constexpr auto end()
More information about the libcxx-commits
mailing list