diff --git a/src/acquisition.cpp b/src/acquisition.cpp index 3eb0354..9748c27 100644 --- a/src/acquisition.cpp +++ b/src/acquisition.cpp @@ -626,6 +626,11 @@ void Acquisition::on_data(const udps_frame_t* frame) { // event window out into an immutable `EventSnapshot`. BufferStore& store = model_.live(); const u64 g = part.time_stride; + // `tick % g == g - 1` as a mask. `plan_partition()` doubles `time_stride` from 1, so `g` is + // a power of two -- but it is a *runtime* value, so the modulo compiled to a real 64-bit + // division rather than the cheap test its comment there claims. See `acc_mask_` for the + // per-channel form and for what that cost on the hot loop. + const u64 g_mask = g - 1; // A re-plan replaced the store, so every ring's `produced()` restarted at 0 while `tick_` // kept counting. `backfill_from()` derives a slot index from a hole's `tick_begin`, which is @@ -655,6 +660,13 @@ void Acquisition::on_data(const udps_frame_t* frame) { value_acc_.assign(channels.size(), 0.0); value_acc_n_.assign(channels.size(), 0); math_sample_.assign(channels.size(), std::nanf("")); + // The group boundaries, as masks, for the per-sample loops below. Here rather than in + // them: `g` and the strides are properties of the partition, and a partition change is + // exactly what this block already reacts to. + acc_mask_.resize(channels.size()); + for (std::size_t i = 0; i < channels.size(); ++i) { + acc_mask_[i] = g * std::max(1, part.stride[i]) - 1; + } time_acc_ = 0.0; abs_time_acc_ = 0.0; time_acc_n_ = 0; @@ -797,6 +809,9 @@ void Acquisition::on_data(const udps_frame_t* frame) { for (std::uint32_t k = 0; k < n; ++k) { const u64 tick = tick_++; + // The shared clock's group boundary, read once and reused by the time ring, the channel + // loop's own per-channel test and the math push: three tests of the same thing. + const bool group_end = (tick & g_mask) == g_mask; const double rel = std::max(stamps_[k] - epoch, last_stamp_); last_stamp_used = rel; last_stamp_ = rel; @@ -808,7 +823,7 @@ void Acquisition::on_data(const udps_frame_t* frame) { last_abs_stamp_ = abs_stamps_[k] - epoch; abs_time_acc_ += last_abs_stamp_; ++time_acc_n_; - if (tick % g == g - 1) { + if (group_end) { store.time().push(time_acc_ / static_cast(time_acc_n_)); time_acc_ = 0.0; // The absolute clock on the same slot and under the same group mean, so an export can @@ -853,8 +868,8 @@ void Acquisition::on_data(const udps_frame_t* frame) { value_acc_[i] += raw * ch.gain + ch.offset; ++value_acc_n_[i]; - const u64 period = g * part.stride[i]; - if (tick % period != period - 1) continue; + const u64 mask = acc_mask_[i]; + if ((tick & mask) != mask) continue; const f32 value = static_cast(value_acc_[i] / static_cast(value_acc_n_[i])); value_acc_[i] = 0.0; @@ -878,7 +893,7 @@ void Acquisition::on_data(const udps_frame_t* frame) { // `publish_event()` clamps its snapshot to every value ring's `produced()`, so a derived // channel pushed after it would be missing its newest sample from the capture -- the same // one-sample clamp the deferred latch above exists for. - if (tick % g == g - 1) model_.push_math_sample(math_sample_); + if (group_end) model_.push_math_sample(math_sample_); if (latch_here) latch(latch_edge); } last_stamp_ = last_stamp_used; @@ -929,8 +944,12 @@ void Acquisition::reserve_hole(std::uint32_t missing, const Partition& part, double dt = part.base_dt; if (!(dt > 0.0)) dt = 1.0e-3; + // The same group tests as the live loop, written the same way: a hole's samples go through + // the same accumulators, so the two phases have to agree sample for sample. + const u64 g_mask = g - 1; for (u64 j = 0; j < raw; ++j) { const u64 tick = tick_++; + const bool group_end = (tick & g_mask) == g_mask; // The hole is stamped at the declared cadence from the last good stamp -- exact in // relative mode, and in stream mode the same continuation the glitch bridge makes. // It must be *stamped* rather than left short: the time ring is binary-searched by @@ -945,7 +964,7 @@ void Acquisition::reserve_hole(std::uint32_t missing, const Partition& part, time_acc_ += rel; abs_time_acc_ += last_abs_stamp_; ++time_acc_n_; - if (tick % g == g - 1) { + if (group_end) { store.time().push(time_acc_ / double(time_acc_n_)); // Null in stream mode, where `time()` already is the absolute clock. if (plot::data::RingBuffer* abs = store.abs_time(); abs != nullptr) { @@ -963,8 +982,8 @@ void Acquisition::reserve_hole(std::uint32_t missing, const Partition& part, // pretend to. One slot per edge, against a hole thousands of slots wide. value_acc_[i] += std::numeric_limits::quiet_NaN(); ++value_acc_n_[i]; - const u64 period = g * part.stride[i]; - if (tick % period != period - 1) continue; + const u64 mask = acc_mask_[i]; + if ((tick & mask) != mask) continue; store.values(i).push(static_cast(value_acc_[i] / double(value_acc_n_[i]))); value_acc_[i] = 0.0; value_acc_n_[i] = 0; @@ -973,7 +992,7 @@ void Acquisition::reserve_hole(std::uint32_t missing, const Partition& part, // its source had. The trigger is *not* fed: a NaN comparison is false, so the engine // could not latch here anyway, and feeding it would advance its hold-off clock over // time nothing was sampled in. - if (tick % g == g - 1) { + if (group_end) { std::fill(math_sample_.begin(), math_sample_.end(), std::nanf("")); model_.push_math_sample(math_sample_); } diff --git a/src/acquisition.hpp b/src/acquisition.hpp index 72a88b8..1da3f70 100644 --- a/src/acquisition.hpp +++ b/src/acquisition.hpp @@ -180,6 +180,15 @@ class Acquisition { /// once the slot's samples have all been pushed. One entry per channel, refilled every slot. std::vector math_sample_; std::vector value_acc_n_; + /// Per channel, `time_stride * stride[i] - 1`: the group boundary as a **mask**, so the + /// innermost loop tests `(tick & mask) == mask` instead of `tick % period == period - 1`. + /// The divisor is a runtime value, so that modulo compiled to a real 64-bit `div` once per + /// channel per *wire* sample -- the filter reads every sample by design, so it was the + /// producer's largest avoidable cost. Both factors are powers of two (`plan_partition()` + /// doubles `time_stride` from 1, and every `stride[i]` is 1), which is what makes the mask + /// equivalent; it is sized and filled with the accumulators below, so a re-plan cannot + /// leave it addressing the previous partition's groups. + std::vector acc_mask_; double time_acc_ = 0.0; double abs_time_acc_ = 0.0; std::uint64_t time_acc_n_ = 0; diff --git a/src/model.cpp b/src/model.cpp index 721abed..7104fcc 100644 --- a/src/model.cpp +++ b/src/model.cpp @@ -118,8 +118,11 @@ Partition plan_partition(const std::vector& channel_dt, double buffer_by // Only now, with the ring as long as the budget can buy, decimate the shared clock // enough for the requested window to fit in it: deciding this earlier would over- // decimate by whatever factor the growth loop was about to add. A power of two keeps - // the ring capacities (themselves powers of two) in step and keeps `tick % period` - // cheap. The cap stops a nonsense request -- a window far past `--max-window` -- from + // the ring capacities (themselves powers of two) in step and lets the producer's group + // test be a mask (`Acquisition::acc_mask_`) rather than a modulo -- a modulo on a stride + // this function chose at runtime is a real 64-bit division, which the test *was* paying + // once per channel per wire sample. + // The cap stops a nonsense request -- a window far past `--max-window` -- from // spinning here; at 2^20 the stored rate is six decades below the wire rate and the // picture is averages of averages anyway. u64 m = 1;