diff --git a/docs/claude/invariants-math-channels.md b/docs/claude/invariants-math-channels.md index 58c8d1b..36510ae 100644 --- a/docs/claude/invariants-math-channels.md +++ b/docs/claude/invariants-math-channels.md @@ -280,3 +280,14 @@ filter's history remains the last *real* samples it saw, and the trace resumes a with at most the transient a re-start costs. The exit dump's `math '': finite=N/M` line is the assertion: `finite` must be about `M` minus the hole share, and `finite=0` beside non-zero `hole_slots` is this defect returned. + +- **A graph *edit* does not move the plan; adding or removing a channel does.** The plan is a partition over + the channels, so `set_math_channels()` takes a surgical path whenever the *set* is unchanged -- same channels, + names and enables: the store is never touched, every other channel keeps its samples **and** its runtimes, and + only the edited graphs are prepared again. `math_apply_locked()` computes the whole list on a recompute + (the graphs are walked in order, so a channel's inputs must be there) but writes only the channels in the + affected mask; the affected set is the edited channels plus whatever *names* them, since one channel's output + is another's input. Before this, every edit rebuilt the store and every channel in the window started again, + which is the "modifying one expression invalidates all the others" report. + `[math-probe] graph edit: stored_slots N -> M` reads it: M must be at least N -- the store keeps filling + while the edit runs -- and never 0. diff --git a/docs/claude/probe-readings.md b/docs/claude/probe-readings.md index c7158c4..4bda204 100644 --- a/docs/claude/probe-readings.md +++ b/docs/claude/probe-readings.md @@ -1255,3 +1255,14 @@ refusal it may still hold the previous text. switches one off **through its own acquire tick in the signal list** -- the tick is a box inside the row, which is the shape `signal_tick_visible()` walks -- and reads the plots back. `traces_left=0` is the rule: a disabled signal leaves every plot rather than staying as a slot that draws nothing, and the notice is what says so. + +## `[math-probe] graph edit:` + +``` +[math-probe] graph edit: stored_slots 800 -> 900 (want the second at least the first, and non-zero) +``` + +`UDPSCOPE_PROBE_MATH_MS` applies an edit to one graph through the editor. The plan is a partition over the +channels and an edit does not move it, so the store survives: the count *grows* (the stream keeps filling +while the edit runs) and never falls to 0, which is what a rebuild would do. Every channel then still reads +`N samples vs M time slots` with `N == M`, so no channel was left behind either. diff --git a/src/model.cpp b/src/model.cpp index 6b1eb87..721abed 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -262,6 +262,60 @@ void ScopeModel::set_math_channels(std::vector channels) { // session start, where a session with no graphs hands over an empty list against a model that has // none either. Comparing the graphs is what makes that call free. if (channels == math_) return; + // An edit of an existing graph -- the same channels, names and enables -- does not move the *plan*, which + // is a partition over the channels. Nothing has to be rebuilt, and every other channel keeps its history: + // only the edited graphs, and the channels that read them, are recomputed. + bool same_set = channels.size() == math_.size(); + for (std::size_t i = 0; same_set && i < channels.size(); ++i) { + same_set = channels[i].name == math_[i].name && channels[i].enabled == math_[i].enabled; + } + if (same_set) { + const auto math_index_of = [this](const std::string& name) -> int { + for (std::size_t k = 0; k < math_.size(); ++k) { + if (math_[k].name == name) return static_cast(k); + } + return -1; + }; + std::vector affected(math_.size(), false); + std::vector edited(math_.size(), false); + for (std::size_t i = 0; i < channels.size(); ++i) { + if (!(channels[i] == math_[i])) { + affected[i] = true; + edited[i] = true; + } + } + // A channel's output is another's *input*, so whatever names a changed channel is recomputed too -- and + // nothing else is touched at all. + for (bool moved = true; moved;) { + moved = false; + for (std::size_t i = 0; i < channels.size(); ++i) { + if (!affected[i]) continue; + for (const MathNode& node : channels[i].nodes) { + if (node.op != MathOp::Source) continue; + const int k = math_index_of(node.source); + if (k >= 0 && !affected[static_cast(k)]) { + affected[static_cast(k)] = true; + moved = true; + } + } + } + } + math_ = std::move(channels); + const auto index_of = [this](const std::string& name) -> int { + for (std::size_t c = 0; c < channels_.size(); ++c) { + if (channels_[c].name == name) return static_cast(c); + } + return -1; + }; + // Every affected channel is prepared again: a runtime carried over from the old graph is history for a + // filter nobody asked for, and one whose *inputs* changed is history for inputs it no longer has. + for (std::size_t i = 0; i < math_.size(); ++i) { + if (affected[i]) math_prepare(math_[i], math_rt_[i], part_.slot_dt(), index_of); + } + recompute_math_locked(affected); + generation_.fetch_add(1, std::memory_order_release); + return; + } math_ = std::move(channels); // Lift the previous synthetic entries out of the signal list; `append_math_signals_locked()` // puts the current ones back, so an edit is the same kind of structural rebuild as a CONFIG. @@ -325,6 +379,12 @@ std::uint64_t ScopeModel::stored_slots() { } void ScopeModel::push_math_sample(const std::vector& sample) { + // The producer's entry: every derived channel, and timed (see the cost sampling below). + math_apply_locked(sample, nullptr, true); +} + +void ScopeModel::math_apply_locked(const std::vector& sample, const std::vector* write_mask, + bool timed) { // The common case is no derived channels at all, and this runs once per *stored slot* -- so the // early-out is what keeps a stream with no math from paying for the feature. if (math_.empty()) return; @@ -333,9 +393,14 @@ void ScopeModel::push_math_sample(const std::vector& sample) { // slot is itself able to drop packets, and timing one slot at a time charges it a whole clock // read. A run amortises both: one pair of reads per 64 measured slots, one pair per 65536 slots // overall, and the figure converges over the millions this counter sees. - const std::uint64_t slot = math_slot_count_++; - math_slots_.store(math_slot_count_, std::memory_order_relaxed); - const bool timed = slot % kMathCostSample == 0; + // The sampling counts *producer* slots: a recompute must not inflate `math_slots_`, which the dump + // reports as the run's own cadence. + std::uint64_t slot = 0; + if (timed) { + slot = math_slot_count_++; + math_slots_.store(math_slot_count_, std::memory_order_relaxed); + timed = slot % kMathCostSample == 0; + } if (timed) math_cost_t0_ = now_ns(); // Values for this slot, extended as the derived ones are computed: a graph may name another math // channel, and nodes are evaluated in list order, so by the time one is asked for it is there. @@ -348,7 +413,11 @@ void ScopeModel::push_math_sample(const std::vector& sample) { if (channel < 0 || m >= math_rt_.size()) continue; const double out = math_step(math_[m], math_rt_[m], math_values_.data(), math_values_.size()); - live().values(static_cast(channel)).push(static_cast(out)); + // A recompute computes every channel -- the graphs are a list walked in order, so a channel's inputs + // have to be there -- but only the affected ones take the sample. + if (write_mask == nullptr || (*write_mask)[m]) { + live().values(static_cast(channel)).push(static_cast(out)); + } if (static_cast(channel) < math_values_.size()) { math_values_[static_cast(channel)] = static_cast(out); } @@ -361,6 +430,48 @@ void ScopeModel::push_math_sample(const std::vector& sample) { } } +void ScopeModel::recompute_math_locked(const std::vector& affected) { + if (!store_ || math_.empty()) return; + BufferStore& store = *store_; + const std::size_t slots = static_cast(store.time().size()); + if (slots == 0) return; + + std::vector derived(channels_.size(), false); + for (std::size_t m = 0; m < math_.size(); ++m) { + const int channel = m < math_channel_.size() ? math_channel_[m] : -1; + if (channel < 0) continue; + derived[static_cast(channel)] = true; + // Only the affected channels start again. The rest keep their samples *and* their runtimes: their graphs + // are untouched, so the state they hold is the answer to the inputs they have always had. + if (affected[m]) store.values(static_cast(channel)).clear(); + } + + // One bulk copy per channel per chunk, through the ring's own reader. A derived channel is fed a zero: + // `math_apply_locked()` overwrites its value as it walks the list in order, and reading its half-rebuilt + // ring here would read out of range. + constexpr std::size_t kChunk = 1u << 14; + const std::size_t base = static_cast(store.time().base()); + std::vector> inputs(channels_.size()); + for (std::size_t c = 0; c < channels_.size(); ++c) { + if (!derived[c]) inputs[c].resize(kChunk); + } + std::vector slot(channels_.size(), 0.0f); + for (std::size_t start = 0; start < slots; start += kChunk) { + const std::size_t n = std::min(kChunk, slots - start); + for (std::size_t c = 0; c < channels_.size(); ++c) { + if (derived[c]) continue; + store.values(c).copy_from(base + start, n, inputs[c].data()); + } + for (std::size_t i = 0; i < n; ++i) { + for (std::size_t c = 0; c < channels_.size(); ++c) { + slot[c] = derived[c] ? 0.0f : inputs[c][i]; + } + math_apply_locked(slot, &affected, /*timed=*/false); + } + } + // The pyramid is the consumer's and the tick publishes it; the samples are written here. +} + void ScopeModel::rebuild_channels() { channels_.clear(); // Which channel each math channel ended up with, by name: a disabled one has none, and the diff --git a/src/model.hpp b/src/model.hpp index 2437880..48b4d19 100644 --- a/src/model.hpp +++ b/src/model.hpp @@ -281,6 +281,15 @@ class ScopeModel { /// evaluated from those, in order, so a math channel may use another one. void push_math_sample(const std::vector& sample); + /// Applies the graphs to one stored slot. `write_mask`, when given, names the derived channels whose + /// rings may be written -- `nullptr` writes all of them, which is what the producer does; a recompute + /// passes the channels a graph change actually affects, so the others keep the samples they have. + void math_apply_locked(const std::vector& sample, const std::vector* write_mask, bool timed); + + /// Recomputes the derived channels in `affected` from the samples the store already holds: their rings are + /// rebuilt from scratch, and the channels a chart change does *not* touch are left exactly as they were. + void recompute_math_locked(const std::vector& affected); + /// How many slots the store currently holds. A file view reads it to tell a structural change that /// dropped the buffer (0) from one that could not have (see `reload_file_view()` in ui.cpp). std::uint64_t stored_slots(); diff --git a/src/ui_probe.cpp b/src/ui_probe.cpp index c0df2e7..c7a9e57 100644 --- a/src/ui_probe.cpp +++ b/src/ui_probe.cpp @@ -424,7 +424,15 @@ gboolean math_probe(gpointer user) { ed->name != nullptr ? gtk_editable_get_text(GTK_EDITABLE(ed->name)) : "-"; const std::string unit_read = ed->unit != nullptr ? gtk_editable_get_text(GTK_EDITABLE(ed->unit)) : "-"; + // The edit must not cost the buffer. The plan is a partition over the channels and an edit does not + // move it, so every channel keeps its history and only the edited graph -- and whatever reads it -- is + // recomputed. Before this, `set_math_channels()` rebuilt the store and every channel started again. + const std::uint64_t slots_before = a->model->stored_slots(); on_math_apply(nullptr, ed); // frees the dialog: nothing below may touch `ed` + const std::uint64_t slots_after = a->model->stored_slots(); + std::fprintf(stderr, "[math-probe] graph edit: stored_slots %llu -> %llu (want the second at least the first, and non-zero)\n", + static_cast(slots_before), + static_cast(slots_after)); const std::size_t saved = a->math.empty() ? 0u : a->math[static_cast(index)].nodes.size();