[libcxx-commits] [libcxx] [libc++] optimization on ranges::drop_view::begin (#72883) (PR #72929)
Hongyu Ouyang via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Dec 14 14:14:27 PST 2023
https://github.com/casavaca updated https://github.com/llvm/llvm-project/pull/72929
>From 2b39f2963d8d9711c8a793f8650384965bafef6d 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.
---
libcxx/include/__ranges/drop_view.h | 7 +++-
.../range.adaptors/range.drop/begin.pass.cpp | 38 +++++++++++++++++++
.../ranges/range.adaptors/range.drop/types.h | 32 ++++++++++++++++
3 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index f10476f0011e73..3ab7a9000cbaff 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<_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())
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_));
+ range_difference_t<_View> __dist = ranges::distance(__base_);
+ return ranges::begin(__base_) + std::min(__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 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 0adb33f30c8558386f231eda3e84ff3114482dd1 Mon Sep 17 00:00:00 2001
From: Hongyu Ouyang <HongyuOuyang at proton.me>
Date: Wed, 13 Dec 2023 16:17:29 -0800
Subject: [PATCH 2/3] Stylish change.
Use `const auto`, just as microsoft STL does in
https://github.com/microsoft/STL/blob/cf1313c39169dc376761eddee23c5e408e01aaa9/stl/inc/ranges#L3359
---
libcxx/include/__ranges/drop_view.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/libcxx/include/__ranges/drop_view.h b/libcxx/include/__ranges/drop_view.h
index 3ab7a9000cbaff..486c51edfed1fd 100644
--- a/libcxx/include/__ranges/drop_view.h
+++ b/libcxx/include/__ranges/drop_view.h
@@ -91,8 +91,8 @@ namespace ranges {
random_access_range<const _View> && sized_range<const _View>))
{
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);
+ const auto __dist = std::min(ranges::distance(__base_), __count_);
+ return ranges::begin(__base_) + __dist;
}
if constexpr (_UseCache)
if (__cached_begin_.__has_value())
@@ -108,8 +108,8 @@ namespace ranges {
constexpr auto begin() const
requires random_access_range<const _View> && sized_range<const _View>
{
- range_difference_t<_View> __dist = ranges::distance(__base_);
- return ranges::begin(__base_) + std::min(__count_, __dist);
+ const auto __dist = std::min(ranges::distance(__base_), __count_);
+ return ranges::begin(__base_) + __dist;
}
_LIBCPP_HIDE_FROM_ABI
>From a47418041343baf1edd6725bd4bab684c9a7d95f 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 3/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
More information about the libcxx-commits
mailing list