diff --git a/docs/STATE_EXEC_DEVELOPER_GUIDE.md b/docs/STATE_EXEC_DEVELOPER_GUIDE.md index 048aa8e28..0cf422baf 100644 --- a/docs/STATE_EXEC_DEVELOPER_GUIDE.md +++ b/docs/STATE_EXEC_DEVELOPER_GUIDE.md @@ -1372,7 +1372,8 @@ created (e.g. via `state-set` or `cvc::state::operator()(path).value(v)`), the internal sequence is: 1. `valueChanged()` fires on the new node — **no observer yet**. -2. `childChanged()` propagates up from parent to root. +2. `childChanged()` fires on the parent and on every ancestor above it, + the root's slots first (`state::emitChildChanged()`). 3. The adapter's `childChanged` handler attaches an observer on the new node. diff --git a/docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md b/docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md index 7ee89baa6..ede4ccc94 100644 --- a/docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md +++ b/docs/roadmap/STATE_LIFETIME_AND_ATOMICITY.md @@ -155,7 +155,9 @@ leaf-pin does not keep alive: - `state_resolve` computes `"state://" + eff->fullName() + …` on **every** resolve (`uri_state.cpp:64`; `state_store` at `:97`). `fullName()` → `parentName()` (`state.h:227-239`) dereferences `_parent` (a raw `const state*`, `state.h:747`), - recursing up the chain. + recursing up the chain. (They are loops up the chain now, and so is the + `childChanged` notification of the ancestors, `state::emitChildChanged()`; the + `_parent` dereferences, and this hazard, are unchanged.) - `effective_node`/`resolveLink` also call `fullName()` on the start and target nodes (`state.cpp:1016,1047`). diff --git a/inc/cvc/state/state.h b/inc/cvc/state/state.h index 459caf86b..14acd1b60 100644 --- a/inc/cvc/state/state.h +++ b/inc/cvc/state/state.h @@ -232,20 +232,13 @@ class state : public boost::enable_shared_from_this { const std::string &name() const { return _name; } const state *parent() const { return _parent; } - // return's the parent's fullName - std::string parentName() const { - std::string tmp; - return parent() ? (!parent()->name().empty() ? ((tmp = parent()->parentName()).empty() - ? parent()->name() - : tmp + SEPARATOR + parent()->name()) - : "") - : ""; - } + // The parent's full name: the names of the ancestors from just below the first one with an + // empty name (normally the root) down to the parent, joined with SEPARATOR; "" when there are + // none. Built in one pass over the _parent chain, not once per level. + std::string parentName() const; - std::string fullName() const { - std::string pn = parentName(); - return pn.empty() ? name() : pn + SEPARATOR + name(); - } + // This node's path from the root: parentName(), then its own name. + std::string fullName() const; boost::posix_time::ptime lastMod(); @@ -275,9 +268,6 @@ class state : public boost::enable_shared_from_this { template state &value(const T &v) { std::string str_value = boost::lexical_cast(v); - // Check if read-only before acquiring lock - std::string full_name = fullName(); - // Phase 8: writes through a writable transparent link route to the // resolved target, mirroring the string overload // (state::value(const std::string&, bool) in state.cpp). Routing to the @@ -305,7 +295,7 @@ class state : public boost::enable_shared_from_this { return *this; } throw read_only_error(boost::str( - boost::format("Cannot write through unresolvable transparent link: %1%") % full_name)); + boost::format("Cannot write through unresolvable transparent link: %1%") % fullName())); } } @@ -315,7 +305,7 @@ class state : public boost::enable_shared_from_this { // Check if this state is read-only if (_readOnly) { throw read_only_error( - boost::str(boost::format("Cannot modify read-only state: %1%") % full_name)); + boost::str(boost::format("Cannot modify read-only state: %1%") % fullName())); } if (_value == str_value) @@ -331,7 +321,7 @@ class state : public boost::enable_shared_from_this { valueChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -422,9 +412,23 @@ class state : public boost::enable_shared_from_this { signal dataChanged; state &operator()(const std::string &childname = std::string()); + // The full names of every node under this one (its descendants, in pre-order: each child, then + // its own descendants, children in name order), those matching `re` when it is not empty. std::vector children(const std::string &re = std::string()); size_t numChildren(); + + // Fired on a node when something under it changes, with that thing's path relative to the + // node. A change signals the changed node's parent and every ancestor above it, the root's + // slots first (emitChildChanged()). Emitting childChanged directly fires only that node's + // slots. map_change_signal childChanged; + + // Fire childChanged for `childname` (a path relative to this node) on this node and on each + // ancestor, with the path relative to that ancestor: the root's slots first, this node's last. + // That is the order the old relay (each node's first childChanged slot re-firing its parent's) + // produced, but this is a loop, so a deep tree costs no stack per level, and a path is built + // only for a node that has slots. + void emitChildChanged(const std::string &childname) const; operator std::string() { return value(); } signal destroyed; @@ -805,8 +809,6 @@ class state : public boost::enable_shared_from_this { protected: state(app &ctx, const std::string &n = std::string(), const state *p = NULL); - void notifyParent(const std::string &childname); - app &_ctx; boost::mutex _mutex; boost::posix_time::ptime _lastMod; diff --git a/src/cvc/state/state.cpp b/src/cvc/state/state.cpp index da3b94715..ee63b89da 100644 --- a/src/cvc/state/state.cpp +++ b/src/cvc/state/state.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -91,19 +92,117 @@ std::size_t next_startup_id() { state::state(app &ctx, const std::string &n, const state *p) : _ctx(ctx), _name(n), _parent(p), _lastMod(boost::posix_time::min_date_time), _hidden(false), _readOnly(false), _initialized(false) { - // This slot propagates child changes up to parents - childChanged.connect( - map_change_signal::slot_type(&state::notifyParent, this, boost::placeholders::_1)); + // Child changes reach the ancestors through emitChildChanged(), not a slot per node. } // ------------- // state::~state // ------------- // Purpose: -// Destructor. Just signals that this object has been destroyed. +// Destructor. Signals that this object has been destroyed, then destroys +// its subtree. // ---- Change History ---- // 02/18/2012 -- Joe R. -- Creation. -state::~state() { destroyed(); } +state::~state() { + destroyed(); + // Destroy the subtree without recursing once per level (~state -> child map -> ~state would + // overflow the stack on a deep tree). A child this node was the last owner of is taken out of + // its map onto a heap stack, depth first, and released only once its own children are gone, so + // every node still has its ancestors (its _parent chain, which a destroyed slot may walk) when it + // dies. A child someone else still owns (a pin) keeps its subtree. + struct pending { + state_ptr node; + bool opened; + }; + std::vector stack; + for (child_map::value_type &kv : _children) + if (kv.second) + stack.push_back({std::move(kv.second), false}); + _children.clear(); + while (!stack.empty()) { + pending &top = stack.back(); + if (!top.opened && top.node.use_count() == 1) { + top.opened = true; + child_map kids; + { + boost::mutex::scoped_lock lock(top.node->_mutex); + kids.swap(top.node->_children); + } + for (child_map::value_type &kv : kids) // may grow `stack`: `top` is not used after this + if (kv.second) + stack.push_back({std::move(kv.second), false}); + continue; + } + stack.pop_back(); // its children are gone (or someone else owns it): let it go + } +} + +// ------------------------------ +// state::parentName / fullName +// ------------------------------ +// One pass up the _parent chain, building the path once: not a call per level, each rebuilding +// the path above it (O(depth^2) characters, and a native frame per level). +std::string state::parentName() const { + boost::container::small_vector names; // nearest first + std::size_t len = 0; + for (const state *p = _parent; p && !p->_name.empty(); p = p->_parent) { + names.push_back(&p->_name); + len += p->_name.size() + 1; + } + std::string out; + out.reserve(len); + for (std::size_t i = names.size(); i-- > 0;) { + out += *names[i]; + if (i > 0) + out += SEPARATOR; + } + return out; +} + +std::string state::fullName() const { + std::string out = parentName(); + if (!out.empty()) + out += SEPARATOR; + out += _name; + return out; +} + +// ------------------------- +// state::emitChildChanged +// ------------------------- +// Each ancestor's path is a suffix of the one the highest ancestor with slots receives, so that +// one is built once and the others are cut from it. Ancestors without slots cost a pointer. +void state::emitChildChanged(const std::string &childname) const { + boost::this_thread::interruption_point(); + boost::container::small_vector chain; // this node, then its ancestors + std::size_t top = 0; // 1 + the index in `chain` of the highest node with slots; 0 if none + for (const state *n = this; n; n = n->_parent) { + chain.push_back(n); + if (!n->childChanged.empty()) + top = chain.size(); + } + if (top == 0) + return; + std::string path; // relative to chain[top - 1] + { + std::size_t len = childname.size(); + for (std::size_t j = 0; j + 1 < top; ++j) + len += chain[j]->_name.size() + 1; + path.reserve(len); + for (std::size_t j = top - 1; j-- > 0;) { + path += chain[j]->_name; + path += SEPARATOR; + } + path += childname; + } + std::size_t offset = 0; // where chain[j]'s relative path starts in `path` + for (std::size_t j = top; j-- > 0;) { + if (!chain[j]->childChanged.empty()) + chain[j]->childChanged(offset == 0 ? path : path.substr(offset)); + if (j > 0) + offset += chain[j - 1]->_name.size() + 1; + } +} // ------------------ // state::instancePtr @@ -268,7 +367,7 @@ state &state::comment(const std::string &c) { commentChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -307,7 +406,7 @@ state &state::hidden(bool h) { hiddenChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -346,7 +445,7 @@ state &state::readOnly(bool ro) { readOnlyChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -395,9 +494,6 @@ std::vector state::values(bool unique) { state &state::value(const std::string &v, bool setValueType) { boost::this_thread::interruption_point(); - // Get fullName before locking - std::string full_name = fullName(); - // Phase 8: writes through a writable transparent link route to the // resolved target. Opaque links and non-writable transparent links // accept writes on the link node itself (the historical default). @@ -418,7 +514,7 @@ state &state::value(const std::string &v, bool setValueType) { return *this; } throw read_only_error(boost::str( - boost::format("Cannot write through unresolvable transparent link: %1%") % full_name)); + boost::format("Cannot write through unresolvable transparent link: %1%") % fullName())); } } @@ -427,7 +523,7 @@ state &state::value(const std::string &v, bool setValueType) { boost::mutex::scoped_lock lock(_mutex); if (_readOnly) { throw read_only_error( - boost::str(boost::format("Cannot modify read-only state: %1%") % full_name)); + boost::str(boost::format("Cannot modify read-only state: %1%") % fullName())); } } @@ -449,7 +545,7 @@ state &state::value(const std::string &v, bool setValueType) { valueChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -469,7 +565,7 @@ void state::touch() { valueChanged(); dataChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } // ------------ @@ -665,9 +761,6 @@ boost::any state::data() { state &state::data(const boost::any &d) { boost::this_thread::interruption_point(); - // Get fullName before locking - std::string full_name = fullName(); - // Phase 8: writes through a writable transparent link route to the // resolved target, mirroring state::value(const std::string&, bool). // Opaque links and non-writable transparent links accept writes on the @@ -691,7 +784,7 @@ state &state::data(const boost::any &d) { return *this; } throw read_only_error(boost::str( - boost::format("Cannot write through unresolvable transparent link: %1%") % full_name)); + boost::format("Cannot write through unresolvable transparent link: %1%") % fullName())); } } @@ -700,7 +793,7 @@ state &state::data(const boost::any &d) { boost::mutex::scoped_lock lock(_mutex); if (_readOnly) { throw read_only_error( - boost::str(boost::format("Cannot modify read-only state: %1%") % full_name)); + boost::str(boost::format("Cannot modify read-only state: %1%") % fullName())); } } @@ -715,7 +808,7 @@ state &state::data(const boost::any &d) { } dataChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); return *this; } @@ -737,51 +830,42 @@ std::string state::dataTypeName() { return _ctx.dataTypeName(data()); } // 02/18/2012 -- Joe R. -- Creation. // 03/15/2012 -- Joe R. -- Added initialized flag. state &state::operator()(const std::string &childname) { - using namespace std; using namespace boost::algorithm; boost::this_thread::interruption_point(); - vector keys; + std::vector keys; split(keys, childname, is_any_of(SEPARATOR)); - if (keys.empty()) - return *this; - BOOST_FOREACH (string &key, keys) + // One hop per key, creating what is missing. Each create-or-get is atomic under that node's + // own lock, released before the next hop (holding it for the whole descent once serialized + // every lookup on the root's mutex), and each hop is held as a state_ptr, so a concurrent + // reset(false) dropping it from its parent's map cannot pull it out from under the walk. A loop: + // this was a call per hop that re-joined the rest of the path each time (per-level recursion, + // quadratic in the path's length). Empty segments are skipped. + state *cur = this; + state_ptr held; // keeps `cur` alive from the first hop on + for (std::string &key : keys) { trim(key); - // Ignore beginning empty keys - while (!keys.empty() && keys.front().empty()) - keys.erase(keys.begin()); - if (keys.empty()) - return *this; - - string nearest = keys.front(); - keys.erase(keys.begin()); - // Resolve (or create) the immediate child under our own lock, then - // RELEASE that lock before recursing into it. - // - // The recursive call used to be made with _mutex still held, so walking - // "a.b.c" from the root kept the root's lock for the entire descent. - // Every path lookup in the app therefore serialized on the root mutex, - // and a 100-deep path held 100 locks at once. Only the create-or-get on - // this node's map has to be atomic, and it still is. - // - // Holding the child as a state_ptr across the recursion also keeps it - // alive for the duration, so a concurrent reset(false) dropping it from - // this node's map can no longer pull it out from under the descent. - state_ptr child; - { - boost::mutex::scoped_lock lock(_mutex); - auto it = _children.find(nearest); - if (it != _children.end() && it->second) { - child = it->second; - } else { - child.reset(new state(_ctx, nearest, this)); - _children[nearest] = child; - _lastMod = boost::posix_time::microsec_clock::universal_time(); - _initialized = true; + if (key.empty()) + continue; + boost::this_thread::interruption_point(); + state_ptr child; + { + boost::mutex::scoped_lock lock(cur->_mutex); + auto it = cur->_children.find(key); + if (it != cur->_children.end() && it->second) { + child = it->second; + } else { + child.reset(new state(cur->_ctx, key, cur)); + cur->_children[key] = child; + cur->_lastMod = boost::posix_time::microsec_clock::universal_time(); + cur->_initialized = true; + } } + held = std::move(child); + cur = held.get(); } - return (*child)(join(keys, SEPARATOR)); + return *cur; } // state::sharedChild @@ -886,7 +970,7 @@ state &state::linkTo(const std::string &target_path) { if (changed) { linkChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } return *this; } @@ -906,7 +990,7 @@ bool state::clearLink() { if (was_link) { linkChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } return was_link; } @@ -932,7 +1016,7 @@ state &state::linkTo(const std::string &target_path, link_mode mode) { if (changed) { linkChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } return *this; } @@ -955,7 +1039,7 @@ state &state::setLinkMode(link_mode mode) { if (changed) { linkChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } return *this; } @@ -978,7 +1062,7 @@ state &state::setLinkWritable(bool writable) { if (changed) { linkChanged(); if (parent()) - parent()->childChanged(name()); + parent()->emitChildChanged(name()); } return *this; } @@ -1348,34 +1432,59 @@ state::send_message_result state::sendMessage(const std::string &payload, // 01/12/2014 -- Joe R. -- Returning empty vector if invalid regex. std::vector state::children(const std::string &re) { boost::this_thread::interruption_point(); - boost::mutex::scoped_lock lock(_mutex); std::vector ret; - BOOST_FOREACH (child_map::value_type val, _children) { - if (!re.empty()) { - try { - boost::regex expression(re.c_str()); - boost::cmatch what; - - _ctx.log(6, boost::str(boost::format("%s :: check match %s\n") % BOOST_CURRENT_FUNCTION % - val.second->fullName())); - - if (boost::regex_match(val.second->fullName().c_str(), what, expression)) { - _ctx.log(6, boost::str(boost::format("%s :: matched! %s\n") % BOOST_CURRENT_FUNCTION % - val.second->fullName())); + boost::regex expression; + if (!re.empty()) { + try { + expression = boost::regex(re.c_str()); + } catch (boost::bad_expression &) { + _ctx.log( + 2, boost::str(boost::format("%s :: invalid regex '%s'\n") % BOOST_CURRENT_FUNCTION % re)); + return ret; + } + } - ret.push_back(val.second->fullName()); - } - } catch (boost::bad_expression &) { - _ctx.log(2, boost::str(boost::format("%s :: invalid regex '%s'\n") % - BOOST_CURRENT_FUNCTION % re)); - return ret; + // Pre-order over the subtree, with a heap stack of levels instead of a call per level (which + // also held every ancestor's lock while the subtree below it was listed). Each node's children + // are snapshotted under its own lock. A child's full name is its parent's, a separator and its + // own name -- or just its own name under a parent with an empty name -- which is fullName()'s + // rule, carried down instead of rebuilt from the root for every node. + struct level { + std::vector> kids; // with their full names + std::size_t next = 0; + }; + const auto open = [](state &n, const std::string &n_full) { + level l; + const std::string &base = n._name.empty() ? std::string() : n_full; + boost::mutex::scoped_lock lock(n._mutex); + l.kids.reserve(n._children.size()); + for (const child_map::value_type &kv : n._children) + if (kv.second) + l.kids.emplace_back(kv.second, base.empty() ? kv.first : base + SEPARATOR + kv.first); + return l; + }; + std::vector stack; + stack.push_back(open(*this, fullName())); + while (!stack.empty()) { + level &l = stack.back(); + if (l.next == l.kids.size()) { + stack.pop_back(); + continue; + } + std::pair kid = std::move(l.kids[l.next++]); + boost::this_thread::interruption_point(); + if (re.empty()) { + ret.push_back(kid.second); + } else { + _ctx.log(6, boost::str(boost::format("%s :: check match %s\n") % BOOST_CURRENT_FUNCTION % + kid.second)); + if (boost::regex_match(kid.second, expression)) { + _ctx.log(6, boost::str(boost::format("%s :: matched! %s\n") % BOOST_CURRENT_FUNCTION % + kid.second)); + ret.push_back(kid.second); } - } else - ret.push_back(val.second->fullName()); - - // Get any matches from this state's children if any. - std::vector childret = val.second->children(re); - ret.insert(ret.end(), childret.begin(), childret.end()); + } + stack.push_back(open(*kid.first, kid.second)); // invalidates `l` } return ret; } @@ -1440,49 +1549,66 @@ bool state::isExpired() const { std::size_t state::sweepExpired() { boost::this_thread::interruption_point(); - // Snapshot direct children under our mutex so recursion happens - // without holding the parent lock (avoids deadlock if subscribers - // re-enter via signals). - std::vector snapshot; - { - boost::mutex::scoped_lock lock(_mutex); - snapshot.reserve(_children.size()); - BOOST_FOREACH (child_map::value_type &val, _children) { - snapshot.push_back(val.second); + // Post-order over the subtree, with a heap stack instead of a call per level: a node's + // children are swept (their own subtrees first), then the node removes its expired children. + // Each node's children are snapshotted under its mutex and walked without it (avoids deadlock + // if subscribers re-enter via signals); the snapshot keeps them alive. + struct level { + state *node; + std::vector kids; + std::size_t next = 0; + }; + const auto open = [](state &n) { + level l{&n, {}, 0}; + boost::mutex::scoped_lock lock(n._mutex); + l.kids.reserve(n._children.size()); + BOOST_FOREACH (child_map::value_type &val, n._children) { + l.kids.push_back(val.second); } - } + return l; + }; std::size_t removed = 0; - BOOST_FOREACH (state_ptr &c, snapshot) { - removed += c->sweepExpired(); - } - - // Now collect direct children that are themselves expired and - // erase them from our child map. Holding shared_ptrs in - // to_remove keeps them alive long enough to fire `expiring`. - std::vector> to_remove; - { - boost::mutex::scoped_lock lock(_mutex); - for (child_map::iterator it = _children.begin(); it != _children.end();) { - if (it->second->isExpired()) { - to_remove.push_back(std::make_pair(it->first, it->second)); - _children.erase(it++); - } else { - ++it; + // Remove `n`'s direct children that are themselves expired. + const auto remove_expired_children = [&removed](state &n) { + // Collect them and erase them from the child map. Holding shared_ptrs in to_remove keeps them + // alive long enough to fire `expiring`. + std::vector> to_remove; + { + boost::mutex::scoped_lock lock(n._mutex); + for (child_map::iterator it = n._children.begin(); it != n._children.end();) { + if (it->second->isExpired()) { + to_remove.push_back(std::make_pair(it->first, it->second)); + n._children.erase(it++); + } else { + ++it; + } } } - } - // Fire `expiring` while the node is still alive, then drop our - // reference so the dtor (and `destroyed`) fires next, then - // notify our own parent chain that our child set changed. - for (std::size_t i = 0; i < to_remove.size(); ++i) { - to_remove[i].second->expiring(); - childChanged(to_remove[i].first); - to_remove[i].second.reset(); - ++removed; + // Fire `expiring` while the node is still alive, then drop our reference so the dtor (and + // `destroyed`) fires next, then notify the parent chain that the child set changed. + for (std::size_t i = 0; i < to_remove.size(); ++i) { + to_remove[i].second->expiring(); + n.emitChildChanged(to_remove[i].first); + to_remove[i].second.reset(); + ++removed; + } + }; + + std::vector stack; + stack.push_back(open(*this)); + while (!stack.empty()) { + level &l = stack.back(); + if (l.next < l.kids.size()) { + state &kid = *l.kids[l.next++]; + stack.push_back(open(kid)); // invalidates `l` + continue; + } + state &n = *l.node; + remove_expired_children(n); + stack.pop_back(); } - return removed; } @@ -1561,21 +1687,6 @@ bool state::startup_connection::disconnect() { : erase_startup_entry(_appStartup, _id); } -// ------------------- -// state::notifyParent -// ------------------- -// Purpose: -// Used to propagate child change signals up the tree to the root node. -// Because of this, every change to the entire tree will trigger the root node's -// childChanged signal. -// ---- Change History ---- -// 02/18/2012 -- Joe R. -- Creation. -void state::notifyParent(const std::string &childname) { - boost::this_thread::interruption_point(); - if (parent()) - parent()->childChanged(name() + SEPARATOR + childname); -} - // ------------------ // isValidStateName // ------------------ diff --git a/src/cvc/state/state_distributed_admin.cpp b/src/cvc/state/state_distributed_admin.cpp index 9d4c1abd2..d32ab37ed 100644 --- a/src/cvc/state/state_distributed_admin.cpp +++ b/src/cvc/state/state_distributed_admin.cpp @@ -209,33 +209,52 @@ struct tarjan_ctx { int next_index = 0; std::vector> sccs; - void strongconnect(int v) { - index_of[v] = next_index; - lowlink[v] = next_index; - ++next_index; - stack.push_back(v); - on_stack[v] = true; - - for (int w : adj[v]) { - if (index_of[w] < 0) { - strongconnect(w); - lowlink[v] = std::min(lowlink[v], lowlink[w]); - } else if (on_stack[w]) { - lowlink[v] = std::min(lowlink[v], index_of[w]); + // Tarjan's strongconnect(v), with its recursion on an explicit stack of frames (vertex, next + // successor), so a chain of a million links costs heap, not a native frame per link. + void strongconnect(int root_v) { + struct frame { + int v; + std::size_t next; + }; + std::vector calls; + const auto enter = [&](int v) { + index_of[v] = next_index; + lowlink[v] = next_index; + ++next_index; + stack.push_back(v); + on_stack[v] = true; + calls.push_back({v, 0}); + }; + enter(root_v); + while (!calls.empty()) { + frame &f = calls.back(); + const int v = f.v; + if (f.next < adj[v].size()) { + const int w = adj[v][f.next++]; + if (index_of[w] < 0) + enter(w); // invalidates `f`; on return, lowlink[v] takes lowlink[w] (below) + else if (on_stack[w]) + lowlink[v] = std::min(lowlink[v], index_of[w]); + continue; } - } - if (lowlink[v] == index_of[v]) { - std::vector comp; - while (true) { - int w = stack.back(); - stack.pop_back(); - on_stack[w] = false; - comp.push_back(w); - if (w == v) - break; + if (lowlink[v] == index_of[v]) { + std::vector comp; + while (true) { + int w = stack.back(); + stack.pop_back(); + on_stack[w] = false; + comp.push_back(w); + if (w == v) + break; + } + sccs.push_back(std::move(comp)); + } + calls.pop_back(); + if (!calls.empty()) { + const int u = calls.back().v; + lowlink[u] = std::min(lowlink[u], lowlink[v]); } - sccs.push_back(std::move(comp)); } } }; diff --git a/src/cvc/tests/CMakeLists.txt b/src/cvc/tests/CMakeLists.txt index 09128c648..7d71f2902 100644 --- a/src/cvc/tests/CMakeLists.txt +++ b/src/cvc/tests/CMakeLists.txt @@ -55,6 +55,7 @@ add_executable(state_sync_adapter_link_forwarding_test state_sync_adapter_link_f add_executable(state_cross_cluster_link_test state_cross_cluster_link_test.cpp) add_executable(state_link_bench_test state_link_bench_test.cpp) add_executable(state_list_test state_list_test.cpp) +add_executable(state_deep_tree_test state_deep_tree_test.cpp) if(CVC_STATE_MEMORY_MANAGER) add_executable(state_memory_manager_test state_memory_manager_test.cpp) endif() @@ -229,6 +230,7 @@ set(TEST_TARGETS state_cross_cluster_link_test state_link_bench_test state_list_test + state_deep_tree_test state_delta_codec_test state_volume_codec_test state_brick_manifest_test @@ -1344,6 +1346,13 @@ target_link_libraries(state_list_test GTest::gtest_main ) +target_link_libraries(state_deep_tree_test + PRIVATE + cvc + GTest::gtest + GTest::gtest_main +) + target_link_libraries(state_delta_codec_test PRIVATE cvc @@ -1631,6 +1640,7 @@ if(CVC_STATE_MEMORY_MANAGER) target_compile_features(state_memory_manager_test PRIVATE cxx_std_17) endif() target_compile_features(state_list_test PRIVATE cxx_std_17) +target_compile_features(state_deep_tree_test PRIVATE cxx_std_17) target_compile_features(state_delta_codec_test PRIVATE cxx_std_17) target_compile_features(state_volume_codec_test PRIVATE cxx_std_17) target_compile_features(state_brick_manifest_test PRIVATE cxx_std_17) @@ -2050,6 +2060,7 @@ if(CVC_STATE_MEMORY_MANAGER) cvc_discover_tests(state_memory_manager_test) endif() cvc_discover_tests(state_list_test) +cvc_discover_tests(state_deep_tree_test) cvc_discover_tests(state_delta_codec_test) cvc_discover_tests(state_volume_codec_test) cvc_discover_tests(state_brick_manifest_test) diff --git a/src/cvc/tests/state_deep_tree_test.cpp b/src/cvc/tests/state_deep_tree_test.cpp new file mode 100644 index 000000000..85c1b8c09 --- /dev/null +++ b/src/cvc/tests/state_deep_tree_test.cpp @@ -0,0 +1,266 @@ +// cvc::state on deep trees. A write builds the node's full path (fullName()) and signals every +// ancestor (childChanged, the root's slots first); listing descendants (children()) walks the +// subtree; destroying a node destroys its subtree. None of that may recurse once per tree +// level, or a deep tree overflows the native stack. Each body runs on a thread with a small +// fixed stack (kStackKiB, wasm's 64 KiB), so per-level recursion fails here on every platform. +// +// For probing, CVC_STATE_DEEP_DEPTH overrides the depth and CVC_STATE_DEEP_STACK_KIB the stack +// (0 runs the body on the calling thread). CVC_STATE_BENCH=1 runs the write benchmark. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace { + +constexpr std::size_t kDepth = 100000; +constexpr std::size_t kStackKiB = 64; + +std::size_t env_size(const char *name, std::size_t fallback) { + const char *s = std::getenv(name); + return s && *s ? static_cast(std::strtoull(s, nullptr, 10)) : fallback; +} + +std::size_t depth() { return env_size("CVC_STATE_DEEP_DEPTH", kDepth); } + +// Run `body` on a thread with a kStackKiB stack and rethrow what it throws. +void on_small_stack(const std::function &body) { + const std::size_t kib = env_size("CVC_STATE_DEEP_STACK_KIB", kStackKiB); + if (kib == 0) { + body(); + return; + } + std::exception_ptr error; + boost::thread::attributes attrs; + attrs.set_stack_size(kib * 1024); + boost::thread t(attrs, [&] { + try { + body(); + } catch (...) { + error = std::current_exception(); + } + }); + t.join(); + if (error) + std::rethrow_exception(error); +} + +// A chain of `n` nodes named "n" under `top`, built one level per call; returns the deepest. +cvc::state &chain(cvc::state &top, std::size_t n) { + cvc::state *node = ⊤ + for (std::size_t i = 0; i < n; ++i) + node = &(*node)("n"); + return *node; +} + +std::string repeat_path(const std::string &prefix, std::size_t n) { + std::string out = prefix; + out.reserve(prefix.size() + 2 * n); + for (std::size_t i = 0; i < n; ++i) + out += ".n"; + return out; +} + +class StateDeepTree : public ::testing::Test { +protected: + cvc::app ctx; + cvc::state &root = cvc::state::instance(ctx); +}; + +} // namespace + +TEST_F(StateDeepTree, WriteDeepInTheTree) { + const std::size_t n = depth(); + cvc::state &top = root("deep"); + cvc::state &leaf = chain(top, n); + std::vector at_root, at_top, at_parent; + auto c1 = root.childChanged.connect([&](const std::string &p) { at_root.push_back(p); }); + auto c2 = top.childChanged.connect([&](const std::string &p) { at_top.push_back(p); }); + auto c3 = + const_cast(leaf.parent())->childChanged.connect([&](const std::string &p) { + at_parent.push_back(p); + }); + std::string full; + on_small_stack([&] { + leaf.value("x"); + full = leaf.fullName(); + }); + const std::string expected = repeat_path("deep", n); + EXPECT_EQ(full, expected); + ASSERT_EQ(at_root.size(), 1u); + EXPECT_EQ(at_root[0], expected); + ASSERT_EQ(at_top.size(), 1u); + EXPECT_EQ(at_top[0], expected.substr(std::string("deep.").size())); + ASSERT_EQ(at_parent.size(), 1u); + EXPECT_EQ(at_parent[0], "n"); + c1.disconnect(); + c2.disconnect(); + c3.disconnect(); + on_small_stack([&] { root("deep").reset(false); }); // destroys the chain + EXPECT_EQ(root("deep").findDescendant("n"), nullptr); +} + +TEST_F(StateDeepTree, DestroyDeepSubtree) { + cvc::state &top = root("gone"); + chain(top, depth()).value("x"); + on_small_stack([&] { top.reset(false); }); + EXPECT_EQ(top.findDescendant("n"), nullptr); + // The app's teardown destroys a deep tree too. + chain(root("left_for_teardown"), depth()); +} + +TEST_F(StateDeepTree, ListDeepDescendants) { + // Every descendant's full name: quadratic in depth by nature, so a shorter chain. + const std::size_t n = std::min(depth(), 3000); + cvc::state &top = root("listed"); + chain(top, n); + std::vector names; + on_small_stack([&] { names = top.children(); }); + ASSERT_EQ(names.size(), n); + EXPECT_EQ(names.front(), "listed.n"); + EXPECT_EQ(names.back(), repeat_path("listed", n)); +} + +TEST_F(StateDeepTree, SignalsReachAncestorsRootFirst) { + // One write signals the written node and each ancestor, the ROOT's slots first and the + // parent's last, each with the path relative to it. + cvc::state &leaf = root("o.p.q.r"); + std::vector seen; + auto connect = [&](cvc::state &s, const char *tag) { + return s.childChanged.connect( + [&seen, tag](const std::string &p) { seen.push_back(std::string(tag) + ":" + p); }); + }; + auto c1 = connect(root, "root"); + auto c2 = connect(root("o"), "o"); + auto c3 = connect(root("o.p"), "p"); + auto c4 = connect(root("o.p.q"), "q"); + auto c5 = leaf.valueChanged.connect([&] { seen.push_back("r:value"); }); + leaf.value("v"); + EXPECT_EQ(seen, (std::vector{"r:value", "root:o.p.q.r", "o:p.q.r", "p:q.r", "q:r"})); + for (auto *c : {&c1, &c2, &c3, &c4, &c5}) + c->disconnect(); +} + +TEST_F(StateDeepTree, NamesAndListsAreUnchanged) { + EXPECT_EQ(root.fullName(), ""); + EXPECT_EQ(root("a").fullName(), "a"); + EXPECT_EQ(root("a.b.c").fullName(), "a.b.c"); + EXPECT_EQ(root("a.b.c").parentName(), "a.b"); + EXPECT_EQ(root("a").parentName(), ""); + root("a.b.d").value("1"); + root("a.e").value("2"); + EXPECT_EQ(root("a").children(), + (std::vector{"a.b", "a.b.c", "a.b.d", "a.e"})); // pre-order, map order + EXPECT_EQ(root("a").children(".*d"), (std::vector{"a.b.d"})); + EXPECT_EQ(root("a").numChildren(), 2u); +} + +TEST_F(StateDeepTree, PinnedDescendantKeepsItsSubtree) { + chain(root("pinned"), 50).value("leaf"); + cvc::state::state_ptr mid = root("pinned").findDescendantShared(repeat_path("n", 9)); + ASSERT_TRUE(mid); + root("pinned").reset(false); // drops the tree's ownership; `mid` still owns its part + EXPECT_EQ(root("pinned").findDescendant("n"), nullptr); + cvc::state *leaf = mid->findDescendant(repeat_path("n", 39)); + ASSERT_NE(leaf, nullptr); + EXPECT_EQ(leaf->value(), "leaf"); +} + +// ---- links ------------------------------------------------------------------- + +namespace { +// `n` link nodes links.l0 -> links.l1 -> ... -> links.l -> links.end (a value), or, with +// `cyclic`, l -> l0. +void link_chain(cvc::state &root, std::size_t n, bool cyclic) { + for (std::size_t i = 0; i < n; ++i) { + const std::string next = i + 1 < n ? "links.l" + std::to_string(i + 1) + : (cyclic ? std::string("links.l0") : "links.end"); + root("links.l" + std::to_string(i)).linkTo(next); + } + if (!cyclic) + root("links.end").value("end"); +} +} // namespace + +TEST_F(StateDeepTree, LongLinkChainResolvesAndIsChecked) { + // Following a chain of links (resolveLink, its hop budget raised to cover it) and finding the + // cycles in the link graph (state_distributed_admin::link_cycles) walk link by link. + const std::size_t n = std::min(depth(), 20000); + link_chain(root, n, false); + cvc::state::link_resolution r; + cvc::state_distributed_admin::link_cycles_result cycles; + on_small_stack([&] { + r = root("links.l0").resolveLink(n + 1); + cycles = cvc::state_distributed_admin::link_cycles(root); + }); + EXPECT_EQ(r.kind, cvc::state::link_resolution_kind::resolved); + ASSERT_NE(r.target, nullptr); + EXPECT_EQ(r.target->value(), "end"); + EXPECT_EQ(r.hops, n); + EXPECT_EQ(cycles.link_nodes_scanned, n); + EXPECT_TRUE(cycles.cycles.empty()); +} + +TEST_F(StateDeepTree, LongLinkCycleIsFound) { + const std::size_t n = std::min(depth(), 20000); + link_chain(root, n, true); + cvc::state_distributed_admin::link_cycles_result cycles; + on_small_stack([&] { cycles = cvc::state_distributed_admin::link_cycles(root); }); + ASSERT_EQ(cycles.cycles.size(), 1u); + EXPECT_EQ(cycles.cycles[0].size(), n); + EXPECT_EQ(cycles.cycles[0].front(), "links.l0"); +} + +TEST_F(StateDeepTree, WriteThroughALinkSignalsTheTargetsAncestors) { + // A writable transparent link routes a write to its target, deep in the tree: the target's + // ancestors are signaled (the root's slots first), the link's are not. + const std::size_t n = std::min(depth(), 20000); + cvc::state &target = chain(root("far"), n); + cvc::state &link = root("near.alias"); + link.linkTo(target.fullName(), cvc::state::link_mode::transparent); + link.setLinkWritable(true); + std::vector at_root, at_near; + auto c1 = root.childChanged.connect([&](const std::string &p) { at_root.push_back(p); }); + auto c2 = root("near").childChanged.connect([&](const std::string &p) { at_near.push_back(p); }); + on_small_stack([&] { link.value("through"); }); + EXPECT_EQ(target.value(), "through"); + EXPECT_EQ(link.resolvedValue(), "through"); + ASSERT_EQ(at_root.size(), 1u); + EXPECT_EQ(at_root[0], repeat_path("far", n)); + EXPECT_TRUE(at_near.empty()); + c1.disconnect(); + c2.disconnect(); +} + +TEST_F(StateDeepTree, WriteBench) { + if (env_size("CVC_STATE_BENCH", 0) != 1) { + SUCCEED(); + return; + } + // A write 30 levels deep with a listener on the root (as the sync adapter and pycvc's watch + // have): fullName(), and the signal to every ancestor. + cvc::state &leaf = chain(root("bench"), 30); + std::size_t heard = 0; + auto c = root.childChanged.connect([&](const std::string &) { ++heard; }); + constexpr int N = 200000; + auto t0 = std::chrono::steady_clock::now(); + for (int i = 0; i < N; ++i) + leaf.value(std::to_string(i & 1)); + auto t1 = std::chrono::steady_clock::now(); + const auto ns = std::chrono::duration_cast(t1 - t0).count(); + std::printf("BENCH state_write_depth30 N=%d wall_ns_total=%lld wall_ns_per_op=%lld\n", N, + static_cast(ns), static_cast(ns / N)); + EXPECT_EQ(heard, static_cast(N)); + c.disconnect(); +}