15 Commits

Include renames

Author	SHA1	Message	Date
dtourolle	3b67b7e1e9	Merge origin/master into master	2026-08-06 21:57:32 +02:00
dtourolleandClaude Opus 5	6802328e97	fix: a push must wake its consumer, even when the ring looked non-empty ... push(), try_push() and push_blocking() fired push_callback_ only on the empty->non-empty edge, and computed that edge from a head_ sampled before the item was published. A PoolNode consumer decides whether to run again from the level (count_ready -> approx_size), so a pop landing in that window left both sides standing down: producer (push) consumer (PoolNode firing) ------------------------ ---------------------------- samples t=782, h=781 -> was_empty = false, no wake pops idx 781, head_ = 782 count_ready(): head_==tail_==782 -> not ready, gate released to Idle tail_.store(783) The item is in the ring, the node is idle, and no wake is outstanding. The failure is absorbing: every later push then sees a non-empty ring, so the edge never fires again and the node sleeps while its backlog grows. Observed as a hang in bench_pipeline at (chain, depth=4, work_us=10, shared pool): all pool workers asleep in worker_loop, the reader blocked in pop(), and 218 items stranded in one channel with head_ stopped at exactly the index where the edge was dropped. Re-reading head_ after the tail_ store does not fix this. That is the store-buffer pattern, and under acquire/release both sides may legally read stale; forbidding it needs seq_cst on the producer's tail_ store and head_ load *and* on the consumer's head_ store and tail_ load, a fence on both hot paths. Firing unconditionally is correct by construction: the callback runs after the publishing store, so a consumer that observes the level at all observes the item. The redundant wakes are cheap -- on_input_ready re-checks the level and SubmitGate::claim() collapses a wake arriving mid-firing into the firing already in flight. The stress test named this exact hazard and could not detect it: it asserted only 1 <= callbacks <= N, which a *missed* callback satisfies. It now requires one callback per successful push, and fails at 1325/10000 against the old code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>	2026-08-06 20:50:34 +02:00
dtourolle	c9aa246322	fix: a re-offered sentinel is not data loss ... 139bfbb made the channel refuse a sentinel offered while one was still pending — correct, and the reason is a data race: overwriting wrote eof_value_ while the consumer could be moving the previous one out of it, which ThreadSanitizer reports as a torn refcount and a heap-use-after-free. That part stands. What was wrong was the accounting. The refusal recorded a drop, and PoolNode reported it through the overflow event callback, on the theory that two control tokens on one channel means the stream ended twice and is a caller protocol error. It is not. A source that has reached the end of its input keeps being polled and keeps returning EOF — that is the normal steady state, not an error — so the token is re-offered on every firing. Refusing a re-offer loses nothing: the pending token carries the same meaning and is already on its way. Found by running it. scene-actor-extraction on a 14 s clip reported [main] ERROR: frames were dropped (channel overflow): frame_source: 2 scene_annotate: 1 [main] The output would describe footage that was never analysed. Refusing to report success. and exited 2, on a run where nothing had been dropped and every frame was analysed. frame_source emits EOF once and then returns it forever (frame_source_node.hpp:76), so the count grows with however many times the source is polled after the end. The pipeline's own loss detector — which exists because a dropped frame silently corrupts the output — was being tripped by a clean run, which is the one thing a loss detector must not do. So SlotBusy now records nothing and reports nothing. The cost, stated plainly: a genuinely distinct second token would also be refused silently, and the channel cannot tell a re-offer from a distinct token. Re-offering is the case that actually occurs; the delivery guarantee that matters — the first token arrives — holds either way. The test asserting the old behaviour is inverted rather than deleted, and now also checks that the token which was accepted is the one delivered. 148/148.	2026-08-06 20:44:38 +02:00
dtourolle	80c2b1fb2f	fix: try_push must distinguish delivered from discarded ... try_push returned bool, and returned *true* for a closed channel — so "the value arrived" and "the value was thrown away because nobody is listening" were the same answer. Every caller was nonetheless correct, because both cases mean "stop trying, do not park and retry". But nothing above the channel could tell the two apart: a node counting successful pushes counted discards among them, and the only record of the loss was the channel's own drop counter, visible solely to whoever read the diagnostics table. Now a three-way PushResult { Taken, Full, Closed }, matching the shape SentinelResult already uses. Behaviour is unchanged at every call site — each treats Closed the same as Taken, and only Full parks — but the distinction is now available to anyone who needs it, and a scoped enum means a future caller cannot silently reintroduce the conflation with `if (push)`. deliver_one benefits immediately: it no longer reaches its teardown path for a closed channel, only for one that is still full, so the last-ditch throwing push it does there to record the loss now records an overflow rather than a drop the channel had already counted.	2026-08-05 16:14:50 +02:00
dtourolle	012b64dd3e	fix: the sentinel must not be delivered ahead of a queued value ... pop() and try_pop_now() observe the ring empty and then call take_sentinel(). The producer can push a value *and* publish the sentinel in the window between those two steps, so the sentinel was delivered with a real value still queued behind it — breaking the "sentinel is strictly last" contract that downstream teardown depends on, and losing that value to any consumer which, like the stress cases here, treats the sentinel as EOF and stops draining. a0c4bf5 closed the variant where the caller's emptiness check ran against a stale tail_ snapshot. This is the one where the check is fresh and simply too early. take_sentinel now re-checks emptiness *after* observing has_eof_, which is what makes it sound rather than merely narrower: the producer publishes the sentinel with a release store after its ring pushes, so a consumer that has observed has_eof_ has necessarily observed every tail_ advance before it. A non-empty ring at that point means those values genuinely precede the sentinel, and returning false hands them over first. Rates, since this is a race and the numbers are the evidence. The existing "sentinel is strictly last" stress cases fail about 1 run in 15 on the commit before this one and 0 in 25 after; they did not fail in 25 runs of the pre-series baseline, so something in this series widened the window rather than opened it. I could not pin down which change, and it does not much matter: the interleaving is reachable from the code as written, and the narrower version was never correct. No new test. The two existing stress cases already assert exactly this and are what caught it; a deterministic reproduction would need a seam inside pop() that the fix then makes unreachable.	2026-08-05 16:11:20 +02:00
dtourolle	0f277c0f98	fix: the drain loops must terminate, and must drain the right channels ... shutdown()'s drain step was an unbounded while (anything, anywhere, is non-empty) poll every channel Three defects in one loop. It drained the wrong thing. Stopping a node should wait for that node's own outputs before moving to the next layer; this waited for the entire graph to fall idle each time. The dynamic Network's version made it explicit — it took a node name and ignored it. Both now track which node feeds each probe and wait only on those. It had no deadline, so anything wedged downstream turned a graceful shutdown into the hang it exists to avoid. Now bounded two ways, because a stalled consumer and a slow one fail differently: a deadline for fill that never changes, and a no-progress counter that keeps waiting as long as the queue is shrinking, so a slow drain is not cut short merely for taking a while. And it could fail to terminate with nothing wedged at all. current_fill came from a snapshot that loaded tail_ before head_. A concurrent pop between the two reads yields a head_ past the sampled tail_, and the unsigned difference wraps to ~2^64 — so a poll for "is it empty yet" runs forever on a channel that is in fact empty. Both indices only ever increase, so loading head_ first can at worst under-report a concurrent push, which this loop tolerates and a wrap does not. size() and snapshot() are both corrected; size() feeds approx_size(), which is what node readiness checks call. Giving up is now reported rather than silent, because undrained data at that point is about to be discarded by the stop that follows, and a graceful shutdown quietly dropping values is the thing worth knowing about. Verified in both directions: with the old loop the new case is killed at a 30 s timeout; with this it returns in under a second, having reported four items its wedged consumer never took. Full suite 138/138. Note the drain timeout is per node and defaults to 5 s, so a graph of N stalled nodes can still take N x 5 s to shut down. That is a deliberate trade against cutting off legitimate slow drains, and set_drain_timeout() exists for callers who want it tighter.	2026-08-05 14:25:00 +02:00
dtourolle	139bfbb794	fix: the sentinel slot holds one token and refuses a second ... push_sentinel wrote eof_value_ unconditionally. Offering a second token before the first was taken did two wrong things at once. It lost the first silently, and a lost EOF is not a lost frame — it is the token every downstream node is waiting for in order to shut down, so losing it wedges the pipeline. And it wrote the storage while the consumer could be moving the previous value out of it. I expected that to be a stale read; ThreadSanitizer shows it is worse. On the shared_ptr storage that non-trivial types use, the racing write tears the refcount, and the stress case added here reports heap-use-after-free in extract() alongside the data race. try_push_sentinel now refuses when the slot is occupied, which turns the slot into a correct SPSC handshake: the producer is the only writer of eof_value_ and the only one that sets has_eof_, the consumer is the only one that clears it, so observing it false is what licenses the write. Refusal is recorded as a drop, and PoolNode reports it through the overflow event callback, because a refused control token going unnoticed is the failure this commit exists to stop. Refusing rather than queueing is deliberate. Two control tokens on one channel means the stream ended twice, which is a caller protocol error and not backpressure; parking and retrying would spin against a slot only the consumer can free, and there is no sensible second value to deliver after the end of a stream. The non-consuming try_push_sentinel exists so a refused token is still the caller's to report — the consuming push_sentinel cannot offer that, since the value has already been moved into its parameter. Single-shot EOF is what every current caller does, so this is latent for them today. It stops being latent the moment a pipeline is reused for a second input, which is what the persistent-pipeline work in 4b6e498 sets up. Verified in both directions under -DKPN_SANITIZER=thread: the new contended case reports three data races and a heap-use-after-free against the old overwrite, and is clean with the handshake. Full suite 137/137, TSan clean across unit and stress suites.	2026-08-05 14:06:26 +02:00
dtourolle	8d319eeb88	fix: an empty channel is not a closed one ... pop_one reported an empty channel the same way it reported a closed one, by throwing ChannelClosedError, and fire_once treats that as "upstream is finished" and calls self_stop(). self_stop disables the node's own inputs *and* outputs, so a benign empty read does not merely skip a frame — it kills the node and, through the disabled channels, whatever depended on it. A node genuinely does get woken with empty inputs: a space callback fires when its output drains, which has nothing to do with input arrival. fire_once already guards against it by checking readiness before popping. That guard is the live protection and it works; this commit makes the thing it is guarding non-lethal. So the pop_one path changed here is unreachable today, and I would rather say that than imply a fixed hang. Its value is that the readiness check is now a performance detail rather than the only thing standing between a routine wake and a dead pipeline. Three separate comment blocks in fire_once exist to warn about exactly this hazard; they were added because it had already been hit during development, and the conflation they warn about is what this removes. Verified in three directions. With the guard and the distinction: passes. With the guard removed but the distinction present: still passes, which is the point — the new ChannelEmptyError path catches what the guard used to. With both removed, reproducing the original code: the node self-stops on the firing that has nothing to read, the next value throws "channel closed" out of its own output channel, and the relay handles one item instead of two.	2026-08-05 13:55:57 +02:00
dtourolle	28e06675f5	fix: park nodes on a full output instead of blocking the worker ... push_blocking parked a scheduler worker inside the push. Nodes own a private single-thread pool, so the parked thread was the only one that could drain that node's own input — hold-and-wait, and under sustained backpressure four nodes of a five-node chain slept in nanosleep at once. channel.hpp already warned about this for sentinels; it applies just as much to data pushes. The scheduler was purely input-driven: on_input_ready() wakes a node when input arrives, with no counterpart for "my output has room". Lacking that signal, blocking the thread was the only way to handle a full output. This adds the missing half. - Channel::try_push + has_space + set_space_callback; the callback fires from both pop() and try_pop_now(). - PoolNode/PoolObjectNode keep a one-slot pending_ buffer with per-element done flags, so a retry cannot duplicate an already-accepted element. One slot suffices because queued_ admits at most one fire_once per node. - The re-check after clearing queued_ closes the lost-wakeup race where a space callback fires while the flag is still up and is swallowed. Two bugs surfaced once nodes actually parked, both fixed here: - pop_one reports an *empty* channel as ChannelClosedError, which is also the node's "upstream finished, self-stop" signal. A node woken by output space with empty inputs therefore killed itself. fire_once now releases the worker when its inputs are not ready rather than falling through. - The drained-park path resubmitted unconditionally instead of via on_input_ready(), firing nodes with nothing to read. compute_priority is now output-aware: mean output fill is deducted from mean input fill, mapped as 0.5·(1 + in - out). Input fill alone asks only "how much work is waiting for me"; a node whose outputs are already full cannot deliver, so running it just parks it again and wastes the slot while the node that would drain that channel waits behind it. The scheduler now favours whoever is furthest downstream of a bottleneck. Also adds a network-level error listener. A node's exception was discarded at the node boundary and survived only as a Closed event, which reports that a node stopped but not why — that missing detail is what made the above slow to diagnose. INode::set_network_error_callback plus StaticNetwork::set_error_handler forward it to the application. Tests: 121/121. test_backpressure_deadlock drives a five-node chain with capacity-2 channels against a slow sink and fails on the old code. The four test_pool_node overflow tests now assert parking rather than the removed drop-and-report behaviour. Known-incomplete: a rare hang remains, roughly 1 run in 20 against a 300s timeout, down from every run failing. Committed because the fix is a large strict improvement and the residual case needs its own reproduction.	2026-07-31 22:40:07 +02:00
dtourolle	4b6e498ba7	feat: persistent-pipeline reuse — push_blocking, node introspection, stateful wrapper ... Adds three pieces needed to build one KPN network and reuse it across many replays/configs instead of tearing down and rebuilding per run: - Channel<T>::push_blocking (+ IVariantChannel/VariantChannel forwarding): lossless backpressure push that waits for space instead of dropping when the ring is full. PyNode's run_loop now uses it so a downstream consumer lagging behind never silently drops a frame. - PyNetwork::node_ptr / node_stats: raw node handle by name (for a binding to dynamic_cast to a concrete wrapper and call functor-specific runtime setters) and a per-node timing snapshot for profiling. - ObjectVariantNodeWrapper: variant-node adapter for functors that need runtime-constructed state (a Config, a loaded gallery), mirroring VariantNodeWrapper's channel plumbing but backed by ObjectNode<Obj>. Built and used downstream in scene-actor-extraction's sae_kpn Python replay bindings for repeated threshold-sweep evaluation of the same pipeline.	2026-07-19 16:55:36 +02:00
dtourolleandClaude Opus 4.8	a0c4bf580e	fix: deliver EOF sentinel only when the ring is freshly empty ... Channel<T>::pop() surfaced the out-of-band sentinel from its empty branch using the tail_ snapshot taken at the top of the loop. Under contention the producer can push more values *and* the sentinel in the window between that snapshot and take_sentinel(), so pop() could return the sentinel while real values still sat in the ring — the sentinel jumping ahead of values pushed before it. No value was lost (a consumer that keeps draining still receives them, and approx_size() keeps counting them so a PoolNode reschedules), but a consumer treating the sentinel as a hard "last message" barrier would act on EOF early. Re-confirm emptiness against a fresh tail_ load before taking the sentinel. Costs one acquire-load on the empty-ring path only; never runs in steady state. The spin and post-spin takes already reload tail_ on the line above them; try_pop_now() already reads tail_ fresh in the same branch — both were correct and are unchanged. The two sentinel stress cases now assert the strict "sentinel is last, after every value" ordering (previously relaxed to avoid the flake this fixes). Verified TSan-clean (2606 assertions, no data races) over repeated runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-14 23:14:45 +02:00
dtourolleandClaude Opus 4.8	19f5a2b0ae	Deliver EOF sentinels out-of-band to prevent teardown deadlock ... Channel::push() drops values on overflow (the intended backpressure policy for data), and PoolNode swallows the resulting ChannelOverflowError. For a control sentinel like EOF this is fatal: a single dropped EOF under backpressure wedges every downstream pop() forever, so the pipeline never tears down. Deliver sentinels out-of-band instead. Channel::push_sentinel() stores the token in a dedicated slot that does not consume ring capacity, so it can never overflow and — crucially — never blocks the caller. That non-blocking property is essential: each KPN node has a single worker thread, so a *blocking* push would park that thread and stop it draining its own input, cascading into a hold-and-wait deadlock under backpressure. The consumer's pop()/try_pop_now() drain the ring first, then deliver the sentinel, so it always arrives after every value pushed before it. approx_size() (which node readiness checks call) counts a pending sentinel as consumable work, so a channel carrying only a sentinel still schedules its consumer's next fire — without this the token would sit undelivered and the pipeline would still deadlock at teardown. PoolNode/PoolObjectNode route values carrying an eof flag (direct .eof or nested .source.eof) through push_sentinel via a SFINAE-safe is_sentinel_value trait; all other values keep the existing lossy throwing push. The trait compiles to false for types without an eof convention, so this is a no-op for pipelines that don't use one. Verified end-to-end: scene_analyze now reaches EOF, flushes its output, and exits cleanly instead of hanging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-04 00:30:40 +02:00
dtourolle	f6bcaa15b0	Performance improvements, better readme and complete python bindings	2026-05-12 21:23:33 +02:00
dtourolle	da8f4d9926	Add debug graph to static network	2026-05-09 08:59:19 +02:00
dtourolle	5e77dc836b	First attempt	2026-05-08 17:48:16 +02:00