perf: the console's constants move to flash, -378 B and -214 B of RAM
8382 -> 8004 B of flash, 725 -> 511 B of RAM on a part that has 2048. The command names were a std::to_array of string_view, and on a Harvard machine that is the worst of both: the characters land in .data and so does the table's own pointer-and-length pair for each of them, so the firmware carried 216 B of RAM for thirteen words that never change - and paid for them in flash too, since .data is copied out of an initialiser image at startup. They are one NUL-separated blob in flash now, walked with lpm. Separators rather than an offset table, because an offset table is the RAM this exists to give back; the names sit in it in match order, so the walk that finds a name is the same walk that compares it and measures it. Flash falls further than RAM does: the initialiser image and the two tables were 216 B of it, and the blob is 87. The header said the names "cannot" be in flash because they are matched at run time. Being matched at run time is not a reason to be in RAM on a machine with two address spaces - only being *written* is, and nothing writes these. Two smaller things came with it. `reset`'s exact-match rule was a bool on every entry to protect one; it is an index found by searching the list, so reordering the commands cannot move the protection onto a different one. And `version` was the last string_view left, holding its own characters and a pointer to them. The matching is now pinned rather than assumed: lookup() is constexpr and the battery asserts the load-bearing order the README documents - `s` is show and not statistics, `st` is statistics, no abbreviation of `reset` resolves, and `helpful` is not `help`. Red-checked by claiming `s` is statistics. Both modes byte-identical, ten tests green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -66,7 +66,7 @@ cmake --preset atmega328p-generated
|
|||||||
cmake --build --preset atmega328p-generated
|
cmake --build --preset atmega328p-generated
|
||||||
```
|
```
|
||||||
|
|
||||||
The firmware is **8382 B** of flash, byte-identical between the generated and
|
The firmware is **8004 B** of flash, byte-identical between the generated and
|
||||||
reflect modes, and `ctest` holds it to that number.
|
reflect modes, and `ctest` holds it to that number.
|
||||||
|
|
||||||
## Atmel Studio
|
## Atmel Studio
|
||||||
|
|||||||
224
src/terminal.hpp
224
src/terminal.hpp
@@ -18,6 +18,33 @@
|
|||||||
// until Ctrl+C, and Ctrl+C abandons a half-typed line anywhere else.
|
// until Ctrl+C, and Ctrl+C abandons a half-typed line anywhere else.
|
||||||
namespace app {
|
namespace app {
|
||||||
|
|
||||||
|
// Commands, in the order they are matched - which is the order the original
|
||||||
|
// firmware matched them in, and that order is load-bearing. An abbreviation
|
||||||
|
// resolves to the *first* entry it prefixes, so `s` is show (not statistics,
|
||||||
|
// not set) exactly as it always was, and anything appended to this list cannot
|
||||||
|
// steal an abbreviation that already meant something else.
|
||||||
|
//
|
||||||
|
// A function rather than a variable, so the list exists only while the flash
|
||||||
|
// blob below is being built and never as storage of its own.
|
||||||
|
consteval auto command_names()
|
||||||
|
{
|
||||||
|
return std::to_array<std::string_view>({
|
||||||
|
"help",
|
||||||
|
"show",
|
||||||
|
"curve",
|
||||||
|
"monitor",
|
||||||
|
"bootloader",
|
||||||
|
"uptime",
|
||||||
|
"statistics",
|
||||||
|
"histogram",
|
||||||
|
"reset",
|
||||||
|
"set",
|
||||||
|
"auto",
|
||||||
|
"version",
|
||||||
|
"save",
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
class terminal {
|
class terminal {
|
||||||
static constexpr char ctrl_c = 0x03;
|
static constexpr char ctrl_c = 0x03;
|
||||||
static constexpr char backspace = 0x08;
|
static constexpr char backspace = 0x08;
|
||||||
@@ -28,31 +55,48 @@ class terminal {
|
|||||||
static inline bool m_monitoring = false;
|
static inline bool m_monitoring = false;
|
||||||
static inline std::uint64_t m_last_monitor = 0;
|
static inline std::uint64_t m_last_monitor = 0;
|
||||||
|
|
||||||
// Commands, in the order they are matched - which is the order the original
|
static constexpr std::uint8_t command_count = command_names().size();
|
||||||
// firmware matched them in, and that order is load-bearing. An abbreviation
|
|
||||||
// resolves to the *first* entry it prefixes, so `s` is show (not statistics,
|
|
||||||
// not set) exactly as it always was, and anything appended to this list
|
|
||||||
// cannot steal an abbreviation that already meant something else.
|
|
||||||
struct command {
|
|
||||||
std::string_view name;
|
|
||||||
bool exact; // reset only: an abbreviation must not be able to wipe data
|
|
||||||
};
|
|
||||||
|
|
||||||
static constexpr auto commands = std::to_array<command>({
|
static constexpr std::size_t name_bytes = [] {
|
||||||
{"help", false},
|
std::size_t total = 0;
|
||||||
{"show", false},
|
for (auto one : command_names()) {
|
||||||
{"curve", false},
|
total += one.size() + 1;
|
||||||
{"monitor", false},
|
}
|
||||||
{"bootloader", false},
|
return total;
|
||||||
{"uptime", false},
|
}();
|
||||||
{"statistics", false},
|
|
||||||
{"histogram", false},
|
// The names as one NUL-separated blob in flash. Matched at run time, which
|
||||||
{"reset", true},
|
// is not a reason to sit in RAM on a machine with two address spaces: a
|
||||||
{"set", false},
|
// table of `string_view` puts the characters *and* its own pointers there,
|
||||||
{"auto", false},
|
// and on this part that was 216 B of 2 KB spent on data that never changes.
|
||||||
{"version", false},
|
// Separators rather than an offset table, because an offset table is the
|
||||||
{"save", false},
|
// RAM this exists to give back.
|
||||||
});
|
static constexpr auto names_data = [] {
|
||||||
|
std::array<char, name_bytes> chars{};
|
||||||
|
std::size_t at = 0;
|
||||||
|
for (auto one : command_names()) {
|
||||||
|
for (char c : one) {
|
||||||
|
chars[at++] = c;
|
||||||
|
}
|
||||||
|
chars[at++] = '\0';
|
||||||
|
}
|
||||||
|
return chars;
|
||||||
|
}();
|
||||||
|
|
||||||
|
static constexpr avr::flash_table<names_data> names{};
|
||||||
|
|
||||||
|
// `reset` must be typed in full - an abbreviation must not be able to wipe
|
||||||
|
// data. Found in the list rather than written as an index, so reordering the
|
||||||
|
// commands cannot move the protection onto a different one.
|
||||||
|
static constexpr std::uint8_t exact_command = [] {
|
||||||
|
const auto list = command_names();
|
||||||
|
for (std::uint8_t i = 0; i < list.size(); ++i) {
|
||||||
|
if (list[i] == "reset") {
|
||||||
|
return i;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return static_cast<std::uint8_t>(list.size());
|
||||||
|
}();
|
||||||
|
|
||||||
// Column the descriptions' colons line up in, counted from the start of the
|
// Column the descriptions' colons line up in, counted from the start of the
|
||||||
// name. The longest name is `bootloader` at 10, so 12 leaves it a space and
|
// name. The longest name is `bootloader` at 10, so 12 leaves it a space and
|
||||||
@@ -66,20 +110,8 @@ class terminal {
|
|||||||
|
|
||||||
// `name ....: ` - the dotted label the original used everywhere it printed a
|
// `name ....: ` - the dotted label the original used everywhere it printed a
|
||||||
// list of things, which is what makes a column of values readable without
|
// list of things, which is what makes a column of values readable without
|
||||||
// counting spaces. One renderer for all three users; only the column differs.
|
// counting spaces. Its width comes from the type, so the padding needs no
|
||||||
static void label(std::string_view text, std::uint8_t column)
|
// hand-counted constant.
|
||||||
{
|
|
||||||
serial << text << ' ';
|
|
||||||
for (auto i = text.size() + 1; i < column; ++i) {
|
|
||||||
serial << '.';
|
|
||||||
}
|
|
||||||
serial << ": "_P;
|
|
||||||
}
|
|
||||||
|
|
||||||
// The same, for a label that is a literal rather than a command name: it
|
|
||||||
// stays in flash, and its width comes from the type, so the padding needs no
|
|
||||||
// hand-counted constant. The command names cannot use this - they are
|
|
||||||
// string_views because they are matched at run time.
|
|
||||||
template <typename Flash>
|
template <typename Flash>
|
||||||
static void label(Flash text, std::uint8_t column)
|
static void label(Flash text, std::uint8_t column)
|
||||||
{
|
{
|
||||||
@@ -90,9 +122,21 @@ class terminal {
|
|||||||
serial << ": "_P;
|
serial << ": "_P;
|
||||||
}
|
}
|
||||||
|
|
||||||
static void help_row(std::string_view name)
|
// The same shape for a command name, whose width is not a type but the walk
|
||||||
|
// to its separator - so it is counted as it is printed rather than measured
|
||||||
|
// first, which would read the blob twice.
|
||||||
|
static void help_row(std::uint8_t which)
|
||||||
{
|
{
|
||||||
label(name, help_column);
|
auto at = name_start(which);
|
||||||
|
std::uint8_t width = 0;
|
||||||
|
for (char c; (c = names.at(at + width)) != '\0'; ++width) {
|
||||||
|
serial << c;
|
||||||
|
}
|
||||||
|
serial << ' ';
|
||||||
|
for (std::uint8_t i = width + 1; i < help_column; ++i) {
|
||||||
|
serial << '.';
|
||||||
|
}
|
||||||
|
serial << ": "_P;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Quarter- C as a signed decimal with a two-digit fraction. The sign is taken
|
// Quarter- C as a signed decimal with a two-digit fraction. The sign is taken
|
||||||
@@ -110,31 +154,31 @@ class terminal {
|
|||||||
static void help()
|
static void help()
|
||||||
{
|
{
|
||||||
serial << "\r\nFanTemp "_P << version << " command overview\r\n"_P;
|
serial << "\r\nFanTemp "_P << version << " command overview\r\n"_P;
|
||||||
help_row(commands[0].name);
|
help_row(0);
|
||||||
serial << "prints this help message\r\n"_P;
|
serial << "prints this help message\r\n"_P;
|
||||||
help_row(commands[1].name);
|
help_row(1);
|
||||||
serial << "shows current temperature and fan speed\r\n"_P;
|
serial << "shows current temperature and fan speed\r\n"_P;
|
||||||
help_row(commands[2].name);
|
help_row(2);
|
||||||
serial << "shows mapping from temperature to fan speed\r\n"_P;
|
serial << "shows mapping from temperature to fan speed\r\n"_P;
|
||||||
help_row(commands[3].name);
|
help_row(3);
|
||||||
serial << "loops the show command until Ctrl+C is pressed\r\n"_P;
|
serial << "loops the show command until Ctrl+C is pressed\r\n"_P;
|
||||||
help_row(commands[4].name);
|
help_row(4);
|
||||||
serial << "enters the bootloader\r\n"_P;
|
serial << "enters the bootloader\r\n"_P;
|
||||||
help_row(commands[5].name);
|
help_row(5);
|
||||||
serial << "shows system uptime\r\n"_P;
|
serial << "shows system uptime\r\n"_P;
|
||||||
help_row(commands[6].name);
|
help_row(6);
|
||||||
serial << "prints overall statistics like min and max temp\r\n"_P;
|
serial << "prints overall statistics like min and max temp\r\n"_P;
|
||||||
help_row(commands[7].name);
|
help_row(7);
|
||||||
serial << "prints a histogram of the temperature\r\n"_P;
|
serial << "prints a histogram of the temperature\r\n"_P;
|
||||||
help_row(commands[8].name);
|
help_row(8);
|
||||||
serial << "resets statistics to 0 in EEPROM and RAM (no abbreviation)\r\n"_P;
|
serial << "resets statistics to 0 in EEPROM and RAM (no abbreviation)\r\n"_P;
|
||||||
help_row(commands[9].name);
|
help_row(9);
|
||||||
serial << "sets the fan speed to the provided value, 0-100\r\n"_P;
|
serial << "sets the fan speed to the provided value, 0-100\r\n"_P;
|
||||||
help_row(commands[10].name);
|
help_row(10);
|
||||||
serial << "turns on automatic fan control\r\n"_P;
|
serial << "turns on automatic fan control\r\n"_P;
|
||||||
help_row(commands[11].name);
|
help_row(11);
|
||||||
serial << "displays firmware version\r\n"_P;
|
serial << "displays firmware version\r\n"_P;
|
||||||
help_row(commands[12].name);
|
help_row(12);
|
||||||
serial << "writes the statistics to EEPROM now\r\n"_P;
|
serial << "writes the statistics to EEPROM now\r\n"_P;
|
||||||
serial << "commands may be abbreviated: 'up' is uptime\r\n"_P;
|
serial << "commands may be abbreviated: 'up' is uptime\r\n"_P;
|
||||||
}
|
}
|
||||||
@@ -259,25 +303,58 @@ class terminal {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Abbreviations: the input matches a command when it is a non-empty prefix
|
// Where the given command's name begins in the blob. A walk over the
|
||||||
// of it. `reset` is the exception and must be typed in full.
|
// separators, for the two callers that need to start at one name.
|
||||||
//
|
static constexpr std::uint8_t name_start(std::uint8_t which)
|
||||||
// starts_with, not substr: substr throws std::out_of_range, and one
|
|
||||||
// potentially-throwing call is enough to pull in std::terminate, which does
|
|
||||||
// not exist in a freestanding AVR build. The link fails rather than the
|
|
||||||
// firmware, so this is a build-time trap rather than a runtime one - but it
|
|
||||||
// is a trap, and the whole file avoids substr for that reason.
|
|
||||||
static bool matches(std::string_view input, const command &c)
|
|
||||||
{
|
{
|
||||||
if (input.empty()) {
|
std::uint8_t at = 0;
|
||||||
return false;
|
for (std::uint8_t i = 0; i < which; ++i) {
|
||||||
|
while (names.at(at) != '\0') {
|
||||||
|
++at;
|
||||||
}
|
}
|
||||||
if (c.exact) {
|
++at;
|
||||||
return input == c.name;
|
|
||||||
}
|
}
|
||||||
return c.name.starts_with(input);
|
return at;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public:
|
||||||
|
// Which command a word names, or `command_count` for none. The input
|
||||||
|
// matches when it is a non-empty prefix of a name; `reset` is the exception
|
||||||
|
// and must be typed in full.
|
||||||
|
//
|
||||||
|
// One pass over the blob answers both, because the names sit in it in match
|
||||||
|
// order: each name is compared as it is walked, and the walk to its
|
||||||
|
// separator is also what measures it.
|
||||||
|
//
|
||||||
|
// Public and `constexpr` so the battery can pin it: the match order is
|
||||||
|
// load-bearing, an abbreviation must never reach `reset`, and reading the
|
||||||
|
// names from flash must not have changed either.
|
||||||
|
static constexpr std::uint8_t lookup(std::string_view input)
|
||||||
|
{
|
||||||
|
if (input.empty()) {
|
||||||
|
return command_count;
|
||||||
|
}
|
||||||
|
std::uint8_t at = 0;
|
||||||
|
for (std::uint8_t i = 0; i < command_count; ++i) {
|
||||||
|
std::uint8_t length = 0;
|
||||||
|
bool prefix = true;
|
||||||
|
for (char c; (c = names.at(at + length)) != '\0'; ++length) {
|
||||||
|
if (length < input.size() && input[length] != c) {
|
||||||
|
prefix = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
const bool whole = input.size() == length;
|
||||||
|
if (prefix && input.size() <= length && (i != exact_command || whole)) {
|
||||||
|
return i;
|
||||||
|
}
|
||||||
|
at += length + 1;
|
||||||
|
}
|
||||||
|
return command_count;
|
||||||
|
}
|
||||||
|
|
||||||
|
static constexpr std::uint8_t no_command = command_count;
|
||||||
|
|
||||||
|
private:
|
||||||
static void dispatch(std::string_view input)
|
static void dispatch(std::string_view input)
|
||||||
{
|
{
|
||||||
// A line that overflowed the buffer is not a command - it is the tail of
|
// A line that overflowed the buffer is not a command - it is the tail of
|
||||||
@@ -299,13 +376,7 @@ class terminal {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::uint8_t which = commands.size();
|
const auto which = lookup(word);
|
||||||
for (std::uint8_t i = 0; i < commands.size(); ++i) {
|
|
||||||
if (matches(word, commands[i])) {
|
|
||||||
which = i;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
switch (which) {
|
switch (which) {
|
||||||
case 0:
|
case 0:
|
||||||
@@ -377,7 +448,10 @@ class terminal {
|
|||||||
}
|
}
|
||||||
|
|
||||||
public:
|
public:
|
||||||
static constexpr std::string_view version = "v2.2";
|
// A flash string, like every other literal the console prints: as a
|
||||||
|
// `string_view` it was the last constant left in RAM, holding its own
|
||||||
|
// characters and a pointer to them.
|
||||||
|
static constexpr auto version = "v2.2"_P;
|
||||||
static constexpr std::uint8_t bar_max = 100;
|
static constexpr std::uint8_t bar_max = 100;
|
||||||
|
|
||||||
// Columns the dotted labels' colons land in, and the curve's span. All four
|
// Columns the dotted labels' colons land in, and the curve's span. All four
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ libavr_format_test()
|
|||||||
|
|
||||||
# The image must stay below the boot section at 0x7e00, and the README states
|
# The image must stay below the boot section at 0x7e00, and the README states
|
||||||
# what it measures; a library advance that moves it says nothing on its own.
|
# what it measures; a library advance that moves it says nothing on its own.
|
||||||
libavr_size_claim_test(fantemp 8382)
|
libavr_size_claim_test(fantemp 8004)
|
||||||
|
|
||||||
# The README says the two modes emit the same image, and only a tree with
|
# The README says the two modes emit the same image, and only a tree with
|
||||||
# both built can say whether they do.
|
# both built can say whether they do.
|
||||||
|
|||||||
@@ -6,6 +6,7 @@
|
|||||||
#include <cstdint>
|
#include <cstdint>
|
||||||
|
|
||||||
#include "curve.hpp"
|
#include "curve.hpp"
|
||||||
|
#include "terminal.hpp"
|
||||||
#include "thermistor.hpp"
|
#include "thermistor.hpp"
|
||||||
|
|
||||||
namespace {
|
namespace {
|
||||||
@@ -81,4 +82,39 @@ consteval bool thermistor_falls()
|
|||||||
}
|
}
|
||||||
static_assert(thermistor_falls());
|
static_assert(thermistor_falls());
|
||||||
|
|
||||||
|
// The console's one parsing rule, read out of the flash blob the names live
|
||||||
|
// in. Every case here is a property of the *order* the names sit in, so this
|
||||||
|
// is what an edit to that list has to answer to.
|
||||||
|
using app::terminal;
|
||||||
|
|
||||||
|
// Full names, and the ends of the list - a walk that miscounts a separator
|
||||||
|
// lands on a neighbour rather than failing, so both ends are named.
|
||||||
|
static_assert(terminal::lookup("help") == 0);
|
||||||
|
static_assert(terminal::lookup("save") == 12);
|
||||||
|
static_assert(terminal::lookup("bootloader") == 4);
|
||||||
|
|
||||||
|
// Abbreviations resolve to the first entry they prefix. `s` prefixes show,
|
||||||
|
// statistics, set and save, and show is first - which is the original's
|
||||||
|
// behaviour and the reason the list is ordered rather than sorted.
|
||||||
|
static_assert(terminal::lookup("s") == 1);
|
||||||
|
static_assert(terminal::lookup("st") == 6);
|
||||||
|
static_assert(terminal::lookup("se") == 9);
|
||||||
|
static_assert(terminal::lookup("sa") == 12);
|
||||||
|
static_assert(terminal::lookup("up") == 5);
|
||||||
|
static_assert(terminal::lookup("b") == 4);
|
||||||
|
|
||||||
|
// `reset` is the exception: it must be typed in full, so no abbreviation of it
|
||||||
|
// resolves - and none of its prefixes names anything else either.
|
||||||
|
static_assert(terminal::lookup("reset") == 8);
|
||||||
|
static_assert(terminal::lookup("rese") == terminal::no_command);
|
||||||
|
static_assert(terminal::lookup("res") == terminal::no_command);
|
||||||
|
static_assert(terminal::lookup("r") == terminal::no_command);
|
||||||
|
|
||||||
|
// Nothing, and nothing that matches.
|
||||||
|
static_assert(terminal::lookup("") == terminal::no_command);
|
||||||
|
static_assert(terminal::lookup("xyzzy") == terminal::no_command);
|
||||||
|
|
||||||
|
// Longer than the name it prefixes is not a match: `helpful` is not `help`.
|
||||||
|
static_assert(terminal::lookup("helpful") == terminal::no_command);
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|||||||
Reference in New Issue
Block a user