From 7dbde7f659f33ec36c8f4a533e046fa2b8bd2bc5 Mon Sep 17 00:00:00 2001 From: TheCoconutChef Date: Sat, 24 Aug 2024 12:08:38 -0400 Subject: [PATCH] allow forward declaration of values held by reader This commit enables the forward declaration of a type held by a reader without any new memory allocation or virtual methods. This is achieved by creating a new class -- observable_reader_node -- in the reader_node hierarchy which acts as a view for the type held by reader_node. Performance Impact Ad-hoc benchmarks (i.e. not in commit) were performed with nanobench. (https://nanobench.ankerl.com/index.html) Compared to baseline, the impact on a reader get method was non-existent. The same was true for map. The impact on watch seemingly ranged from indiscernible to rather small, i.e. in the order of 0.5-1.5%, although why there even is an impact isn't clear. Downsides and Limitations 1. If it matters, the protected keyword has now been introduced within the reader node class hierarchy. This is because the link method and the observers access method had to be moved in observable_reader_node to avoid virtual method declaration. 2. While the values held by a reader are now fwd declarable, those held by a cursor are not. This arguably introduces an inconsistency in the API. --- lager/detail/nodes.hpp | 92 ++++++++++++++++++++++++++++-------------- lager/reader.hpp | 4 +- test/cursor.cpp | 23 +++++++++++ 3 files changed, 86 insertions(+), 33 deletions(-) diff --git a/lager/detail/nodes.hpp b/lager/detail/nodes.hpp index 87ed89b8..b0bf68e3 100644 --- a/lager/detail/nodes.hpp +++ b/lager/detail/nodes.hpp @@ -128,26 +128,19 @@ struct notifying_guard_t }; /*! - * Base class for the various node types. Provides basic - * functionality for setting values and propagating them to children. + * Interface for nodes capable of notifying observers. */ template -class reader_node : public reader_node_base +class observable_reader_node { public: using value_type = T; using signal_type = signal; - reader_node(T value) - : current_(std::move(value)) - , last_(current_) - {} - - virtual void recompute() = 0; - virtual void refresh() = 0; + virtual void refresh() = 0; - const value_type& current() const { return current_; } - const value_type& last() const { return last_; } + const value_type& current() const { return *current_view_; } + const value_type& last() const { return *last_view_; } void link(std::weak_ptr child) { @@ -159,6 +152,56 @@ class reader_node : public reader_node_base "Child node must not be linked twice"); children_.push_back(child); } + auto observers() -> signal_type& { return observers_; } + +protected: + observable_reader_node(const T* current, const T* last) + : current_view_(current) + , last_view_(last) + { + } + + void collect() + { + using namespace std; + children_.erase(remove_if(begin(children_), + end(children_), + mem_fn(&weak_ptr::expired)), + end(children_)); + } + + const std::vector>& children() const + { + return children_; + } + +private: + const T* current_view_; + const T* last_view_; + signal_type observers_; + std::vector> children_; +}; +/*! + * Base class for the various node types. Provides basic + * functionality for setting values and propagating them to children. + */ +template +class reader_node + : public reader_node_base + , public observable_reader_node +{ +public: + using value_type = typename observable_reader_node::value_type; + using signal_type = typename observable_reader_node::signal_type; + + reader_node(T value) + : observable_reader_node(¤t_, &last_) + , current_(std::move(value)) + , last_(current_) + { + } + + virtual void recompute() = 0; template void push_down(U&& value) @@ -171,12 +214,12 @@ class reader_node : public reader_node_base void send_down() final { - recompute(); + this->recompute(); if (needs_send_down_) { last_ = current_; needs_send_down_ = false; needs_notify_ = true; - for (auto& wchild : children_) { + for (auto& wchild : this->children()) { if (auto child = wchild.lock()) { child->send_down(); } @@ -193,9 +236,9 @@ class reader_node : public reader_node_base notifying_guard_t notifying_guard(notifying_); bool garbage = false; - observers_(last_); - for (size_t i = 0, size = children_.size(); i < size; ++i) { - if (auto child = children_[i].lock()) { + this->observers()(last_); + for (auto& wchild : this->children()) { + if (auto child = wchild.lock()) { child->notify(); } else { garbage = true; @@ -203,27 +246,14 @@ class reader_node : public reader_node_base } if (garbage && !notifying_guard.value_) { - collect(); + this->collect(); } } } - auto observers() -> signal_type& { return observers_; } - private: - void collect() - { - using namespace std; - children_.erase(remove_if(begin(children_), - end(children_), - mem_fn(&weak_ptr::expired)), - end(children_)); - } - value_type current_; value_type last_; - std::vector> children_; - signal_type observers_; bool needs_send_down_ = false; bool needs_notify_ = false; diff --git a/lager/reader.hpp b/lager/reader.hpp index 28a3c800..7b26e492 100644 --- a/lager/reader.hpp +++ b/lager/reader.hpp @@ -125,9 +125,9 @@ class reader_base * Provides access to reading values of type `T`. */ template -class reader : public reader_base> +class reader : public reader_base> { - using base_t = reader_base>; + using base_t = reader_base>; public: using base_t::base_t; diff --git a/test/cursor.cpp b/test/cursor.cpp index 7e807d86..bfdf764c 100644 --- a/test/cursor.cpp +++ b/test/cursor.cpp @@ -226,3 +226,26 @@ TEST_CASE("lenses over with expression") CHECK(person_data->name == "new name"); CHECK(name.get() == "new name"); } + +struct Foo; +struct Bar +{ + lager::reader foo; + reader get_reader() const; +}; +struct Baz +{ + Baz(const lager::reader& r); +}; +TEST_CASE("forward declare a reader value") +{ + auto bar = Bar(); + auto baz = Baz(bar.get_reader()); +} +struct Foo +{}; +lager::reader Bar::get_reader() const +{ + return lager::make_constant(Foo()); +} +Baz::Baz(const lager::reader&) {}