From 24c1dccf006626685a9927992a910c14c448a931 Mon Sep 17 00:00:00 2001 From: Ben Meadors Date: Mon, 6 Jul 2026 14:52:22 -0500 Subject: [PATCH] CI: track RAM (.data+.bss) in size reports and gate on per-env budgets (#10899) * CI: track RAM (.data+.bss) in size reports and gate on per-env budgets The 2.8.0 nRF52840 heap regression (99% heap in field reports) shipped invisibly because CI only tracked flash. On nRF52840 the heap arena is the linker gap after .bss, so every byte of static .data+.bss growth shrinks the usable heap 1:1 - RAM needs the same guardrails flash already has. What's added: - bin/platformio-custom.py emits ram_bytes (.data + .bss from the ELF, via the toolchain size tool) into each .mt.json manifest. Heap/stack placeholder sections are deliberately excluded. - bin/collect_sizes.py records {flash_bytes, ram_bytes} per env; bin/size_report.py grows RAM and RAM-delta columns in the PR size report. Older artifacts without ram_bytes (and legacy int-schema baselines) degrade to "n/a" instead of crashing. - bin/ram_budgets.json: per-env RAM/flash budgets, enforced only for envs listed there. Seeded with rak4631: ram 113,000 (current 110,948 + ~2 KB slack), flash 786,000 (current 765,192 + ~20 KB; the app region is 0x27000..0xEA000 = 798,720 and the image must stay clear of the warm-store ring guard in extra_scripts/nrf52_warm_region.py). - New size-budget-gate CI job runs size_report.py --enforce-budgets and fails the build on violation; the informational firmware-size-report job now also renders budget usage into the PR comment. - src/main.cpp: opt-in boot heap watermark (-DMESHTASTIC_HEAP_WATERMARK_CHECK) logs LOG_ERROR when less than 20% of the heap is free at the end of setup(). Off by default; skipped on platforms without heap accounting. How budgets are raised: deliberately, never automatically. If a change needs more headroom, bump the env's limit in bin/ram_budgets.json in the same PR and justify the increase in the PR description. Verified: python3 bin/test_size_scripts.py (23/23 pass, including ram_bytes parsing, n/a fallback, and over/under/missing-env budget-gate cases). * Address review: fail the budget gate closed, fix RAM section matching - size-budget-gate workflow: drop continue-on-error on the manifest download and the empty-dir fallback, so the job fails when the data it gates on cannot be fetched. - size_report.py --enforce-budgets now fails closed on every missing-data path instead of trivially passing: empty collected sizes, a budgeted env that was not built, or a manifest without the budgeted metric. Report-only mode keeps rendering those as n/a. - load_budgets() rejects zero/negative/non-integer budgets with a clear error (a typo'd budget could previously crash budget_markdown with ZeroDivisionError or silently skip the check); the percentage render keeps a defensive guard for direct callers. - compute_ram_bytes(): count RISC-V small-data sections (.sdata/.sbss) and exclude ESP-IDF .rtc.* sections, which live outside the heap-competing SRAM. - Trim the heap-watermark comment in main.cpp to two lines. bin/test_size_scripts.py: 27/27 - the two fail-open assertions are flipped to fail-closed, with new report-only counterparts plus cases for empty sizes under enforcement and malformed budgets. --- .github/workflows/main_matrix.yml | 30 ++- bin/collect_sizes.py | 19 +- bin/platformio-custom.py | 51 +++- bin/ram_budgets.json | 24 ++ bin/size_report.py | 302 ++++++++++++++++++---- bin/test_size_scripts.py | 413 +++++++++++++++++++++++++++--- src/main.cpp | 18 ++ 7 files changed, 769 insertions(+), 88 deletions(-) create mode 100644 bin/ram_budgets.json diff --git a/.github/workflows/main_matrix.yml b/.github/workflows/main_matrix.yml index c71afe16d..305fffc0f 100644 --- a/.github/workflows/main_matrix.yml +++ b/.github/workflows/main_matrix.yml @@ -337,7 +337,7 @@ jobs: if: github.event_name == 'pull_request' id: report run: | - ARGS="./current-sizes.json" + ARGS="./current-sizes.json --budgets bin/ram_budgets.json" if [ -f ./develop-sizes.json ]; then ARGS="$ARGS --baseline develop:./develop-sizes.json" fi @@ -375,6 +375,34 @@ jobs: ./pr-number.txt retention-days: 5 + # RAM/flash guardrails: fails CI when an env listed in bin/ram_budgets.json + # exceeds its static RAM (.data+.bss) or flash budget. Kept separate from + # firmware-size-report, which is informational and continue-on-error. + size-budget-gate: + permissions: + contents: read + actions: read + runs-on: ubuntu-latest + needs: [build] + steps: + - uses: actions/checkout@v7 + + # No continue-on-error / empty-dir fallback: the gate must fail closed when + # the data it enforces on cannot be fetched (size_report.py additionally + # fails on missing budgeted envs under --enforce-budgets). + - name: Download current manifests + uses: actions/download-artifact@v8 + with: + path: ./manifests/ + pattern: manifest-* + merge-multiple: true + + - name: Collect current firmware sizes + run: python3 bin/collect_sizes.py ./manifests/ ./current-sizes.json + + - name: Enforce RAM/flash budgets + run: python3 bin/size_report.py ./current-sizes.json --budgets bin/ram_budgets.json --enforce-budgets + release-artifacts: permissions: # Needed for 'gh release upload'. contents: write diff --git a/bin/collect_sizes.py b/bin/collect_sizes.py index b9d04c2fc..5b780ad02 100644 --- a/bin/collect_sizes.py +++ b/bin/collect_sizes.py @@ -1,6 +1,15 @@ #!/usr/bin/env python3 -"""Collect firmware binary sizes from manifest (.mt.json) files into a single report.""" +"""Collect firmware binary sizes from manifest (.mt.json) files into a single report. + +Output schema (consumed by bin/size_report.py): + {"": {"flash_bytes": , "ram_bytes": }} + +flash_bytes is the size of the main firmware image (.bin). ram_bytes is the +static RAM footprint (.data + .bss) emitted into the manifest by +bin/platformio-custom.py; it is omitted for manifests that predate it, and +size_report.py renders those as "n/a". +""" import json import os @@ -8,7 +17,7 @@ import sys def collect_sizes(manifest_dir): - """Scan manifest_dir for .mt.json files and return {board: size_bytes} dict.""" + """Scan manifest_dir for .mt.json files and return {board: sizes_dict}.""" sizes = {} for fname in sorted(os.listdir(manifest_dir)): if not fname.endswith(".mt.json"): @@ -34,7 +43,11 @@ def collect_sizes(manifest_dir): bin_size = entry["bytes"] break if bin_size is not None: - sizes[board] = bin_size + entry = {"flash_bytes": bin_size} + ram_bytes = data.get("ram_bytes") + if isinstance(ram_bytes, int) and not isinstance(ram_bytes, bool): + entry["ram_bytes"] = ram_bytes + sizes[board] = entry return sizes diff --git a/bin/platformio-custom.py b/bin/platformio-custom.py index f1946770c..61fa03610 100644 --- a/bin/platformio-custom.py +++ b/bin/platformio-custom.py @@ -45,6 +45,49 @@ def infer_architecture(board_cfg): return "stm32" return None +def compute_ram_bytes(env): + """Static RAM usage (.data + .bss) of the ELF, via the toolchain size tool. + + Deliberately excludes heap/stack placeholder sections (e.g. the nRF52 .heap + section): on nRF52840 the heap arena is the linker gap after .bss, so static + RAM growth shrinks the usable heap 1:1 - which is exactly why we track it. + Returns None when the value cannot be determined; the manifest then simply + omits ram_bytes and downstream size reports show "n/a". + """ + elf = env.File(env.subst("$BUILD_DIR/${PROGNAME}.elf")) + if not elf.exists(): + return None + size_tool = env.subst("$SIZETOOL") or "size" + try: + output = subprocess.check_output( + [size_tool, "-A", elf.get_abspath()], + env=env["ENV"], + universal_newlines=True, + ) + except Exception as exc: + print(f"mtjson: skipping ram_bytes ({size_tool} failed: {exc})") + return None + ram = 0 + found = False + for line in output.splitlines(): + parts = line.split() + if len(parts) < 2: + continue + name = parts[0] + # Main-SRAM static sections: .data/.bss, platform-prefixed variants (e.g. + # ESP32 .dram0.data/.dram0.bss), and RISC-V small-data .sdata/.sbss. + # ESP-IDF .rtc.* sections live outside the heap-competing SRAM; .heap and + # .tdata never match. + if name.startswith(".rtc"): + continue + if name in (".data", ".bss", ".sdata", ".sbss") or name.endswith(".data") or name.endswith(".bss"): + try: + ram += int(parts[1]) + found = True + except ValueError: + continue + return ram if found else None + def manifest_gather(source, target, env): global manifest_ran if manifest_ran: @@ -98,9 +141,9 @@ def manifest_gather(source, target, env): d["part_name"] = partition_map[p] out.append(d) print(d) - manifest_write(out, env) + manifest_write(out, env, compute_ram_bytes(env)) -def manifest_write(files, env): +def manifest_write(files, env, ram_bytes=None): # Defensive: also skip manifest writing if we cannot determine architecture def get_project_option(name): try: @@ -137,6 +180,10 @@ def manifest_write(files, env): "has_mui": False, "has_inkhud": False, } + # Static RAM footprint (.data + .bss); consumed by bin/collect_sizes.py for + # the CI size report and the bin/ram_budgets.json budget gate. + if ram_bytes is not None: + manifest["ram_bytes"] = ram_bytes # Get partition table (generated in esp32_pre.py) if it exists if env.get("custom_mtjson_part"): # custom_mtjson_part is a JSON string, convert it back to a dict diff --git a/bin/ram_budgets.json b/bin/ram_budgets.json new file mode 100644 index 000000000..b48903a0b --- /dev/null +++ b/bin/ram_budgets.json @@ -0,0 +1,24 @@ +{ + "_comment": [ + "Per-environment static-size budgets, enforced by the size-budget-gate CI job", + "via: bin/size_report.py --budgets bin/ram_budgets.json --enforce-budgets", + "Only environments listed here are gated, and only for the metrics they list.", + "", + "ram_bytes = static RAM (.data + .bss from the ELF, emitted into the .mt.json", + "manifest by bin/platformio-custom.py). On nRF52840 the heap arena is the linker", + "gap after .bss, so every byte of static RAM growth shrinks the usable heap 1:1;", + "that is how the 2.8.0 heap regression shipped without CI noticing.", + "", + "flash_bytes = size of the main firmware image (.bin). The rak4631 app region is", + "0x27000..0xEA000 = 798,720 bytes, and the image must also stay clear of the", + "warm-store record-ring guard (extra_scripts/nrf52_warm_region.py).", + "", + "Budgets are raised DELIBERATELY, never automatically: if your change needs more", + "headroom, bump the limit here in the same PR and justify the increase in the PR", + "description." + ], + "rak4631": { + "ram_bytes": 113000, + "flash_bytes": 786000 + } +} diff --git a/bin/size_report.py b/bin/size_report.py index fd31d12af..5107a39e1 100644 --- a/bin/size_report.py +++ b/bin/size_report.py @@ -4,6 +4,7 @@ Usage: size_report.py [--baseline