perf(tool): ask for the save notice's progress only when the save pipeline's revision moved

[flugifix]

What and why

Closes #771

While a save notice is on screen, M.run in ui/home.lua asked SavePipeline.getProgress() on every run of the tool, and repainted only when a snapshot string of four of its fields had changed. getProgress() (tasks/msp/save_pipeline.lua) builds a new table on every call, so each run paid for a table only to find that nothing it draws had changed. A failed outcome stays until the pilot dismisses it, so this went on for as long as the notice was read.

The pipeline now keeps a revision number in its shared state (_G.rfsuite.tasks.savePipeline, where the run already lives, since every page loads its own copy of the module) and raises it wherever a field getProgress() reads changes: a phase entered (which also covers the step index), the EEPROM commit acknowledged, a save started, finished or reset, an outcome claimed, expired or dismissed, a stored save dismissed when the tool closes. SavePipeline.getRevision() returns it. M.run compares it with the number it saw last and asks for the table only when it moved; the snapshot comparison still decides whether that is a repaint, so the revision only has to move whenever something may have changed. Fields the notice does not report (the probe's bookkeeping, a dismissal requested before the commit) do not raise it. getProgress() still returns a new table on every call, and the notice's draw in buildUI still asks for it.

What it costs, master 1b8d789bf against this branch

The issue's reproduction on desktop Lua 5.3.6 (64-bit), 1 000 calls with the collector stopped, extended with an instruction count (debug.sethook(..., "", 1), as bin/accounting counts) and with the tool's own per-run check as a row: the block of M.run, taken from each tree's ui/home.lua, with that tree's save_pipeline.lua. Both trees in the same run, in both row orders:

call	master bytes / call	branch bytes / call	master instructions	branch instructions
control: empty function	0	0	4	4
control: a table of eight numbers	184	184	14	14
getProgress(), failed outcome on screen	568	568	31	31
getProgress(), write in progress	568	568	64	64
the tool's run check, failed outcome on screen	568	0	78	34
the tool's run check, write in progress	568	0	161	84
the tool's run check, no save	0	0	37	31

getProgress() itself is unchanged and still allocates its table; the tool now asks for it only when the revision moved. (A one-off allocation of up to 1.6 kB per 1 000-call window shows on both trees and shrinks to a quarter over 4 000 calls, so it is not per call.)

lua5.3 bin/accounting/measure.lua, master against this branch (--check: 142 rows, 0 failures on both). Eight rows moved, each by +3, every other row is unchanged: the builds of the pages that load the save pipeline, whose module now creates two more functions when it loads.

row	master	this branch
page.setup_configuration_page.build	3639	3642
page.setup_esc_motors_rpm_page.build	3719	3722
page.setup_esc_motors_telemetry_page.build	3353	3356
page.setup_esc_motors_throttle_page.build	3542	3545
page.setup_gps_page.build	3276	3279
page.setup_mixer_swash_page.build	3770	3773
page.setup_mixer_tail_page.build	3428	3431
page.setup_radio_config_page.build	3259	3262

None of its scenarios runs a save, so the run check is priced by the table above, not there. How many runs a second the tool gets on a radio was not measured; nothing was run on a radio for these figures, and a radio's Lua is 32-bit, so the bytes per table are smaller there and the number of tables is the same.

What does not change

What the tool decides on every run. Both trees' save_pipeline.lua (two copies of the module over one shared state, as a page and the tool hold them) were driven through their API only — start, the queue's replies, errors and a lost message, wakeup with a stepped clock, the connect chain, dismiss, dismissWhenStored, reclaim, takeResult, reset — in twelve scripted saves: success with and without a restart (the notice clearing itself after its linger), a failed write standing until dismissed, a restart save dismissed after the commit and claimed on re-entry, the tool closed before and after the commit and reopened, a step timeout, a board that never comes back, a second save refused while one runs, a new save replacing a standing outcome, a reset during a save, a failed restart command and a save with no writes. After every event the tool's run check ran twice. Over 286 runs, whether a repaint was scheduled, whether the notice is visible and the snapshot are identical on master and on this branch, line for line (65 repaints on each); the branch asked for the table 84 times instead of 286. Removing any one of the eight raises that the scripted saves can tell apart makes the branch differ from master (the two others, in start and at the commit acknowledgement, are always followed by a phase change today and are kept so that the change is marked where it happens).

On the EdgeTX 2.12.2 simulator (TX16S MK3) with a Rotorflight 4.6.0 SITL flight controller, the standalone tool (no dashboard widget on the card), master against this branch, and a third card with the branch's raise at the end of a run removed as a control. A test hook, identical on all three cards and in files this change does not touch, started saves the way a page does, through its own copy of the pipeline, and logged every notice the tool drew: a save of two writes (the notice follows both steps, shows Saved and clears itself), the same save with its writes kept from reaching the board (the notice shows Not saved after the retries, and still shows it 12 seconds later), a dismissal, a save with a restart (the board exits on the reboot command and does not come back: Restarting, Waiting, then The flight controller has not come back yet, standing), and RTN, which read that notice away. Master and this branch drew the same notices in the same order, and the 23 screenshots taken along the way match pixel for pixel except one taken 0.5 s into the first save, which caught a different step. The one difference in the draw logs is the Applying settings notice of the EEPROM commit: it was drawn on whichever card's commit lasted 0.15 s and not where it lasted 0.05 s (one save each way on each tree), because the tool repaints on the run after a change, on master as here. The control card never drew Not saved or has not come back yet: its notice stayed on whatever it had drawn last before the save ended (P771 step 1, Waiting for the flight controller...) until the dismissal, so the screen would show a counter that misses a change. The bench says nothing about memory or instruction counts.

Related: #13 (where the query and the snapshot check came from). #763 is the same shape for the dashboard's connection check (tasks/msp/runtime.lua) and is answered separately by #788; this change leaves that file alone, and the two share no file.

Checklist

See CONTRIBUTING.md for each rule.

• A release note is added under release-notes/, or a line below says why not.
• docs/ is updated, or a line below says why no page changes.
• Every string a pilot reads goes through i18n.

Documentation: no page -- nothing a pilot can observe changes; the save notice shows, updates, clears and waits for its dismissal exactly as docs/reference/saving.md describes it, and no page under docs/ describes the save pipeline's API.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Repository: rotorflight/rotorflight-lua-edgetx-suite/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: d2596be9-fc56-44d4-826b-54a5f956c3cb

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[heliflieger]

The revision is raised at every point where a field getProgress() reports can move, and the snapshot comparison still decides the repaint -- so the revision only has to be conservative, which is the right way round: an invisible raise costs an integer, a missed raise would leave a stale notice. state.lastSaveRevision starts nil, so the first run always asks. The twelve-save scripted drive with the per-raise removal controls is what makes "nothing the tool decides changed" a measured statement rather than an assertion. Approving.

[heliflieger]

lastSaveRevision starts nil, so the first run always fetches the table; after that an invisible raise costs only the comparison. The counter lives in the shared state beside the run it describes, so a second copy of the module reads the same number -- and even a state that reset it to 0 would differ from a non-zero lastSaveRevision and ask again.