Repository navigation
state: walk the tree and its links without recursing once per level - #664
Merged
Merged
Conversation
cvc::state recursed once per tree level on every write: fullName() rebuilt the path parent by parent, and childChanged reached the root through each node's first slot (notifyParent) re-firing its parent's signal. Destroying a subtree, listing descendants (children()), sweeping expiries and resolving a long path in operator() recursed the same way, and link_cycles() recursed once per link in a chain. On a 64 KiB stack (wasm's), a write 41 levels deep overflowed it. - fullName()/parentName(): one pass up the _parent chain, building the path once (it was O(depth^2) characters). A write builds its full name only for the error it throws. - emitChildChanged(): the parent and every ancestor in a loop, the root's slots first, each with the path relative to it -- the order the relay produced. Only nodes with slots get a path, cut from one string. notifyParent is gone; emitting childChanged directly now fires only that node's slots. - ~state: destroys its subtree with a heap stack, leaf-first, so every node still has its ancestors when it dies. A child someone else owns keeps its subtree. - operator(): a loop over the path's keys (a call per key, re-joining the rest of the path each time, was also quadratic in its length). - children(): pre-order with a heap stack, each node's children snapshotted under its own lock (it held every ancestor's lock), full names carried down instead of rebuilt per node. - sweepExpired(): post-order with a heap stack. - link_cycles(): Tarjan's strongconnect on an explicit stack. Link resolution (resolveLink, resolveRemote, resolvedValue) and the transparent-link alias walk were already loops with hop budgets. New state_deep_tree_test runs on a 64 KiB thread: a write 100000 levels deep, its teardown, children() 3000 deep, resolving and cycle-checking a 20000-link chain, a write through a transparent link to a target 20000 deep, and checks that signal order, names, lists and pinned subtrees are unchanged.
transfix
added a commit
that referenced
this pull request
Oct 18, 2026
The walks #664 left recursive: ptree()/json()/save() and their loader, traverse(), and reset(). - ptree()/json()/save() write one level: an entry per node keyed by its full path, in pre-order, holding its value ("" for a childless empty node below the start). Those are the leaves of the nested document they used to write, in the same order. boost's ptree, write_json and read_json recurse once per nesting level, so the nested form could not be made deep-safe without a codec of our own. The loader walks every nesting level with a heap stack, so nested documents still load. - traverse() visits each node once. It traverse()d every descendant children() listed, so a node d levels down a chain was visited 2^(d-1) times (state_cluster_shard::snapshot() listed entries that many times). Pre-order with a heap stack, traverseEnter/Exit nested, the regex as before; it walks held nodes instead of re-finding them by name from the root, which recreated nodes removed meanwhile. - reset(true) clears pre-order and touches post-order as before, with a heap stack and no lock held while signaling. The recursion held every ancestor's lock during each touch, so a slot that looked its node up from the root (as the sync adapter's do) deadlocked. reset(false) drops the child map after releasing the lock, so destroyed() slots don't run under it. New state_deep_tree_test cases run on a 64 KiB stack: json/save/restore round trip 1000 deep, traverse 3000 deep, reset 100000 deep (3000 with signals), plus the visit order, reset order, flat format, legacy nested load and the lock-free signaling. state_snapshot_protocol_test checks a snapshot lists each node once.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
cvc::staterecursed once per tree level on every write, and in several other places:fullName()rebuilt the path parent by parent, which is O(depth²) characters.childChangedreached the root through each node's first slot (notifyParent) re-firing its parent's signal: one boost::signals2 invocation frame per level.children(),sweepExpired()and resolving a long path inoperator()recursed the same way.state_distributed_admin::link_cycles()recursed once per link in a chain.On a 64 KiB stack (wasm's), a write 41 levels deep overflowed. That is why #657's snapshot codec has to spill containers to keep snapshot trees about 30 levels deep. This was asked as "can fullName() and the change signals be scheduled on the state_exec scheduler?".
cvc::statesits below state_exec, and its signals are a synchronous contract for every observer, so this removes the recursion in place instead and keeps what observers see.What
fullName()/parentName(): one pass up the_parentchain, building the path once. A write now builds its full name only for the error it throws, not on every write.emitChildChanged()(new, public): fireschildChangedon the parent and every ancestor in a loop.SignalsReachAncestorsRootFirstpasses on both master and this branch.notifyParentis gone. EmittingchildChangeddirectly now fires only that node's slots; nothing in the repo did that.~state: destroys its subtree with a heap stack, leaf-first, so every node still has its ancestors when it dies. A child someone else owns (a pin) keeps its subtree.destroyed()fires on the node being destroyed first, then on its descendants leaf-first (before, all in pre-order). Only tests connect todestroyed, and none depends on the order.operator(): a loop over the path's keys. Before, it was a call per key that re-joined the rest of the path, which was also quadratic in the path's length.children(): pre-order with a heap stack. Each node's children are snapshotted under its own lock; before, it held every ancestor's lock. Full names are carried down instead of rebuilt per node. The output is the same, including with a regex.sweepExpired(): post-order with a heap stack.link_cycles()runs Tarjan's strongconnect on an explicit stack.resolveLink,resolveRemote,resolvedValue/resolvedData,sendMessage) andtransparent_link_aliaseswere already loops with hop budgets.Measurements (MSVC 14.38 Release, Windows)
Deepest operation that passes, before → after. A 64 KiB or 256 KiB stack is a thread with that stack; 1 MiB is the main thread.
children()of a chain (test capped at 3000)A write 30 levels deep with a listener on the root (as the sync adapter and pycvc's
watchhave) took 11.3 µs before and 2.2 µs after. That is the median of 8 interleaved rounds per side,StateDeepTree.WriteBench.Tests
state_deep_tree_test(10), run on a 64 KiB thread:children()3000 deep;fullName()/parentName();children()order with and without a regex;CVC_STATE_DEEP_DEPTH/CVC_STATE_DEEP_STACK_KIBoverride depth and stack, andCVC_STATE_BENCH=1runs the benchmark.image_testReadsCmykInputAsRgbandcvc_cli_testVol2Img/Img2Vol: ImageMagick has no decode delegate here.volume_io_test: an intermittent access violation inCvcRawRoundTrip/RawivToMrcConversion. Five runs of a master-based build: 3 failed (2 crashes). This branch: 4 of 5 failed, and 2 of 2 earlier runs passed.hdf5_volume_testtimed out once under load. It passed in 527 s in the other run, close to the runner's 600 s cap.check_test_targets.pyis OK.Not in this PR (still recursive)
ptree()/json()and their restore. The boost property tree they build is itself recursive, both to build and to destroy.traverse(). It also visits a node once per ancestor, becausechildren()lists all descendants andtraverse()recurses into each.reset(true).transparent_link_aliases()on every replicated mutation, and that lists the whole tree each time. It's a separate performance item.