Repository navigation
perf(dashboard): build the Urban, Grid and Deck render key without a temporary list - #787
Conversation
…temporary list
L.renderKey runs every 0.5 s in the host's state pass and joined its terms with
table.concat({ ... }, "|"), although the comment above it said no table was
allocated. The table constructor was most of what a call cost: 201 bytes per
call for Urban and Grid, 217 for Deck, on desktop Lua 5.3 with unchanged inputs.
The key is now one `..` expression with the same terms, the same order and the
same separator; the diversity term is parenthesised because `..` binds tighter
than `and`/`or`. A call with unchanged inputs now allocates 1 byte, and 200 or
216 bytes less with a long model-image path. The key is byte-identical for
every input, and the comment says what the function still allocates.
Closes rotorflight#765
The key always carries a model picture's path -- the snapshot falls back to the suite logo when no picture is found -- so it is longer than Lua 5.3's 40-byte short-string limit and is a new string on every call. Say that in the comment instead of "nothing", and quote the saving in the release note for that case: 200 bytes less per call in Urban and Grid, 216 in Deck.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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. Comment |
heliflieger
left a comment
heliflieger
left a comment
There was a problem hiding this comment.
The three copies carry the same terms, in the same order, with the same separator, and a key comparison over 31 inputs per theme is the right check for a change whose whole point is that nothing observable moves -- including the controls that must differ. The comment above the function now says what a call still allocates (the key string) instead of claiming no allocation, which is the part the issue was really about. Approving.
| .. "|" .. (L.diversity(state) and "div" or "") | ||
| -- The phase, and it is `themePhase` rather than `flightMode` on purpose. | ||
| -- | ||
| -- A host that knows the two refinement phases puts the widget into `armed` on the arm edge |
There was a problem hiding this comment.
These parentheses are load-bearing and correct: .. binds tighter than and/or, so .. L.diversity(state) and "div" or "" would have concatenated the boolean instead of the string. Every other term is an explicit tostring(...), so no number's rendering can change either way.
2 participants
What and why
Closes #765
L.renderKeyinwidgets/dashboard/themes/urban/layout.lua,grid/layout.luaanddeck/layout.luaruns every 0.5 s in the host's state pass and returnedtable.concat({ "urban", tostring(zone.w), ... }, "|"), although the comment above it said the function makes no table allocation. The table constructor was most of what a call cost. The code is mine; Grid and Deck carry it as copies of Urban's.The key is now one
..expression with the same terms, in the same order, with the same separator. The diversity term (L.diversity(state) and "div" or "") is parenthesised, because..binds tighter thanand/or; every other term already is atostring(...)call. The comment now says what a call still allocates: the key string itself. In practice that is on every call, because the key always carries a model picture's path (the snapshot falls back to the suite logo's when no picture is found), which takes it over Lua 5.3's 40-byte short-string limit, so the key is a new string each time on master and on this branch alike. The issue's reproduction uses an emptyderivedand therefore a short key; with the logo's path in the key, master allocates about 319 bytes per call, not 201.What it costs, master
1b8d789bfagainst this branchThe issue's reproduction (Lua 5.3.6, 64-bit desktop,
bin/accounting's stubs, unchanged inputs, collector stopped, 1000 calls), extended with instructions per call (debug.sethook(..., "", 1), asbin/accountingcounts) and two more inputs:derived.model_imageset to the suite logo's path, and to a picture path with Deck'slqSourceset. Both trees in the same run, each run twice in opposite theme order; the two runs agree.lqSourceThe saving is the table, exactly: 200 bytes per call for Urban and Grid (nine slots), 216 for Deck (ten), whatever the key holds. What is left is the key string. The count hook bills three more instructions per call (four for Deck): one constant load per separator replaces the table constructor and the call to
table.concat, whose own work in C (its buffer and the copies) the hook does not count. At two calls a second that is 6 to 8 instructions a second against about 400 to 430 bytes a second less garbage.lua5.3 bin/accounting/measure.lua, master against this branch: the output is identical line for line (142 rows, 0 failures on both), so no row moved. Its passes do not reachrenderKey, which the host calls at most every 0.5 s.Nothing was run on a radio for these figures; a radio's Lua is 32-bit, so the bytes are smaller there.
What does not change
The key, for every input. Compared in one run between master and this branch, 31 inputs for each of the three themes (93 keys): each term changed on its own (zone width, also as a float, and height; the cell count, also through
themeConfig.v_maxand with 0; the model picture, short and long; diversity astrue,2,0andfalse;themePhase, andflightModeas its fallback; the transmitter voltage moved, moved back, moved inside its 0.15 V dead band andnil; the battery pick pending, not a table andnil; Deck'slqSource), plus unrelated telemetry,statenil,zonenil, both nil andstate = {}. 0 of 93 keys differ. On both trees every term change moves the key, and the dead-band move, the unrelated telemetry and thearmedmode under an unchangedthemePhasedo not. A copy whose last separator is/differs in 93 of 93; a copy that drops Urban'spendingterm or Deck'slqSourceterm is caught by the term check on exactly those two inputs.On the EdgeTX 2.12.2 simulator (TX16S MK3, 800x480) against a Rotorflight 4.6.0 SITL flight controller, with a picture under
/IMAGES/named after the board's craft name, the dashboard with this branch connects, builds Urban, Grid and Deck and draws the picture in each, with the same number of theme builds per run as master (two in every run). Master and this branch differ only in the clock and, on Urban and Grid, in the arming-disabled reason in the status bar, which rotates every two seconds; two runs of master differ there as well. Urban, Grid and Deck differ from each other in more than 129 000 pixels, so the comparison sees which theme drew. No run logged a Lua error.Related: #755 and #756, which brought Grid and Deck with their copies of the function. #763 and #764 (merged as #775) are the other allocations on the same state pass. #777 changes the same three
layout.luafiles elsewhere (the statistics screens); the two merge without a conflict (checked against its current head).Checklist
See CONTRIBUTING.md for each rule.
release-notes/, or a line below says why not.docs/is updated, or a line below says why no page changes.Documentation: no page -- nothing a pilot can observe changes; the key, and so every rebuild, is identical for every input, and no page under
docs/describes what the render key allocates.