Skip to content

Commit

Permalink
allow forward declaration of values held by reader
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
TheCoconutChef committed Sep 9, 2024
1 parent 503ea6a commit 7dbde7f
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 33 deletions.
92 changes: 61 additions & 31 deletions lager/detail/nodes.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <typename T>
class reader_node : public reader_node_base
class observable_reader_node
{
public:
using value_type = T;
using signal_type = signal<const value_type&>;

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<reader_node_base> child)
{
Expand All @@ -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<reader_node_base>::expired)),
end(children_));
}

const std::vector<std::weak_ptr<reader_node_base>>& children() const
{
return children_;
}

private:
const T* current_view_;
const T* last_view_;
signal_type observers_;
std::vector<std::weak_ptr<reader_node_base>> children_;
};
/*!
* Base class for the various node types. Provides basic
* functionality for setting values and propagating them to children.
*/
template <typename T>
class reader_node
: public reader_node_base
, public observable_reader_node<T>
{
public:
using value_type = typename observable_reader_node<T>::value_type;
using signal_type = typename observable_reader_node<T>::signal_type;

reader_node(T value)
: observable_reader_node<T>(&current_, &last_)
, current_(std::move(value))
, last_(current_)
{
}

virtual void recompute() = 0;

template <typename U>
void push_down(U&& value)
Expand All @@ -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();
}
Expand All @@ -193,37 +236,24 @@ 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;
}
}

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<reader_node_base>::expired)),
end(children_));
}

value_type current_;
value_type last_;
std::vector<std::weak_ptr<reader_node_base>> children_;
signal_type observers_;

bool needs_send_down_ = false;
bool needs_notify_ = false;
Expand Down
4 changes: 2 additions & 2 deletions lager/reader.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ class reader_base
* Provides access to reading values of type `T`.
*/
template <typename T>
class reader : public reader_base<detail::reader_node<T>>
class reader : public reader_base<detail::observable_reader_node<T>>
{
using base_t = reader_base<detail::reader_node<T>>;
using base_t = reader_base<detail::observable_reader_node<T>>;

public:
using base_t::base_t;
Expand Down
23 changes: 23 additions & 0 deletions test/cursor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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> foo;
reader<Foo> get_reader() const;
};
struct Baz
{
Baz(const lager::reader<Foo>& r);
};
TEST_CASE("forward declare a reader value")
{
auto bar = Bar();
auto baz = Baz(bar.get_reader());
}
struct Foo
{};
lager::reader<Foo> Bar::get_reader() const
{
return lager::make_constant(Foo());
}
Baz::Baz(const lager::reader<Foo>&) {}

0 comments on commit 7dbde7f

Please sign in to comment.