From fd715afcc52b27127de4e7a6a89a7782fdef5676 Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 11:46:34 +0700 Subject: Add `code-size` skill and integrate `metrics_compare_base.py` tool - Introduced a `code-size` skill under `.claude/skills` for evaluating TinyUSB code size changes between the base branch and current branch. - Added `metrics_compare_base.py`, automating code size comparison with granular options for examples, boards, and CI-wide runs. - Updated `AGENTS.md` to include quick references and usage guidance for the new feature. --- tools/metrics_compare_base.py | 252 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 252 insertions(+) create mode 100644 tools/metrics_compare_base.py (limited to 'tools') diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py new file mode 100644 index 000000000..a189e3143 --- /dev/null +++ b/tools/metrics_compare_base.py @@ -0,0 +1,252 @@ +#!/usr/bin/env python3 +"""Build base branch (master) and current tree, then compare code size metrics. + +Creates cmake-metrics//{base,build} directories for each board. +With --combined, also writes cmake-metrics/_combined/metrics_compare.md aggregating +all boards into a single comparison. + +Usage: + python tools/metrics_compare_base.py -b raspberry_pi_pico + python tools/metrics_compare_base.py -b raspberry_pi_pico -b raspberry_pi_pico2 + python tools/metrics_compare_base.py -b raspberry_pi_pico -f portable/raspberrypi + python tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc + python tools/metrics_compare_base.py -b raspberry_pi_pico -e device/cdc_msc --bloaty + python tools/metrics_compare_base.py --ci # first board of each arm-gcc family, combined + python tools/metrics_compare_base.py -b pico -b pico2 --combined # aggregate listed boards +""" +import argparse +import glob +import json +import os +import subprocess +import sys + +TINYUSB_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +METRICS_DIR = os.path.join(TINYUSB_ROOT, 'cmake-metrics') + +verbose = False + + +def run(cmd, **kwargs): + if verbose: + print(f' $ {cmd}') + return subprocess.run(cmd, shell=True, capture_output=True, text=True, **kwargs) + + +def ci_first_boards(): + """Return the first board (alphabetical) of each arm-gcc CI family.""" + matrix_py = os.path.join(TINYUSB_ROOT, '.github', 'workflows', 'ci_set_matrix.py') + if not os.path.isfile(matrix_py): + return [] + ret = run(f'{sys.executable} {matrix_py}') + if ret.returncode != 0: + return [] + try: + data = json.loads(ret.stdout) + except json.JSONDecodeError: + return [] + families = data.get('arm-gcc', []) + boards = [] + bsp_root = os.path.join(TINYUSB_ROOT, 'hw', 'bsp') + for family in families: + family_boards = sorted( + d for d in os.listdir(os.path.join(bsp_root, family, 'boards')) + if os.path.isdir(os.path.join(bsp_root, family, 'boards', d)) + ) if os.path.isdir(os.path.join(bsp_root, family, 'boards')) else [] + if family_boards: + boards.append(family_boards[0]) + return boards + + +def build_board(src_dir, build_dir, board, example=None): + """Configure and build examples for a board. Returns True on success.""" + os.makedirs(build_dir, exist_ok=True) + ret = run(f'cmake -B {build_dir} -G Ninja -DBOARD={board} -DCMAKE_BUILD_TYPE=MinSizeRel ' + f'{os.path.join(src_dir, "examples")}') + if ret.returncode != 0: + print(f' Error configuring {board}: {ret.stderr}') + return False + target = f'--target {os.path.basename(example)}' if example else '' + ret = run(f'cmake --build {build_dir} {target}', timeout=600) + if ret.returncode != 0: + print(f' Error building {board}: {ret.stderr}') + return False + return True + + +def generate_metrics(build_dir, out_basename, filter_str, example=None): + """Run metrics.py combine on .map.json files. Returns metrics json path or None.""" + if example: + patterns = glob.glob(f'{build_dir}/{example}/*.map.json') + else: + patterns = glob.glob(f'{build_dir}/**/*.map.json', recursive=True) + if not patterns: + print(f' Error: no .map.json files in {build_dir}' + (f' for {example}' if example else '')) + return None + + metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') + ret = run(f'{sys.executable} {metrics_py} combine -f {filter_str} -j -q ' + f'-o {out_basename} {" ".join(patterns)}') + if ret.returncode != 0: + print(f' Error: {ret.stderr}') + return None + return f'{out_basename}.json' + + +def main(): + global verbose + + parser = argparse.ArgumentParser(description='Compare code size metrics with base branch') + parser.add_argument('-b', '--board', action='append', default=[], + help='Board name (repeatable). Required unless --ci is given.') + parser.add_argument('-f', '--filter', default='tinyusb/src', + help='Path filter for metrics (default: tinyusb/src)') + parser.add_argument('--base-branch', default='master', + help='Base branch to compare against (default: master)') + parser.add_argument('-e', '--example', action='append', default=None, + help='Compare specific example (repeatable, e.g. -e device/cdc_msc -e host/cdc_msc_hid)') + parser.add_argument('--bloaty', action='store_true', + help='Use bloaty for detailed section/symbol diff (requires -e)') + parser.add_argument('--ci', action='store_true', + help='Add the first board of every arm-gcc CI family. Implies --combined.') + parser.add_argument('--combined', action='store_true', + help='Aggregate map.json files across all boards into one comparison ' + '(in cmake-metrics/_combined/), instead of (or in addition to) per-board.') + parser.add_argument('-v', '--verbose', action='store_true', + help='Print build commands') + args = parser.parse_args() + verbose = args.verbose + + if args.bloaty and not args.example: + parser.error('--bloaty requires -e/--example') + + if args.ci: + args.combined = True + ci_boards = ci_first_boards() + if not ci_boards: + parser.error('--ci: failed to derive boards from .github/workflows/ci_set_matrix.py') + # Append, dedup, preserve order + seen = set(args.board) + for b in ci_boards: + if b not in seen: + args.board.append(b) + seen.add(b) + + if not args.board: + parser.error('at least one -b BOARD is required (or pass --ci)') + + metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') + linkermap_dir = os.path.join(TINYUSB_ROOT, 'tools', 'linkermap') + worktree_dir = os.path.join(METRICS_DIR, '_worktree') + + # Step 1: Create worktree for base branch + print(f'[1/5] Setting up {args.base_branch} worktree...') + if os.path.isdir(worktree_dir): + run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + ret = run(f'git -C {TINYUSB_ROOT} worktree add {worktree_dir} {args.base_branch}') + if ret.returncode != 0: + print(f'Error creating worktree: {ret.stderr}') + sys.exit(1) + + # Ensure linkermap is available + wt_linkermap = os.path.join(worktree_dir, 'tools', 'linkermap') + if not os.path.exists(wt_linkermap) and os.path.exists(linkermap_dir): + os.symlink(linkermap_dir, wt_linkermap) + + try: + examples = args.example or [None] + # For --combined: track every (base_build, cur_build) pair so we can aggregate at the end. + built_pairs = [] + + for board in args.board: + print(f'\n=== {board} ===') + board_dir = os.path.join(METRICS_DIR, board) + base_build = os.path.join(board_dir, 'base') + cur_build = os.path.join(board_dir, 'build') + + # Step 2: Build base (all examples, cmake will skip already-built) + print(f'[2/5] Building {args.base_branch} for {board}...') + if not build_board(worktree_dir, base_build, board): + continue + + # Step 3: Build current + print(f'[3/5] Building current for {board}...') + if not build_board(TINYUSB_ROOT, cur_build, board): + continue + + built_pairs.append((board, base_build, cur_build)) + base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter + + for example in examples: + suffix = f'_{example.replace("/", "_")}' if example else '' + label = f' ({example})' if example else '' + + # Step 4: Generate metrics + print(f'[4/5] Generating metrics for {board}{label}...') + base_json = generate_metrics(base_build, os.path.join(board_dir, f'base_metrics{suffix}'), + base_filter, example) + cur_json = generate_metrics(cur_build, os.path.join(board_dir, f'build_metrics{suffix}'), + args.filter, example) + if not base_json or not cur_json: + continue + + # Step 5: Compare + out_base = os.path.join(board_dir, f'metrics_compare{suffix}') + print(f'[5/5] Comparing {board}{label}...') + ret = run(f'{sys.executable} {metrics_py} compare -m -o {out_base} {base_json} {cur_json}') + print(ret.stdout) + + # Optional: bloaty diff + if args.bloaty and example: + elf_name = os.path.basename(example) + base_elf = os.path.join(base_build, example, f'{elf_name}.elf') + cur_elf = os.path.join(cur_build, example, f'{elf_name}.elf') + if os.path.exists(base_elf) and os.path.exists(cur_elf): + src_filter = f'--source-filter={args.filter}' if args.filter else '' + print(f'--- bloaty sections ---') + ret = run(f'bloaty --domain=vm -d compileunits,sections {src_filter} {cur_elf} -- {base_elf}') + print(ret.stdout) + print(f'--- bloaty symbols ---') + ret = run(f'bloaty --domain=vm -d compileunits,symbols -s vm {src_filter} {cur_elf} -- {base_elf}') + print(ret.stdout) + else: + print(f' bloaty: ELF not found') + + # Optional combined comparison across all boards + if args.combined and built_pairs: + combined_dir = os.path.join(METRICS_DIR, '_combined') + os.makedirs(combined_dir, exist_ok=True) + base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter + base_maps = [] + cur_maps = [] + for _board, base_build, cur_build in built_pairs: + base_maps += glob.glob(f'{base_build}/**/*.map.json', recursive=True) + cur_maps += glob.glob(f'{cur_build}/**/*.map.json', recursive=True) + if not base_maps or not cur_maps: + print(' combined: no map.json files collected, skipping') + else: + print(f'\n=== combined ({len(args.board)} boards) ===') + base_out = os.path.join(combined_dir, 'base_metrics') + cur_out = os.path.join(combined_dir, 'build_metrics') + ret = run(f'{sys.executable} {metrics_py} combine -f {base_filter} -j -q ' + f'-o {base_out} {" ".join(base_maps)}') + if ret.returncode != 0: + print(f' combined base error: {ret.stderr}') + else: + ret = run(f'{sys.executable} {metrics_py} combine -f {args.filter} -j -q ' + f'-o {cur_out} {" ".join(cur_maps)}') + if ret.returncode != 0: + print(f' combined current error: {ret.stderr}') + else: + out_combined = os.path.join(combined_dir, 'metrics_compare') + ret = run(f'{sys.executable} {metrics_py} compare -m ' + f'-o {out_combined} {base_out}.json {cur_out}.json') + print(ret.stdout) + print(f' combined report: {out_combined}.md') + finally: + print(f'\nCleaning up worktree...') + run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + + +if __name__ == '__main__': + main() -- cgit v1.3.1 From f5d6c6ba91e7176ddf5965608c361ccf5d515bde Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 11:56:37 +0700 Subject: Improve remote execution in `hil_ci.sh` --- .claude/skills/code-size/SKILL.md | 2 +- AGENTS.md | 4 +- test/hil/hil_ci.sh | 21 ++++- tools/metrics_compare_base.py | 192 +++++++++++++++++++++++++++----------- 4 files changed, 158 insertions(+), 61 deletions(-) (limited to 'tools') diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md index f10380374..e12a30d86 100644 --- a/.claude/skills/code-size/SKILL.md +++ b/.claude/skills/code-size/SKILL.md @@ -64,7 +64,7 @@ python3 tools/metrics_compare_base.py -b raspberry_pi_pico --base-branch v0.18.0 - Single example, single board: ~30 s - All examples, single board: ~60-90 s -- `--ci` (all arm-gcc families, first board each): 4-8 minutes (parallel build) +- `--ci` (all arm-gcc families, first board each): 4-8 minutes — sequential sweep across boards (Ninja parallelizes within each board, not across) Use timeouts ≥ 10 minutes (600000 ms) for `--ci`. diff --git a/AGENTS.md b/AGENTS.md index eefe9dde1..5c9908d19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,10 +103,10 @@ openocd -f interface/jlink.cfg -f target/stm32h7x.cfg openocd -f interface/cmsis-dap.cfg -f target/rp2040.cfg -c "adapter speed 5000" ``` -**Terminal 2 — connect GDB** (JLink :2331, OpenOCD :3333): +**Terminal 2 — connect GDB** (replace `` with `2331` for JLinkGDBServer or `3333` for OpenOCD): ```bash arm-none-eabi-gdb /tmp/build/firmware.elf -(gdb) target remote :2331 +(gdb) target remote : (gdb) monitor reset halt (gdb) load (gdb) break main # optional, to stop at entry diff --git a/test/hil/hil_ci.sh b/test/hil/hil_ci.sh index d1b5f7def..96872e2e1 100644 --- a/test/hil/hil_ci.sh +++ b/test/hil/hil_ci.sh @@ -26,6 +26,7 @@ ARGS=() while [[ $# -gt 0 ]]; do case "$1" in -b) + [[ $# -ge 2 ]] || { echo "error: -b requires a BOARD argument" >&2; exit 1; } BOARD="$2" ARGS+=("$1" "$2") shift 2 @@ -37,9 +38,14 @@ while [[ $# -gt 0 ]]; do esac done -# Setup remote directory +# Setup remote directory. Use `bash -s` + heredoc so REMOTE_DIR (user-overridable) +# is passed as a positional parameter and never reinterpreted by the remote shell. echo "==> Setting up remote $REMOTE:$REMOTE_DIR" -ssh "$REMOTE" "rm -rf $REMOTE_DIR && mkdir -p $REMOTE_DIR/test/hil $REMOTE_DIR/examples" +ssh "$REMOTE" bash -s -- "$REMOTE_DIR" <<'REMOTE' +set -e +rm -rf -- "$1" +mkdir -p -- "$1/test/hil" "$1/examples" +REMOTE # Copy HIL test script and config echo "==> Copying test scripts" @@ -60,7 +66,7 @@ if [ -n "$BOARD" ]; then BUILD_DIR="$ROOT_DIR/examples/cmake-build-$BOARD" if [ ! -d "$BUILD_DIR" ]; then echo "Error: build directory not found: $BUILD_DIR" - echo "Build first with: cd examples && cmake -DBOARD=$BOARD -G Ninja -B cmake-build-$BOARD .. && cmake --build cmake-build-$BOARD" + echo "Build first with: cd examples && cmake -DBOARD=$BOARD -G Ninja -B cmake-build-$BOARD . && cmake --build cmake-build-$BOARD" exit 1 fi echo "==> Copying binaries for $BOARD" @@ -72,7 +78,12 @@ else done fi -# Run test +# Run test. Use `bash -s` so REMOTE_DIR + ARGS reach the remote shell as positional +# parameters; quoting and metacharacters in args are preserved. CONFIG_BASENAME="$(basename "$CONFIG")" echo "==> Running HIL test on $REMOTE" -ssh -t "$REMOTE" "cd $REMOTE_DIR && python3 -u test/hil/hil_test.py -B examples ${ARGS[*]} test/hil/$CONFIG_BASENAME" +ssh "$REMOTE" bash -s -- "$REMOTE_DIR" "${ARGS[@]}" "test/hil/$CONFIG_BASENAME" <<'REMOTE' +cd -- "$1" +shift +exec python3 -u test/hil/hil_test.py -B examples "$@" +REMOTE diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index a189e3143..0fb767bb7 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -18,19 +18,57 @@ import argparse import glob import json import os +import re +import shlex import subprocess import sys TINYUSB_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) METRICS_DIR = os.path.join(TINYUSB_ROOT, 'cmake-metrics') +def tinyusb_src_filter(checkout_dir): + """Return a path-substring filter that uniquely matches TinyUSB stack source files + in `checkout_dir`. The substring is the absolute path to the checkout's `src/` + dir — collision-free with vendored deps (pico-sdk, lwip, FreeRTOS, etc.) which + live at unrelated paths.""" + return os.path.realpath(os.path.join(checkout_dir, 'src')) + os.sep + verbose = False def run(cmd, **kwargs): + """Run a command. cmd must be a list (no shell=True).""" + if not isinstance(cmd, list): + raise TypeError('run() requires a list, got str — fix the caller') if verbose: - print(f' $ {cmd}') - return subprocess.run(cmd, shell=True, capture_output=True, text=True, **kwargs) + print(f' $ {" ".join(shlex.quote(str(c)) for c in cmd)}') + return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + + +def symlink_deps(main_root, worktree_dir): + """Symlink dependency directories (fetched by tools/get_deps.py) from the main + checkout into the temporary worktree. Without this, the base build fails because + the worktree doesn't have the untracked deps.""" + def link_subdirs(rel_parent): + src_parent = os.path.join(main_root, rel_parent) + dst_parent = os.path.join(worktree_dir, rel_parent) + if not os.path.isdir(src_parent): + return + os.makedirs(dst_parent, exist_ok=True) + for entry in os.listdir(src_parent): + src = os.path.join(src_parent, entry) + dst = os.path.join(dst_parent, entry) + if os.path.isdir(src) and not os.path.exists(dst): + os.symlink(src, dst) + + # lib/* and tools/* deps (e.g. lib/lwip, tools/linkermap) + link_subdirs('lib') + link_subdirs('tools') + # hw/mcu// (e.g. hw/mcu/raspberry_pi/Pico-PIO-USB) + hw_mcu = os.path.join(main_root, 'hw', 'mcu') + if os.path.isdir(hw_mcu): + for vendor in os.listdir(hw_mcu): + link_subdirs(os.path.join('hw', 'mcu', vendor)) def ci_first_boards(): @@ -38,7 +76,7 @@ def ci_first_boards(): matrix_py = os.path.join(TINYUSB_ROOT, '.github', 'workflows', 'ci_set_matrix.py') if not os.path.isfile(matrix_py): return [] - ret = run(f'{sys.executable} {matrix_py}') + ret = run([sys.executable, matrix_py]) if ret.returncode != 0: return [] try: @@ -59,23 +97,34 @@ def ci_first_boards(): def build_board(src_dir, build_dir, board, example=None): - """Configure and build examples for a board. Returns True on success.""" + """Configure and build examples for a board. Returns True on success. + + When `example` is given, only that target is built (`cmake --build --target NAME`), + keeping single-example workflows fast. + """ os.makedirs(build_dir, exist_ok=True) - ret = run(f'cmake -B {build_dir} -G Ninja -DBOARD={board} -DCMAKE_BUILD_TYPE=MinSizeRel ' - f'{os.path.join(src_dir, "examples")}') + ret = run(['cmake', '-B', build_dir, '-G', 'Ninja', + f'-DBOARD={board}', '-DCMAKE_BUILD_TYPE=MinSizeRel', + os.path.join(src_dir, 'examples')]) if ret.returncode != 0: print(f' Error configuring {board}: {ret.stderr}') return False - target = f'--target {os.path.basename(example)}' if example else '' - ret = run(f'cmake --build {build_dir} {target}', timeout=600) + cmd = ['cmake', '--build', build_dir] + if example: + cmd += ['--target', os.path.basename(example)] + ret = run(cmd, timeout=600) if ret.returncode != 0: print(f' Error building {board}: {ret.stderr}') return False return True -def generate_metrics(build_dir, out_basename, filter_str, example=None): - """Run metrics.py combine on .map.json files. Returns metrics json path or None.""" +def generate_metrics(build_dir, out_basename, filters, example=None): + """Run metrics.py combine on .map.json files. Returns metrics json path or None. + + `filters` is a list of substrings; metrics.py keeps a compile unit if its path + contains any of them. + """ if example: patterns = glob.glob(f'{build_dir}/{example}/*.map.json') else: @@ -85,8 +134,11 @@ def generate_metrics(build_dir, out_basename, filter_str, example=None): return None metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') - ret = run(f'{sys.executable} {metrics_py} combine -f {filter_str} -j -q ' - f'-o {out_basename} {" ".join(patterns)}') + cmd = [sys.executable, metrics_py, 'combine'] + for f in filters: + cmd += ['-f', f] + cmd += ['-j', '-q', '-o', out_basename, *patterns] + ret = run(cmd) if ret.returncode != 0: print(f' Error: {ret.stderr}') return None @@ -99,8 +151,12 @@ def main(): parser = argparse.ArgumentParser(description='Compare code size metrics with base branch') parser.add_argument('-b', '--board', action='append', default=[], help='Board name (repeatable). Required unless --ci is given.') - parser.add_argument('-f', '--filter', default='tinyusb/src', - help='Path filter for metrics (default: tinyusb/src)') + parser.add_argument('-f', '--filter', action='append', default=None, + help='Path-substring filter (repeatable). When given, ' + 'overrides the default and is applied to BOTH base and ' + 'current builds. Default: each side\'s own absolute ' + '/src/ path, which uniquely matches TinyUSB ' + 'stack code without colliding with vendored deps.') parser.add_argument('--base-branch', default='master', help='Base branch to compare against (default: master)') parser.add_argument('-e', '--example', action='append', default=None, @@ -136,22 +192,28 @@ def main(): parser.error('at least one -b BOARD is required (or pass --ci)') metrics_py = os.path.join(TINYUSB_ROOT, 'tools', 'metrics.py') - linkermap_dir = os.path.join(TINYUSB_ROOT, 'tools', 'linkermap') worktree_dir = os.path.join(METRICS_DIR, '_worktree') + # Per-side filters: when no override is given, each build uses its own + # absolute /src/ path so we only match TinyUSB stack code from that + # checkout (and never vendored-dep `src/` like pico-sdk/src/...). + if args.filter: + base_filters = cur_filters = list(args.filter) + else: + base_filters = [tinyusb_src_filter(worktree_dir)] + cur_filters = [tinyusb_src_filter(TINYUSB_ROOT)] + # Step 1: Create worktree for base branch print(f'[1/5] Setting up {args.base_branch} worktree...') if os.path.isdir(worktree_dir): - run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') - ret = run(f'git -C {TINYUSB_ROOT} worktree add {worktree_dir} {args.base_branch}') + run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) + ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', worktree_dir, args.base_branch]) if ret.returncode != 0: print(f'Error creating worktree: {ret.stderr}') sys.exit(1) - # Ensure linkermap is available - wt_linkermap = os.path.join(worktree_dir, 'tools', 'linkermap') - if not os.path.exists(wt_linkermap) and os.path.exists(linkermap_dir): - os.symlink(linkermap_dir, wt_linkermap) + # Symlink dependency dirs (lib/*, hw/mcu/*/*, tools/*) so the worktree builds. + symlink_deps(TINYUSB_ROOT, worktree_dir) try: examples = args.example or [None] @@ -164,18 +226,23 @@ def main(): base_build = os.path.join(board_dir, 'base') cur_build = os.path.join(board_dir, 'build') - # Step 2: Build base (all examples, cmake will skip already-built) - print(f'[2/5] Building {args.base_branch} for {board}...') - if not build_board(worktree_dir, base_build, board): - continue - - # Step 3: Build current - print(f'[3/5] Building current for {board}...') - if not build_board(TINYUSB_ROOT, cur_build, board): + # Build only the requested examples (or all if -e not given). Single-example + # mode used to build everything and filter at metric time — that was wasted work. + board_failed = False + for example in examples: + build_label = f' --target {os.path.basename(example)}' if example else '' + print(f'[2/5] Building {args.base_branch} for {board}{build_label}...') + if not build_board(worktree_dir, base_build, board, example): + board_failed = True + break + print(f'[3/5] Building current for {board}{build_label}...') + if not build_board(TINYUSB_ROOT, cur_build, board, example): + board_failed = True + break + if board_failed: continue built_pairs.append((board, base_build, cur_build)) - base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter for example in examples: suffix = f'_{example.replace("/", "_")}' if example else '' @@ -184,16 +251,16 @@ def main(): # Step 4: Generate metrics print(f'[4/5] Generating metrics for {board}{label}...') base_json = generate_metrics(base_build, os.path.join(board_dir, f'base_metrics{suffix}'), - base_filter, example) + base_filters, example) cur_json = generate_metrics(cur_build, os.path.join(board_dir, f'build_metrics{suffix}'), - args.filter, example) + cur_filters, example) if not base_json or not cur_json: continue # Step 5: Compare out_base = os.path.join(board_dir, f'metrics_compare{suffix}') print(f'[5/5] Comparing {board}{label}...') - ret = run(f'{sys.executable} {metrics_py} compare -m -o {out_base} {base_json} {cur_json}') + ret = run([sys.executable, metrics_py, 'compare', '-m', '-o', out_base, base_json, cur_json]) print(ret.stdout) # Optional: bloaty diff @@ -202,50 +269,69 @@ def main(): base_elf = os.path.join(base_build, example, f'{elf_name}.elf') cur_elf = os.path.join(cur_build, example, f'{elf_name}.elf') if os.path.exists(base_elf) and os.path.exists(cur_elf): - src_filter = f'--source-filter={args.filter}' if args.filter else '' + # Bloaty expects one regex; OR-join all filters (current side + # for the new ELF, base side for the base ELF). + bloaty_regex = '(' + '|'.join( + re.escape(f) for f in (cur_filters + base_filters) + ) + ')' + bloaty_common = ['bloaty', '--domain=vm', f'--source-filter={bloaty_regex}'] print(f'--- bloaty sections ---') - ret = run(f'bloaty --domain=vm -d compileunits,sections {src_filter} {cur_elf} -- {base_elf}') + ret = run(bloaty_common + ['-d', 'compileunits,sections', cur_elf, '--', base_elf]) print(ret.stdout) print(f'--- bloaty symbols ---') - ret = run(f'bloaty --domain=vm -d compileunits,symbols -s vm {src_filter} {cur_elf} -- {base_elf}') + ret = run(bloaty_common + ['-d', 'compileunits,symbols', '-s', 'vm', + cur_elf, '--', base_elf]) print(ret.stdout) else: print(f' bloaty: ELF not found') - # Optional combined comparison across all boards + # Optional combined comparison across all boards. + # Aggregates the per-board metrics JSONs (not raw map.json globs) so the argv + # stays small even with --ci spanning many boards. if args.combined and built_pairs: combined_dir = os.path.join(METRICS_DIR, '_combined') os.makedirs(combined_dir, exist_ok=True) - base_filter = args.filter.replace('tinyusb/', '', 1) if args.filter.startswith('tinyusb/') else args.filter - base_maps = [] - cur_maps = [] - for _board, base_build, cur_build in built_pairs: - base_maps += glob.glob(f'{base_build}/**/*.map.json', recursive=True) - cur_maps += glob.glob(f'{cur_build}/**/*.map.json', recursive=True) - if not base_maps or not cur_maps: - print(' combined: no map.json files collected, skipping') + + # Use the no-suffix per-board JSONs (whole-board metrics). Combined mode + # is meant for board-level sweeps; -e/--example combinations skip combined. + base_jsons, cur_jsons = [], [] + for board, _, _ in built_pairs: + bj = os.path.join(METRICS_DIR, board, 'base_metrics.json') + cj = os.path.join(METRICS_DIR, board, 'build_metrics.json') + if os.path.isfile(bj) and os.path.isfile(cj): + base_jsons.append(bj) + cur_jsons.append(cj) + + if not base_jsons or not cur_jsons: + print(' combined: no per-board metrics found (did you pass -e? skip --combined with -e)') else: - print(f'\n=== combined ({len(args.board)} boards) ===') + print(f'\n=== combined ({len(base_jsons)} boards) ===') base_out = os.path.join(combined_dir, 'base_metrics') cur_out = os.path.join(combined_dir, 'build_metrics') - ret = run(f'{sys.executable} {metrics_py} combine -f {base_filter} -j -q ' - f'-o {base_out} {" ".join(base_maps)}') + + # Per-board JSONs are already filtered to TinyUSB-only files; combine + # without re-filtering so we don't accidentally drop entries. + def _combine(out_basename, inputs): + cmd = [sys.executable, metrics_py, 'combine', + '-j', '-q', '-o', out_basename, *inputs] + return run(cmd) + + ret = _combine(base_out, base_jsons) if ret.returncode != 0: print(f' combined base error: {ret.stderr}') else: - ret = run(f'{sys.executable} {metrics_py} combine -f {args.filter} -j -q ' - f'-o {cur_out} {" ".join(cur_maps)}') + ret = _combine(cur_out, cur_jsons) if ret.returncode != 0: print(f' combined current error: {ret.stderr}') else: out_combined = os.path.join(combined_dir, 'metrics_compare') - ret = run(f'{sys.executable} {metrics_py} compare -m ' - f'-o {out_combined} {base_out}.json {cur_out}.json') + ret = run([sys.executable, metrics_py, 'compare', '-m', + '-o', out_combined, f'{base_out}.json', f'{cur_out}.json']) print(ret.stdout) print(f' combined report: {out_combined}.md') finally: print(f'\nCleaning up worktree...') - run(f'git -C {TINYUSB_ROOT} worktree remove --force {worktree_dir}') + run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) if __name__ == '__main__': -- cgit v1.3.1 From 6ba8aeff1603ae54e0fcf2309b0f19e335a16cdc Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 12:45:55 +0700 Subject: metrics_compare_base: catch TimeoutExpired; fix code-size skill docs - run() now catches subprocess.TimeoutExpired (only triggered by `cmake --build`'s timeout=600) and returns CompletedProcess(rc=124) so the caller falls through to error reporting and worktree cleanup instead of crashing with a traceback. - code-size SKILL.md: document the actual default filter (per-side absolute /src/ path, not the old `tinyusb/src` substring) and adjust the reporting guidance to match what the report rows actually contain. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/code-size/SKILL.md | 4 ++-- tools/metrics_compare_base.py | 12 ++++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/.claude/skills/code-size/SKILL.md b/.claude/skills/code-size/SKILL.md index e12a30d86..f3c51ccfa 100644 --- a/.claude/skills/code-size/SKILL.md +++ b/.claude/skills/code-size/SKILL.md @@ -30,7 +30,7 @@ Infer from the user's request: - **Example:** named example → `-e /` (e.g. `-e device/cdc_msc`). "All examples" → omit `-e`. - **Bloaty:** only with `-e`. Use when the user wants a section/symbol-level breakdown for a single binary. - **Base ref:** default `master`. Override with `--base-branch ` (tag or commit also works). -- **Filter:** default `tinyusb/src` (only counts TinyUSB stack code, not example/BSP). Change only if asked. +- **Filter:** default is the absolute path of each side's `/src/` directory, which uniquely identifies TinyUSB stack code without matching vendored deps that also have a `src/` (e.g. `pico-sdk/src/`). Override with one or more `-f SUBSTRING` flags to use repo-relative substrings instead. Change only if asked. ## Common invocations @@ -72,5 +72,5 @@ Use timeouts ≥ 10 minutes (600000 ms) for `--ci`. After running: - Show the markdown report's summary table to the user. -- Highlight any rows with non-zero diff in `tinyusb/src` paths — those are the actual stack-size deltas. +- Highlight any rows with non-zero `% diff` — under the default filter every row is a TinyUSB stack source file (e.g. `usbd.c`, `cdc_device.c`, `dcd_.c`), so any non-zero delta is a real stack-size impact. - If the diff is unexpected, follow up with a single-example `--bloaty` run to localize. diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index 0fb767bb7..a541dae79 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -37,12 +37,20 @@ verbose = False def run(cmd, **kwargs): - """Run a command. cmd must be a list (no shell=True).""" + """Run a command. cmd must be a list (no shell=True). On `timeout=`-induced + TimeoutExpired, return a CompletedProcess with rc=124 instead of letting the + exception propagate, so the caller can fall through to error reporting and + worktree cleanup rather than crashing with a traceback.""" if not isinstance(cmd, list): raise TypeError('run() requires a list, got str — fix the caller') if verbose: print(f' $ {" ".join(shlex.quote(str(c)) for c in cmd)}') - return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + try: + return subprocess.run(cmd, capture_output=True, text=True, **kwargs) + except subprocess.TimeoutExpired as e: + msg = f'Command timed out after {e.timeout}s: {" ".join(shlex.quote(str(c)) for c in cmd)}' + stderr = (e.stderr or '') + ('\n' if e.stderr else '') + msg + return subprocess.CompletedProcess(cmd, 124, stdout=(e.stdout or ''), stderr=stderr) def symlink_deps(main_root, worktree_dir): -- cgit v1.3.1 From 17572a960a53e27ffa07d7d7fda3486bfcc95a2d Mon Sep 17 00:00:00 2001 From: hathach Date: Wed, 29 Apr 2026 12:59:40 +0700 Subject: metrics_compare_base: use git worktree add --detach `git worktree add ` fails if is already checked out elsewhere (main repo, another worktree). --detach checks out the ref at a detached HEAD instead of claiming the branch, making the script work regardless of what is currently checked out. Co-Authored-By: Claude Opus 4.7 (1M context) --- tools/metrics_compare_base.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/metrics_compare_base.py b/tools/metrics_compare_base.py index a541dae79..799a96800 100644 --- a/tools/metrics_compare_base.py +++ b/tools/metrics_compare_base.py @@ -215,7 +215,11 @@ def main(): print(f'[1/5] Setting up {args.base_branch} worktree...') if os.path.isdir(worktree_dir): run(['git', '-C', TINYUSB_ROOT, 'worktree', 'remove', '--force', worktree_dir]) - ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', worktree_dir, args.base_branch]) + # --detach: check out the ref at a detached HEAD instead of trying to claim the + # branch. Lets us add a worktree of `master` even if master is already checked + # out elsewhere (main repo, another worktree). + ret = run(['git', '-C', TINYUSB_ROOT, 'worktree', 'add', '--detach', + worktree_dir, args.base_branch]) if ret.returncode != 0: print(f'Error creating worktree: {ret.stderr}') sys.exit(1) -- cgit v1.3.1