improved perfs and event management
This commit is contained in:
1 parent
667ed0c100
commit
bb99be5889
17 files changed
+626
-59
No files matched your search
@@ -12,6 +12,33 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
|
||||
`plan_partition()` rather than recomputing the factor, because two expressions for that
|
||||
decision is how the buffer and the view end up disagreeing about the stored rate. It
|
||||
takes `mutex()` itself — do not call it holding the lock.
|
||||
- **A rebuild that would plan the *same* storage clears the store instead of allocating one.**
|
||||
`rebuild_locked()` compares the new `Partition` against the current one with
|
||||
`Partition::same_storage_as()` — every field that decides an allocation: `time_capacity`,
|
||||
`time_stride`, `abs_time`, `base_dt` and the per-channel `stride`/`value_capacity` vectors — and
|
||||
on a match calls `BufferStore::reset()` rather than constructing a new one. `base_dt` is in the
|
||||
comparison on purpose: it is not a size, but it is what the slots *mean*, and keeping samples
|
||||
under a changed `base_dt` would re-date the whole ring.
|
||||
**Construction is the cost.** A `RingBuffer<T>` holds a `std::vector<std::atomic<T>>`, which is
|
||||
value-initialised, so making a store touches every byte of `--buffer-mb` — about a second per
|
||||
gigabyte, mostly first-touch page faults — while `reset()` is one relaxed store per ring plus an
|
||||
`invalidate()` and an epoch back to zero. It must leave nothing of the old history behind, which
|
||||
is why it clears the time ring, the absolute-time ring, every value ring, and both the published
|
||||
window and the pyramid of every channel: a reused store that kept a stale published window would
|
||||
hand the renderer samples the new plan does not describe.
|
||||
`reset()` runs on the consumer thread under `ScopeModel::mutex()`, which is what makes a
|
||||
`RingBuffer::clear()` safe here at all — the producer holds that same mutex for the whole of
|
||||
`on_data()`.
|
||||
Every allocation that *is* made says so: `[udpscope] buffer allocated: N channels, S s span,
|
||||
T ms` on stderr. A start used to emit fifteen of those; two is a real start, and a count that
|
||||
climbs again is this rule having stopped applying.
|
||||
- **Enables are applied in one batch, because each one re-plans.** `set_signals_enabled()` takes
|
||||
the mutex once, moves every signal in the list, and re-plans and bumps the generation **once** —
|
||||
and not at all when nothing actually moved, which is the common case for a restored session
|
||||
re-enabling what is already on. Calling `set_signal_enabled()` in a loop is a buffer rebuild per
|
||||
signal; `apply_layout()` and the waveform loader collect their wanted signals into one vector for
|
||||
that reason, and `apply_layout()` records the session's span **first**, with no channels bound, so
|
||||
the `apply_time_window()` that follows re-plans nothing.
|
||||
- **The shared clock is decimated to fit the requested window; channels never are.** A
|
||||
window too long to store at the wire rate used to be refused (the control was capped to
|
||||
`Partition::span_s`), which made a 600 s view of a 1 MS/s stream unreachable.
|
||||
|
||||
@@ -207,6 +207,23 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
|
||||
thread and `gtk_picture_set_paintable()` hands over the result -- and a NULL texture at least says
|
||||
so. A picture that must be on screen is also a picture with a **height request**: `can_shrink`
|
||||
alone asks for no height at all in a box.
|
||||
- **A cache that is the *only* source of a widget's content must be created by the thing that reads
|
||||
it.** The event pane's thumbnails come from an `EventThumbCache` hung off the list box with
|
||||
`g_object_set_data_full`, and `event_preview_texture()` answers out of that cache or not at all.
|
||||
Nothing ever created it — so for as long as the pane existed, **every** row came up without a
|
||||
picture, which is what "event screenshot are unusable and sometime missing" was. A missing cache
|
||||
there is not a slow path, it is a blank one; `event_thumb_cache()` therefore creates one on first
|
||||
use. Owned by the **list**, because nothing else has the pane's lifetime: textures of a list that
|
||||
no longer exists are so much held memory, and the destroy notify is where they are unref'd.
|
||||
- **Hover magnification is a tooltip, not a popover of ours.** A row's thumbnail is 138 px wide and
|
||||
the stored preview is 480: the small one says *which* event this is, and the zoom says what is in
|
||||
it. GTK's own tooltip (`gtk_widget_set_has_tooltip` + `query-tooltip` + `gtk_tooltip_set_custom`)
|
||||
is shown, placed, dismissed **and destroyed** by GTK, and takes no pointer grab. A popover per row
|
||||
would be a parented widget to unparent on every rebuild of the pane — one defect below — and a
|
||||
grab to lose, for a thing with no interaction in it. Two details: `can_shrink` must be FALSE on the
|
||||
tooltip's picture or the tooltip is given the smallest size its content allows (the opposite of the
|
||||
point), and the path handed to the handler has to be **owned by the closure**
|
||||
(`g_signal_connect_data` with a destroy func), since the record it came from is a loop variable.
|
||||
- **Unparenting a popover inside its own `closed` emission is a use-after-free.** Dismissals go
|
||||
through `gtk_popover_popdown()`, which emits `closed`; a handler that unparents the popover there
|
||||
drops the reference the parent held, the widget dies, and GTK's own popdown path then runs on freed
|
||||
|
||||
@@ -31,6 +31,21 @@ Part of [CLAUDE.md](../../CLAUDE.md). These are rules that cost a defect to lear
|
||||
reasons (an event pin) call `sync_window_display()` alone, which only restates the control and
|
||||
never moves the plot — calling `apply_time_window()` there would clamp a legitimately short
|
||||
capture up to the requested window.
|
||||
- **With nothing bound, `set_time_window()` frames `{-window, 0}` and never `{0, window}`.** Under
|
||||
a following x group a range states two things — its span, and *through its maximum* where the
|
||||
window sits behind the live edge — so a maximum **at** the edge is what makes the offset zero.
|
||||
When there is a live edge to read that is `{newest - window, newest}`; when there is not, the
|
||||
edge the live view would put at x = 0 is 0, so the equivalent statement is `{-window, 0}`.
|
||||
Asking for `{0, window}` there is an offset of a *whole window*. This is not a rare fallback:
|
||||
"no samples bound" is the state every structural change leaves behind, and
|
||||
`apply_time_window()` re-plans the rings and *then* calls this, so it is the axis a mode change
|
||||
is drawn with until the first sample lands. With `{0, window}` the two or three frames after
|
||||
every event or waveform open drew the time base one screen to the left of where the following
|
||||
frames drew it — a trace that jumps a screen sideways and comes back, which is what "opening
|
||||
events or even waveform sometime result in data flickering (like time base) and is unusable"
|
||||
was. The reading is the dump's traceless paint counts: `mode=live` 38 → 0 and `mode=waiting`
|
||||
2 → 40, the latter being the point — a waiting view now paints its empty axis in the *same*
|
||||
place every frame instead of twitching into a framed one.
|
||||
- **The x labels go relative on the tick *step against the label resolution at the axis'
|
||||
magnitude*, never on the span.** `RowSet::update_time_labels()`'s question is only ever "can
|
||||
this step be written at this magnitude": `TickFormat::Time` carries three significant figures
|
||||
|
||||
@@ -987,9 +987,10 @@ configuration that carries settings, is the defect this exists for — a saved h
|
||||
[events] live, recording off: button_visible=0 pane=0 pane_w=0 tick=1 recording=0
|
||||
[events] recording on, no trigger: button_visible=1 pane=0 pane_w=0 tick=1 recording=1
|
||||
[events] after trigger on: button_visible=1 pane=0 pane_w=0 tick=1 recording=1
|
||||
[events] dir=/tmp/k5/cache/udpscope/events/127.0.0.1-21023 n=1 newest: dur=2.000000 pre=1.000000 post=1.000000 edge=1.000100 signals=4 samples=20001 rate=10000 seq=1 thumb_bytes=230454 recorded=1 pane=1
|
||||
[events] activated row0: replaying=1 pane=1 pane_w=240 tick=0 window=2.000 thumbnail=1 thumb_size=126x42 thumb_visible=1
|
||||
[events] replayed: channels=4 samples=20001 replaying=1 waveform=1
|
||||
[events] preview redrawn: signals=3 stored=480x132 row_drawn=1
|
||||
[events] dir=/home/u/.cache/udpscope/events/127.0.0.1-21023 n=3 newest: stem=20261013-184140-0003 dur=2.000000 pre=1.600000 post=0.400000 edge=5.660100 signals=3 samples=20001 rate=10000 seq=3 thumb_bytes=190134 recorded=1 pane=1
|
||||
[events] activated row0: replaying=1 pane=1 pane_w=240 pane_box=228 tick=0 window=2.000 axis=[-1.600,0.400] marker=0.000 thumbnail=1 thumb_size=138x46 thumb_visible=1 rows_kept=1 selected=2/2
|
||||
[events] replayed: channels=3 samples=20001 replaying=1 waveform=1
|
||||
[events] live again: replaying=0 waveform=0
|
||||
[events] recording off, trigger off: button_visible=1 pane=1 pane_w=240 tick=1 recording=0
|
||||
[events] recording off, trigger on: button_visible=1 pane=1 pane_w=240 tick=1 recording=0
|
||||
@@ -1001,21 +1002,35 @@ configuration that carries settings, is the defect this exists for — a saved h
|
||||
[events] group left: replaying=0 kind=0 waveform=0
|
||||
```
|
||||
|
||||
`thumb_size` is the built size of the row's picture (96x32 logical, plus the CSS border), so it moves with
|
||||
the font and the display's scale factor; what it must never be is `0x0`.
|
||||
`thumb_size` is the built size of the row's picture: `kEventThumbW` wide, and as tall as the *stored*
|
||||
preview's own aspect makes it at that width, clamped to `[kEventThumbH, kEventThumbMaxH]`. So it moves
|
||||
with the event's **signal count** (a three-signal event sits on the floor at `138x46`, a six-signal one
|
||||
reads `138x75`) as well as with the CSS border and the display's scale factor. What it must never be is
|
||||
`0x0`, and a height pinned to the floor for *every* event says the aspect fit was dropped and tall
|
||||
previews are being squashed again.
|
||||
|
||||
`[events] preview redrawn:` is the repair path, and the probe forces it: step 1 deletes the newest
|
||||
`.bmp`, drops the thumbnail cache and refreshes the pane, so this line is `EventStore::redraw_preview()`
|
||||
drawing the picture again from the `.npy`. `stored` is the file it wrote, read back out of the BMP
|
||||
header — `480` wide and `44 * signals` tall — and `row_drawn=1` says the row got a paintable from it.
|
||||
The probe therefore needs a cache a **previous** run populated: it fires ~4.5 s in, before this run's
|
||||
first latch.
|
||||
|
||||
| field | value it must read | the defect a wrong reading names |
|
||||
| --- | --- | --- |
|
||||
| `n`, `stem`, `seq` | one entry per latch, named by its time and the run's counter | two events in one second overwriting each other, or a cache that is never written |
|
||||
| `dur` | `pre + post` for the event's window | the snapshot's own window not recorded, so the list disagrees with the picture |
|
||||
| `signals`, `samples` | the event's channels and rows | a table built from the *rows* instead of the snapshot: only what happened to be plotted |
|
||||
| `thumb_bytes` | a real 24-bit BMP (`54 + 480*160*3`) | a preview that is empty or missing -- the widget snapshot of a `GtkGLArea`, which is NULL |
|
||||
| `thumb_bytes` | a real 24-bit BMP, `54 + 480 * (44 * signals) * 3` rounded up per row to a multiple of four -- so it *grows with the signal count* (190134 at three signals, 380214 at six) | a preview that is empty or missing -- the widget snapshot of a `GtkGLArea`, which is NULL -- or a picture back on a fixed total height, which gave a six-signal event 26 px a signal and an eight-signal one 20: a smear, not a waveform |
|
||||
| `recorded` | 1 while the trigger panel's switch is on | the switch not wired, or recording defaulting off (it must be on: the cache is for the event you did not expect) |
|
||||
| `pane` | 1 once the history button has been clicked | the button's handler not revealing the pane |
|
||||
| `live` | `button_visible=0 pane=0** | a pane or a button on screen with no trigger mode: a column of nothing beside a live plot |
|
||||
| `after trigger on` | `button_visible=1 pane=0` | the pane revealed by trigger mode alone (it must be on demand from the toolbar button), or the button missing in trigger mode |
|
||||
| `recording off, trigger off` and `recording off, trigger on` | `button_visible=1 pane=1 recording=0` in both **once the cache holds anything** (the probe's own run does by this point) | the button going away with the switch when events are already written -- an event on disk outlives the switch that wrote it -- or `pane_w` not being 240 with the pane open. With an *empty* cache the same two lines read `button_visible=0 pane=0`, which is the empty column the item exists to prevent |
|
||||
| `thumbnail`, `thumb_size`, `thumb_visible` | 1, a non-zero size, and 1 | the picture loaded through `gtk_picture_set_filename()`, which fails silently (the paint-able is there, the allocation is there, nothing is drawn), or a `GtkPicture` given a size request and no cap -- its natural size is its content, so the row's text is squeezed off. `thumb_size` is the reading that catches both |
|
||||
| `thumbnail`, `thumb_size`, `thumb_visible` | 1, a non-zero size, and 1 | the picture loaded through `gtk_picture_set_filename()`, which fails silently (the paint-able is there, the allocation is there, nothing is drawn), or a `GtkPicture` given a size request and no cap -- its natural size is its content, so the row's text is squeezed off. `thumb_size` is the reading that catches both. It also catches the cache never being *created*: `event_preview_texture()` answers from the cache or not at all, so a reader that does not create one leaves **every** row blank, which is what `thumbnail=0` meant for as long as it read 0 |
|
||||
| `preview redrawn` | `stored=480x<44*signals>` and `row_drawn=1` | a preview that cannot be repaired. The `.npy` is the event and the `.bmp` is only a picture of it, so a write that failed, a truncated file or a cache from an older build must be redrawn rather than shown as a blank row -- `row_drawn=0` is a row the user reported as "sometimes missing" |
|
||||
| `axis`, `marker`, `pane_box` (in `activated row0`) | `[-pre, +post]` of the event just opened, `marker=0.000`, and a `pane_box` close to `pane_w` | an opened event framed on something other than its own pin; a trigger line left **parked** (a large negative) rather than on the edge, which is a file opened without its marker; or a pane whose *rendered* width is 0 while the divider says 240 -- the position is what was asked for, `pane_box` is what the pane actually got, and a divider at 240 with a zero-width pane is still no pane |
|
||||
| `rows_kept`, `selected` (in `activated row0`) | `rows_kept=1` and `selected=N/N` -- the counts either side of the activation equal | the list **rebuilt** by the activation. It is the row *object* that is compared, not its index: a rebuilt list has a row 0 too, and only identity tells a list that was left alone from one that was torn down under the user's scroll position and their comparison selection. `mark_events_on_screen()` must rewrite the titles in place |
|
||||
| `replaying` (last line) | 0 | a replay with no way back: *Live* must start a fresh client and thread |
|
||||
| `pane_w` | the divider's position: 0 while the pane is closed, its remembered width (240 by default) while it is open | the pane reserving a column while closed (a paned hands its start child its minimum unless the position says otherwise), or a width that is not remembered |
|
||||
| `tick` | 1 in live mode, **0** once a file is on screen | the signal list still offering "acquire this signal" for samples that are already stored |
|
||||
@@ -1252,10 +1267,20 @@ refusal it may still hold the previous text.
|
||||
## `[enable-all-probe]` and switching a signal off
|
||||
|
||||
```
|
||||
[enable-all-probe] 4 channel(s) enabled and assigned to plots
|
||||
[enable-all-probe] 4 channel(s) enabled and assigned to plots, notice='connecting to 127.0.0.1:21023 -- planning the 200 MB buffer'
|
||||
[enable-all-probe] switched 'sine' off: traces_left=0 (want 0) notice=''sine' is off: 1 trace left the plots'
|
||||
```
|
||||
|
||||
The first line's `notice` is the **startup** notice, and it is read here because this probe is the
|
||||
earliest thing that reads the status bar. It is set as the window is presented -- before the first
|
||||
CONFIG can take the model's mutex, so before anything can block -- and it names the host and the
|
||||
buffer being planned, because that is the window's only answer to "why is nothing happening yet".
|
||||
An empty `notice` here is the window that used to come up silent while a large buffer was allocated:
|
||||
at `--buffer-mb 4000` that is the dialog disappearing and then over a second of a frozen, blank
|
||||
window. (The buffer is allocated on the producer thread under `ScopeModel::mutex()`, which the frame
|
||||
tick also takes, so the freeze is real and cannot be papered over in the UI -- see the
|
||||
`[udpscope] buffer allocated:` line below.)
|
||||
|
||||
`UDPSCOPE_ENABLE_ALL_MS` enables every signal and assigns the acquired channels across the rows, then
|
||||
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
|
||||
@@ -1270,6 +1295,30 @@ which must be untouched:
|
||||
[enable-all-probe] switched 'sine' off: traces_left=0 (want 0) math_off=[math x2,math3] math_on=[math lp] notice=''sine' is off: 1 trace left the plots; math off: math x2 and 1 more'
|
||||
```
|
||||
|
||||
## `[udpscope] buffer allocated:`
|
||||
|
||||
```
|
||||
[udpscope] buffer allocated: 4 channels, 839 s span, 74 ms
|
||||
```
|
||||
|
||||
Not a probe — it is printed by `ScopeModel::rebuild_locked()` in every build, because this is the one
|
||||
operation in the application whose duration a user can see with no instrument: the store's rings are
|
||||
`std::vector<std::atomic<T>>` and so **value-initialised**, which touches the whole `--buffer-mb`
|
||||
(roughly a second per GB, mostly first-touch page faults), and it runs on the producer thread holding
|
||||
`ScopeModel::mutex()` — which the frame tick also takes. So the line's `ms` is a frozen window, not
|
||||
bookkeeping.
|
||||
|
||||
**It is the count that is the assertion, not the duration.** One line per *real* change of plan. A
|
||||
rebuild whose plan describes the storage already allocated clears the store instead
|
||||
(`Partition::same_storage_as()` → `BufferStore::reset()`) and prints nothing, so a startup or a mode
|
||||
change that reads more than one or two of these is a re-plan that should have been coalesced. Startup
|
||||
asked for **fifteen**; it now asks for two or three. A run at `--buffer-mb 4000` measured those
|
||||
coalesced allocations at 1148 ms each and a max frame tick of 1186 ms — the same number, which is the
|
||||
evidence that the freeze *is* this call and not something downstream of it.
|
||||
|
||||
The companion reading is the startup `notice` on the `[enable-all-probe]` line above: the allocation
|
||||
cannot be made free, so the window must at least say what it is doing while it happens.
|
||||
|
||||
## `[math-probe] graph edit:`
|
||||
|
||||
```
|
||||
|
||||
@@ -26,6 +26,12 @@ all. A fifth warning, or any critical, is a finding.
|
||||
An *incremental* build reads **two** of those four: the generated GResource `.c` files are not
|
||||
recompiled when only `src/` changed, so their `-Woverlength-strings` do not reappear.
|
||||
|
||||
A third reading every run offers for free: **how many `[udpscope] buffer allocated:` lines it
|
||||
printed.** The line is not behind a probe — it is in every build — and one line is one *real* change
|
||||
of buffer plan, each of which blocks the main thread for the time it takes to touch the whole
|
||||
`--buffer-mb`. A normal run prints two or three. Fifteen at startup is the defect it was added for;
|
||||
see [probe-readings.md](probe-readings.md).
|
||||
|
||||
That second baseline used to be "exactly two, `gtk_widget_get_height` and `gtk_widget_get_width`,
|
||||
from the dump asking a destroyed widget for its size", and tolerating them was a mistake: the dump
|
||||
runs from the destroy handler, so the widget pointer is **freed memory**, and a critical is only
|
||||
|
||||
Reference in new issue
Block a user