From 66f9bda923a0c900ab3d0d0c70c7c2f778258403 Mon Sep 17 00:00:00 2001 From: Marcus Boerger Date: Thu, 29 Feb 2024 17:16:14 +0100 Subject: [PATCH] * Optimize `AnyScan`, `ConstScan` and `ConvertingScan` by dropping clone layer. We also explicitly support multiple iterations on one object. (#16) --- CHANGELOG.md | 4 + mbo/container/any_scan.h | 154 +++++++++++++++++++++------------------ 2 files changed, 88 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d933114..3ec25a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.2.21 + +* Optimize `AnyScan`, `ConstScan` and `ConvertingScan` by dropping clone layer. We also explicitly support multiple iterations on one object. + # 0.2.20 * Add `empty` and `size` to `AnyScan`, `ConstScan` and `ConvertingScan`. diff --git a/mbo/container/any_scan.h b/mbo/container/any_scan.h index 3ba10b5..84cc55c 100644 --- a/mbo/container/any_scan.h +++ b/mbo/container/any_scan.h @@ -260,24 +260,39 @@ class MakeAnyScanData { template class AnyScanImpl { + public: + using difference_type = DifferenceType; + using element_type = std::remove_cvref_t; + using value_type = ValueType; + using pointer = ValueType*; + using const_pointer = const ValueType*; + using reference = ValueType&; + using const_reference = const ValueType&; + + AnyScanImpl() = delete; + private: static constexpr ScanMode kScanMode = ScanModeVal; static constexpr bool kAccessByRef = kScanMode != ScanMode::kConverting; using AccessType = std::conditional_t; - using FuncSave = std::function; using FuncMore = std::function; using FuncCurr = std::function; using FuncNext = std::function; - using FuncZero = std::function; - using FuncSize = std::function; - struct AccessFuncs { - FuncSave save; + struct IterFuncs { FuncMore more; FuncCurr curr; FuncNext next; + }; + + using FuncIter = std::function; + using FuncZero = std::function; + using FuncSize = std::function; + + struct AccessFuncs { + FuncIter iter; FuncZero empty; FuncSize size; }; @@ -332,37 +347,25 @@ class AnyScanImpl { return std::shared_ptr(new Iterator(container.begin())); } - public: - using difference_type = DifferenceType; - using element_type = std::remove_cvref_t; - using value_type = ValueType; - using pointer = ValueType*; - using const_pointer = const ValueType*; - using reference = ValueType&; - using const_reference = const ValueType&; - - AnyScanImpl() = delete; - - private: // NOLINTBEGIN(bugprone-forwarding-reference-overload) // For MakAnyScan / MakeConstScan template requires(kAccessByRef) explicit AnyScanImpl(MakeAnyScanData data) - : clone_([data = std::move(data)]() { - using IteratorValueType = decltype(*std::declval>()); - CheckCompatible(); - auto pos = MakeSharedIterator(data.container()); - return std::make_shared(AccessFuncs{ - .save = [save = pos] {}, - .more = [&data = data, &it = *pos]() -> bool { return it != data.container().end(); }, - .curr = [&it = *pos]() -> AccessType { return *it; }, - .next = [&it = *pos] { ++it; }, - .empty = [&data = data] { return data.empty(); }, - .size = [&data = data] { return data.size(); }, - }); - }) {} + : funcs_{ + .iter = + [data = data] { + auto pos = MakeSharedIterator(data.container()); + return IterFuncs{ + .more = [data = data, pos = pos]() -> bool { return *pos != data.container().end(); }, + .curr = [&it = *pos]() -> AccessType { return *it; }, + .next = [&it = *pos] { ++it; }, + }; + }, + .empty = [data = data] { return data.empty(); }, + .size = [data = data] { return data.size(); }, + } {} // For MakConvertingScan template @@ -371,17 +374,20 @@ class AnyScanImpl { && std::constructible_from> && std::constructible_from) explicit AnyScanImpl(MakeAnyScanData data) - : clone_([data = std::move(data)] { - auto pos = MakeSharedIterator(data.container()); - return std::make_shared(AccessFuncs{ - .save = [pos = pos] {}, - .more = [&data = data, &it = *pos]() -> bool { return it != data.container().end(); }, - .curr = [&it = *pos]() -> AccessType { return AccessType(*it); }, - .next = [&it = *pos] { ++it; }, - .empty = [&data = data] { return data.empty(); }, - .size = [&data = data] { return data.size(); }, - }); - }) {} + : funcs_{ + // NOTE: data must be copied here! + .iter = + [data = data] { + auto pos = MakeSharedIterator(data.container()); + return IterFuncs{ + .more = [data = data, pos = pos]() -> bool { return *pos != data.container().end(); }, + .curr = [&it = *pos]() -> AccessType { return AccessType(*it); }, + .next = [&it = *pos] { ++it; }, + }; + }, + .empty = [data = data] { return data.empty(); }, + .size = [data = data] { return data.size(); }, + } {} // NOLINTEND(bugprone-forwarding-reference-overload) @@ -396,9 +402,14 @@ class AnyScanImpl { using pointer = ItPointer; using reference = ItReference; - iterator_impl() : funcs_(nullptr) {} + iterator_impl() + : funcs_{ + .more = [] { return false; }, + .curr = nullptr, + .next = [] {}, + } {} - explicit iterator_impl(std::shared_ptr funcs) : funcs_(std::move(funcs)) {} + explicit iterator_impl(const AccessFuncs& funcs) : funcs_(funcs.iter()) {} reference operator*() const noexcept requires kAccessByRef @@ -407,50 +418,53 @@ class AnyScanImpl { // That means we bypass any protection an iterator may have, but we can make this function // `noexcept` assuming the iterator is noexcept for access. On the other hand we expect that // out of bounds access may actually raise. So we effectively side step such exceptions. - ABSL_CHECK(funcs_ && funcs_->more()); - return funcs_->curr(); + ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); + return funcs_.curr(); } value_type operator*() const noexcept requires(!kAccessByRef) { - ABSL_CHECK(funcs_ && funcs_->more()); - return value_type(funcs_->curr()); + ABSL_CHECK(funcs_.curr != nullptr && funcs_.more()); + return value_type(funcs_.curr()); } const_pointer operator->() const noexcept requires kAccessByRef { - return (funcs_ && funcs_->more()) ? &funcs_->curr() : nullptr; + return funcs_.curr != nullptr && funcs_.more() ? &funcs_.curr() : nullptr; } iterator_impl& operator++() noexcept { - if (funcs_) { - funcs_->next(); - } + funcs_.next(); return *this; } iterator_impl operator++(int) noexcept { // NOLINT(cert-dcl21-cpp) iterator_impl result = *this; - if (funcs_) { - funcs_->next(); - } + funcs_.next(); return result; } template bool operator==(const iterator_impl& other) const noexcept { - if (this == &other || funcs_ == other.funcs_) { + if (this == &other) { return true; } - if (funcs_ == nullptr && !other.funcs_->more()) { - return true; + const bool l_end = !funcs_.more(); + const bool o_end = !other.funcs_.more(); + // No need to also check `funcs_.curr != nullptr`, as then `more` would have returned false. + if (l_end || o_end) { + return l_end == o_end; } - if (other.funcs_ == nullptr && !funcs_->more()) { - return true; + if constexpr (kAccessByRef) { + // Approximate equal address means equal iterator. That is not always correct as a container + // might have the same element reference twice. + return &funcs_.curr() != &other.funcs_.curr(); + } else { + // Cannot take address of rvalue temp. Comparing actual values would be incorrect. + return false; } - return false; } template @@ -459,30 +473,30 @@ class AnyScanImpl { } private: - std::shared_ptr funcs_; // Cannot be const. + IterFuncs funcs_; // Cannot be const. }; using iterator = iterator_impl; using const_iterator = iterator_impl; - iterator begin() noexcept { return iterator(std::move(clone_())); } + iterator begin() noexcept { return iterator(funcs_); } - iterator end() noexcept { return iterator(nullptr); } + iterator end() noexcept { return iterator(); } - const_iterator begin() const noexcept { return const_iterator(std::move(clone_())); } + const_iterator begin() const noexcept { return const_iterator(funcs_); } - const_iterator end() const noexcept { return const_iterator(nullptr); } + const_iterator end() const noexcept { return const_iterator(); } - const_iterator cbegin() const noexcept { return const_iterator(std::move(clone_())); } + const_iterator cbegin() const noexcept { return const_iterator(funcs_); } - const_iterator cend() const noexcept { return const_iterator(nullptr); } + const_iterator cend() const noexcept { return const_iterator(); } - bool empty() const noexcept { return clone_()->empty(); } + bool empty() const noexcept { return funcs_.empty(); } - std::size_t size() const noexcept { return clone_()->size(); } + std::size_t size() const noexcept { return funcs_.size(); } private: - const FuncClone clone_; + AccessFuncs funcs_; }; } // namespace container_internal