[JensOgorek]

Motivation

A rule that reads a property looked the property up by its name in the module's map every time it was evaluated, and core.output did the same for each of its fields in every loop. A name longer than 15 characters, which std::string cannot hold without allocating, also cost a heap allocation per lookup. On a bench ESP32, 500 property reads in rules took 5 ms more per loop with short names and 19 ms more with a 16-character name than with this PR, that is 10 and 38 µs per read. A field of core.output with a 16-character name took 25 µs longer; with short names the difference is below the 1 ms resolution of the measurement. Long names are common: six of the 44 fields of F30's core line have one, like wheels.linear_speed_from_ticks, and so do most mirrors of a BusTelemetry (#365), such as p2t.delta_2.angle_left, which arm interlocks read in every loop.

Implementation

• A property expression is now a VariableExpression that keeps the variable it resolves when the code is compiled, plus the module and property name for telemetry layouts.
• A module only ever adds variables to its property map: in its constructor, or later under a new name (Proxy for unknown or declared properties, SerialBus for clock offsets, BusTelemetry mirrors in Mirror values of serial bus peers in BusTelemetry modules #365). Declaring an existing name assigns to its variable. So the variable of a property never changes; telemetry frames and the enabled property already rely on this.
• A field of core.output keeps its variable from the first loop that finds it. Until then it is looked up in every loop as before, so the output may still name a variable or property that appears later.
• Module::get_property takes the name by reference, searches the map once instead of twice and returns a reference to the variable, like get_enabled_property.
• A property of the wrong type still reports property is not a boolean, a variable variable is not a boolean, as before.

Cost: 8 B of heap per property in compiled code and per output field; 1 792 B less flash.

Progress

• The implementation is complete.
• Documentation has been updated (or is not necessary).
• Tested on hardware (or is not necessary).

Bench test 1: property reads in rules

One ESP32 board, built with ESP-IDF 5.3.1, main ec5d268 against this PR baab50c. After each reset, the console received:

i1 = Input(34)
i2 = Input(35)
i3 = Input(36)
i4 = Input(39)
int loops = 0
int hit = 0
bool on = true
when on then loops = loops + 1 end
str s = ""
str t = "0123456789…"
when on then s = t; s = t; … end

t has 1900 characters, and 10 rules with 20 assignments s = t each copy it 200 times per loop. That is a code path this PR does not touch, and it keeps the loop just under its 10 ms period, so that any further work stretches the loop. Then followed 50 rules with 10 property reads each, all of them evaluated and false:

when core.millis < 0 or core.heap < 0 or i1.level < 0 or i2.level < 0 or i3.level < 0 or i4.level < 0 or i1.change < 0 or i2.change < 0 or i3.change < 0 or i4.change < 0 then hit = hit + 1 end

or the same with ten times core.last_message_age < 0, a name of 16 characters. The loop time is 20 s of core.millis divided by the loops counted meanwhile. A loop that overruns waits for the next 1 ms tick, so the loop time has a resolution of 1 ms, 2 µs per read; 10 ms means the work fits into the period. Each run was done twice with the same result.

Rules besides the counter	main ec5d268	this PR baab50c
200 string assignments	10 ms	10 ms
200 string assignments + 500 reads of short names	19 ms	14 ms
200 string assignments + 500 reads of core.last_message_age	34 ms	15 ms
500 reads of core.last_message_age, no string assignments	22 ms	10 ms

With this PR the heap holds 8 B more per read (32 104 instead of 36 048 B free with the 500 short reads).

The rework eac45c9, which makes the property expression a variable expression, gave the same 10, 15 and 10 ms for the string assignments alone, with the 500 reads of core.last_message_age added, and for those reads alone.

With the review fixes 03d82c6 the three runs gave 10, 15 and 10 ms again, and when core.millis then … reports property is not a boolean in every loop, when x then … with an int x reports variable is not a boolean.

Bench test 2: fields of core.output

Same board and the same 200 string assignments, with the console at 921600 baud (core.set_baudrate(921600)), so that the line does not wait for the UART. core.output named 200 fields cycling through i1.level, i2.level, … i4.change, or 80 times core.last_message_age (100 would exceed the 2 KB console line). The runs with this PR were done twice with the same result.

Output	main ec5d268	this PR baab50c
none	10 ms	10 ms
200 fields with short names	22.0 ms	22.0 ms
80 fields core.last_message_age	21.4 ms	19.4 ms

With short names the time goes into formatting the values; the lookup is below the resolution.

core.output("core.millis later") before int later = 7 behaves the same on both builds: error in module "core": unknown variable "later" until the declaration, then the value, 7 and after later = 8 8.

※

[falkoschindler]

@JensOgorek Thanks, the core of this holds up: I traced every write to a module's properties map and to Global::variables, and nothing replaces a variable under an existing name, #365 included (it uses emplace), so neither a compiled property expression nor a cached core.output field can go stale. The review turned up four small things, all fixed in 03d82c6 on the branch; the PR description has the two bullets that changed.

Fixed in 03d82c6

• The invariant was stated only at its users. The "never replaced" rule stood in comments at PropertyExpression and output_element_t, not at the properties map itself in module.h. A comment above the map now names the rule and the four places that rely on it (compiled property expressions, core.output fields, telemetry frames, enabled_property). Enforcing it with a private map and an add_property that throws would touch about 30 module constructors and is left out.
• A property of the wrong type said "variable". The five evaluate_* methods of VariableExpression reported "variable is not a boolean" for a property too, e.g. for when core.millis then ..., where the condition type is not checked at compile time and the error is echoed every loop. A protected virtual kind() gives "variable" or "property", so the messages read as before the rework.
• The cast order in to_field is load-bearing. Since a PropertyExpression is a VariableExpression now, the generic branch in telemetry.cpp would match a property too and then fail to find it in Global::variables. A comment says the PropertyExpression cast must stay first.
• get_property returned the shared pointer by value. It now returns const Variable_ptr & like get_enabled_property; the callers that need a copy (PropertyExpression, the core.output cache, wheels.cpp) still take it once.

※

[JensOgorek]

Retest of 03d82c6 passed — merged as 006e04c

@falkoschindler thanks for the four fixes. On the bench ESP32 a property of the wrong type reports property is not a boolean again and a variable variable is not a boolean, bench test 1 gave the same 10, 15 and 10 ms, and the flash saving is now 1 792 B instead of 1 840 B; both are in the description.

※