[libcxx-commits] [libcxx] [libc++] optimization on ranges::drop_view::begin (#72883) (PR #72929)
Hongyu Ouyang via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Nov 21 14:57:07 PST 2023
https://github.com/casavaca updated https://github.com/llvm/llvm-project/pull/72929
>From 081f76d1710b456b175078eb831e643c6fa71d05 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/2] [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.
---
libcxx/include/__ranges/drop_view.h | 7 +++-
.../range.adaptors/range.drop/begin.pass.cpp | 30 +++++++++++++++++
.../ranges/range.adaptors/range.drop/types.h | 32 +++++++++++++++++++
3 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index f10476f0011e739..9e4b530ff105b7a 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -90,6 +90,10 @@ namespace ranges {
requires (!(__simple_view<_View> &&
random_access_range<const _View> && sized_range<const _View>))
{
+ if constexpr (random_access_range<const _View> && sized_range<const _View>) {
+ auto __dist = ranges::distance(__base_);
+ return ranges::begin(__base_) + std::min<range_difference_t<_View>>(__count_, __dist);
+ }
if constexpr (_UseCache)
if (__cached_begin_.__has_value())
return *__cached_begin_;
@@ -104,7 +108,8 @@ namespace ranges {
constexpr auto begin() const
requires random_access_range<const _View> && sized_range<const _View>
{
- return ranges::next(ranges::begin(__base_), __count_, ranges::end(__base_));
+ auto __dist = ranges::distance(__base_);
+ return ranges::begin(__base_) + std::min<range_difference_t<_View>>(__count_, __dist);
}
_LIBCPP_HIDE_FROM_ABI
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 cff088453d226ef..18c1bd8a53c61e8 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,36 @@ constexpr bool test() {
static_assert(!BeginInvocable<const ForwardView>);
+ {
+ // non-common non-simple view,
+ // random_access_range<const V> && sized_range<const V>
+ // 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.
+ // Here, we test by counting how many times sentinel eq function is called.
+ // If ranges::advance is used, it will be called N times,
+ // but if we optimized it, it will be called 0 times.
+ int sentinel_cmp_calls = 0;
+ using NonCommonView = MaybeSimpleNonCommonView<false>;
+ static_assert(std::ranges::random_access_range<const NonCommonView>);
+ static_assert(std::ranges::sized_range<const NonCommonView>);
+ std::ranges::drop_view dropView9(NonCommonView{{}, 0, &sentinel_cmp_calls}, 4);
+ assert(dropView9.begin() == globalBuff + 4);
+ assert(sentinel_cmp_calls == 0);
+ }
+
+ {
+ // non-common simple view, same as above.
+ int sentinel_cmp_calls = 0;
+ using NonCommonView = MaybeSimpleNonCommonView<true>;
+ static_assert(std::ranges::random_access_range<const NonCommonView>);
+ static_assert(std::ranges::sized_range<const NonCommonView>);
+ std::ranges::drop_view dropView9(NonCommonView{{}, 0, &sentinel_cmp_calls}, 4);
+ assert(dropView9.begin() == globalBuff + 4);
+ assert(sentinel_cmp_calls == 0);
+ }
+
{
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 32bbddc05ed973a..1fc3f05bf5eaab3 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 75d46d7471f0711b8ad343768a5112660fbd9e1d Mon Sep 17 00:00:00 2001
From: Hongyu Ouyang <HongyuOuyang at proton.me>
Date: Tue, 21 Nov 2023 14:55:50 -0800
Subject: [PATCH 2/2] Fixes and address feedbacks from @huixie90
---
libcxx/include/__ranges/drop_view.h | 6 +--
.../range.adaptors/range.drop/begin.pass.cpp | 44 +++++++++++--------
2 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index 9e4b530ff105b7a..cec0c3637625a74 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -90,9 +90,9 @@ namespace ranges {
requires (!(__simple_view<_View> &&
random_access_range<const _View> && sized_range<const _View>))
{
- if constexpr (random_access_range<const _View> && sized_range<const _View>) {
- auto __dist = ranges::distance(__base_);
- return ranges::begin(__base_) + std::min<range_difference_t<_View>>(__count_, __dist);
+ if constexpr (random_access_range<_View> && sized_range<_View>) {
+ range_difference_t<_View> __dist = ranges::distance(__base_);
+ return ranges::begin(__base_) + std::min(__count_, __dist);
}
if constexpr (_UseCache)
if (__cached_begin_.__has_value())
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 18c1bd8a53c61e8..995b6979179c0e9 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
@@ -83,32 +83,40 @@ constexpr bool test() {
{
// non-common non-simple view,
- // random_access_range<const V> && sized_range<const V>
// 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.
- // Here, we test by counting how many times sentinel eq function is called.
- // If ranges::advance is used, it will be called N times,
- // but if we optimized it, it will be called 0 times.
- int sentinel_cmp_calls = 0;
- using NonCommonView = MaybeSimpleNonCommonView<false>;
- static_assert(std::ranges::random_access_range<const NonCommonView>);
- static_assert(std::ranges::sized_range<const NonCommonView>);
- std::ranges::drop_view dropView9(NonCommonView{{}, 0, &sentinel_cmp_calls}, 4);
- assert(dropView9.begin() == globalBuff + 4);
- assert(sentinel_cmp_calls == 0);
+ // 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
+ // sentinal and counting how many times the sentinal 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 = 0;
- using NonCommonView = MaybeSimpleNonCommonView<true>;
- static_assert(std::ranges::random_access_range<const NonCommonView>);
- static_assert(std::ranges::sized_range<const NonCommonView>);
- std::ranges::drop_view dropView9(NonCommonView{{}, 0, &sentinel_cmp_calls}, 4);
- assert(dropView9.begin() == globalBuff + 4);
- assert(sentinel_cmp_calls == 0);
+ 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);
}
{
More information about the libcxx-commits
mailing list