perf(ui): pass the label as an argument and share one empty spec
The label was the last field the wrapper wrote into a caller's spec, and on a two-key text spec it was the key that forced a rehash. It now rides as an argument like type, so ui.text writes nothing at all, and a button's padding and centring are the tree's defaults rather than fields patched in from Lua. With no builder writing into a spec any more, every spec-less call can share one immutable table instead of allocating its own. It is frozen with a __newindex that raises, because reintroducing a write would otherwise leak a field into every spec-less node built afterwards -- a fault with no symptom anywhere near its cause. One lua_State exists at a time, so the guard costs about 100 bytes in total. ui.spacer passes its spec through rather than copying w and h into a fresh table, and the style alias on a text spec is dropped for textStyle, which no caller used. ui.rebuild now collects before the repaint rather than after it. The collect was already there and its comment already named the hazard, but the painter is the very next thing to want a large contiguous block for its band, and it was being handed a heap still holding a screen's worth of dead spec tables -- a C++ allocation gets no emergency collection the way a failed Lua one does. Worth 5.6kB of free heap at paint time; the largest block is unchanged, because the freed specs are small and scattered. Measured in the emulator, 12 sensors / 131 nodes: live Lua at build end 79.9kB -> 79.7kB and build time unchanged at 52ms. The raw heap figure looks worse because removing the rehashes also removed the allocation pressure that had been pacing the incremental collector, so the dead spec tables now sit uncollected until something asks for them; live usage is what did not change. ui.lua stripped bytecode 9499 -> 9340 bytes.
This commit is contained in:
+33
-21
@@ -14,7 +14,6 @@ local ui = {}
|
||||
---@field at? table
|
||||
---@field label? string
|
||||
---@field font? GuiFont
|
||||
---@field style? GuiTextStyle
|
||||
---@field fit? integer Maximum label width.
|
||||
---@field background? GuiColor
|
||||
---@field face? GuiColor
|
||||
@@ -165,8 +164,19 @@ tree.setPainter(function(id, x, y, w, h, clipX, clipY, clipW, clipH)
|
||||
end
|
||||
end)
|
||||
|
||||
local function build(spec, kind)
|
||||
spec = spec or {}
|
||||
-- Shared by every builder called without a spec. Safe only because nothing writes
|
||||
-- into a spec any more: type, interactive and label are arguments, and a button's
|
||||
-- padding is the tree's default rather than something patched in here. Frozen so
|
||||
-- that reintroducing a write fails at the write instead of leaking a field into
|
||||
-- every spec-less node built afterwards, which is a fault with no symptom near it.
|
||||
local EMPTY = setmetatable({}, {
|
||||
__newindex = function()
|
||||
error("ui specs are read-only during a build", 2)
|
||||
end,
|
||||
})
|
||||
|
||||
local function build(spec, kind, label)
|
||||
spec = spec or EMPTY
|
||||
-- Nodes outside a build would reset the arena under the screen already on the
|
||||
-- panel, chrome included. Rebuilding is the only way to change one.
|
||||
if laidOut then
|
||||
@@ -178,7 +188,7 @@ local function build(spec, kind)
|
||||
-- twice for the small ones -- on the build path that peaks the heap. Children
|
||||
-- come straight off the array part, which tree.create never reads.
|
||||
local id = tree.create(nil, spec, kind,
|
||||
spec.on_enter ~= nil or spec.on_exit ~= nil or spec.on_click ~= nil)
|
||||
spec.on_enter ~= nil or spec.on_exit ~= nil or spec.on_click ~= nil, label)
|
||||
for _, child in ipairs(spec) do
|
||||
tree.attach(id, child)
|
||||
end
|
||||
@@ -237,25 +247,25 @@ end
|
||||
---@param spec UiSpec
|
||||
---@return NodeId
|
||||
function ui.spacer(spec)
|
||||
spec = spec or {}
|
||||
return build({ w = spec.w, h = spec.h }, "box")
|
||||
-- Passed through rather than copied into a fresh {w, h}: tree.create reads what it
|
||||
-- needs and ignores the rest.
|
||||
return build(spec or EMPTY, "box")
|
||||
end
|
||||
|
||||
---@param text string
|
||||
---@param spec? UiSpec
|
||||
---@return NodeId
|
||||
function ui.text(text, spec)
|
||||
spec = spec or {}
|
||||
spec.label = text
|
||||
spec.textStyle, spec.style = spec.style, nil
|
||||
return build(spec, "text")
|
||||
-- Writes nothing: the label rides as an argument, so a two-key text spec stays
|
||||
-- two keys instead of rehashing to four.
|
||||
return build(spec or EMPTY, "text", text)
|
||||
end
|
||||
|
||||
---@param text string
|
||||
---@param spec? UiSpec
|
||||
---@return NodeId
|
||||
function ui.label(text, spec)
|
||||
spec = spec or {}
|
||||
spec = spec or EMPTY
|
||||
-- Sized by the intrinsic width tree.create measures from the same text, font and
|
||||
-- style, so this measures only when the text has to be truncated. Writing w and h
|
||||
-- here measured the string a second time and grew the spec by two keys, which on a
|
||||
@@ -276,14 +286,13 @@ end
|
||||
---@param spec UiSpec
|
||||
---@return NodeId
|
||||
function ui.button(spec)
|
||||
spec = spec or {}
|
||||
spec.pad = spec.pad or 8
|
||||
spec.align = spec.align or "center"
|
||||
spec = spec or EMPTY
|
||||
local label, font = spec.label, spec.font
|
||||
spec.label = nil
|
||||
local id = build(spec, "button")
|
||||
if label then
|
||||
tree.create(id, { label = label, font = font or screen.FONT_UI }, "text")
|
||||
-- The button's own spec.label is not read by tree.create, so it needs no
|
||||
-- clearing; the child text node carries the label instead.
|
||||
tree.create(id, font and { font = font } or EMPTY, "text", false, label)
|
||||
end
|
||||
return id
|
||||
end
|
||||
@@ -412,13 +421,16 @@ function ui.rebuild()
|
||||
end
|
||||
tree.dropScratch()
|
||||
laidOut = true
|
||||
-- A build allocates a spec table per node and drops them all here, and the painter is
|
||||
-- the very next thing to want a large contiguous block for its band. Collecting before
|
||||
-- the repaint rather than after it sizes the band against the heap that exists, not one
|
||||
-- still holding a screen's worth of dead spec tables -- a C++ allocation gets no
|
||||
-- emergency collection the way a failed Lua one does. It costs a few milliseconds on a
|
||||
-- screen change nobody can see, and leaves the heap in a known state instead of one that
|
||||
-- depends on when the incremental GC last ran.
|
||||
collectgarbage()
|
||||
screen.clear(ui.theme.background)
|
||||
tree.draw(root)
|
||||
-- A build allocates a spec table per node and drops them all here, and the next thing an
|
||||
-- app does may be the one that needs a contiguous WiFi buffer. Collecting now costs a few
|
||||
-- milliseconds on a screen change nobody can see, and leaves the heap in a known state
|
||||
-- instead of one that depends on when the incremental GC last ran.
|
||||
collectgarbage()
|
||||
end
|
||||
|
||||
-- Pushes a node's float offset into the tree, then snaps our copy back to whatever the
|
||||
|
||||
+2
-2
@@ -71,7 +71,7 @@ tree = {
|
||||
reset = function()
|
||||
nodes, focus, buttonCount = {}, nil, 0
|
||||
end,
|
||||
create = function(parent, spec, kind, interactive)
|
||||
create = function(parent, spec, kind, interactive, label)
|
||||
local id = #nodes + 1
|
||||
local x = 0
|
||||
if kind == "button" then
|
||||
@@ -81,7 +81,7 @@ tree = {
|
||||
nodes[id] = {
|
||||
parent = parent,
|
||||
type = kind,
|
||||
label = spec.label,
|
||||
label = label,
|
||||
interactive = interactive or false,
|
||||
rect = { x, 0, 90, 50 },
|
||||
pressed = false,
|
||||
|
||||
@@ -99,9 +99,10 @@ bool readFlag(lua_State* state, int index, const char* key) {
|
||||
return value;
|
||||
}
|
||||
|
||||
ui::Align readAlign(lua_State* state, int index, const char* key) {
|
||||
ui::Align readAlign(lua_State* state, int index, const char* key,
|
||||
ui::Align fallback = ui::START) {
|
||||
lua_getfield(state, index, key);
|
||||
ui::Align align = ui::START;
|
||||
ui::Align align = fallback;
|
||||
if (!lua_isnoneornil(state, -1)) {
|
||||
const char* value = luaL_checkstring(state, -1);
|
||||
if (strcmp(value, "center") == 0) {
|
||||
@@ -199,10 +200,13 @@ int create(lua_State* state) {
|
||||
bool present = false;
|
||||
spec.w = readSize(state, 2, "w", present);
|
||||
spec.h = readSize(state, 2, "h", present);
|
||||
const int16_t pad = readNumber(state, 2, "pad", 0);
|
||||
// A button pads and centres its label by default, which the Lua wrapper used to
|
||||
// do by writing both into the caller's spec and rehashing it.
|
||||
const bool button = type == ui::BUTTON;
|
||||
const int16_t pad = readNumber(state, 2, "pad", button ? 8 : 0);
|
||||
spec.padT = spec.padR = spec.padB = spec.padL = static_cast<uint8_t>(pad);
|
||||
spec.gap = static_cast<uint8_t>(readNumber(state, 2, "gap", 0));
|
||||
spec.align = readAlign(state, 2, "align");
|
||||
spec.align = readAlign(state, 2, "align", button ? ui::CENTER : ui::START);
|
||||
spec.justify = readAlign(state, 2, "justify");
|
||||
|
||||
lua_getfield(state, 2, "at");
|
||||
@@ -230,9 +234,10 @@ int create(lua_State* state) {
|
||||
: static_cast<int32_t>(luaL_checkinteger(state, -1));
|
||||
lua_pop(state, 1);
|
||||
|
||||
lua_getfield(state, 2, "label");
|
||||
// An argument for the same reason type is: a text node's spec is small, so the
|
||||
// label was usually the key that pushed it into a rehash.
|
||||
const char* label =
|
||||
lua_isnoneornil(state, -1) ? nullptr : luaL_checkstring(state, -1);
|
||||
lua_isnoneornil(state, 5) ? nullptr : luaL_checkstring(state, 5);
|
||||
if (label && type == ui::TEXT) {
|
||||
GuiProvider& gui = Runtime::from(state)->gui();
|
||||
// Measured with the style it will be painted in, not the normal one: a bold
|
||||
@@ -249,7 +254,6 @@ int create(lua_State* state) {
|
||||
const uint16_t id = tree(state).add(parent, spec, type, flags);
|
||||
if (label)
|
||||
tree(state).setLabel(id, label);
|
||||
lua_pop(state, 1);
|
||||
|
||||
// Styled here rather than by a second call from Lua: the spec is already on the
|
||||
// stack, and a node naming no colours must cost no Style entry, which is a
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -166,7 +166,7 @@ int main() {
|
||||
"tree.reset()\n"
|
||||
"local root = tree.create(nil, {w = 'fill', h = 'fill', "
|
||||
"pad = 4, gap = 2}, 'box')\n"
|
||||
"local label = tree.create(root, {label = 'Hello'}, 'text')\n"
|
||||
"local label = tree.create(root, {}, 'text', false, 'Hello')\n"
|
||||
"local button = tree.create(root, {h = 40}, 'button', true)\n"
|
||||
"tree.setStyle(root, {background = 0xFFFFFF, fill = 0xFFFFFF, color = 0, "
|
||||
"face = 0xEEEEEE,\n"
|
||||
|
||||
Reference in New Issue
Block a user