Skip to content

[DRAFT][CI-VERIFY] Bloaty size reports on arm jobs - #1

Draft
rascani wants to merge 10 commits into
mainfrom
test/bloaty-ci-verify
Draft

[DRAFT][CI-VERIFY] Bloaty size reports on arm jobs#1
rascani wants to merge 10 commits into
mainfrom
test/bloaty-ci-verify

Conversation

@rascani

@rascani rascani commented May 23, 2026

Copy link
Copy Markdown
Owner

Draft PR to verify the new bloaty PR-comment workflow added in
.github/workflows/bloaty-size-comment.yml. The synthetic +190-byte change
to runtime/executor/program.cpp ([bloaty-ci-verify-string]) should
produce a sticky comment showing the regression on both arm-bare_metal
and arm-zephyr-preset.

Do not merge. Will be force-pushed for additional test cases.

rascani added 2 commits May 29, 2026 13:53
The test-arm-cortex-m-size-test job now builds the PR's merge base in
addition to head, runs bloaty against both, and uploads a per-leg
artifact. A new workflow_run-triggered workflow downloads the artifacts
and posts a sticky PR comment with per-segment, per-section, and
per-bucket deltas plus the top-5 symbols by Δ.

Reporting is best-effort and never fails the size job. Existing
threshold gates are unchanged. The custom bloaty data source in
test/bloaty/executorch.bloaty groups demangled symbols into
runtime/extension/backends/kernels/etc buckets so the diff is readable.

Drafted with Claude.
Synthetic +~190 byte change to verify the bloaty PR-comment workflow
reports the regression correctly. Look for the [bloaty-ci-verify-string]
marker — revert this commit before any real PR.
rascani added 8 commits May 29, 2026 15:55
Three fixes from the first CI run on PR pytorch#19888:

1. Base build was failing because the worktree was at /tmp/base-worktree,
   but CMakeLists.txt:420 requires the repo dir to be named exactly
   `executorch`. Worktree is now /tmp/bloaty-base/executorch.

2. bloaty isn't in apt on the executorch-ubuntu-22.04-arm-sdk image.
   Install via conda-forge instead (conda is already in PATH).

3. `set -e` inside a subshell doesn't fire when the subshell itself is
   on the left of `||` (per bash spec). Replaced with explicit
   `|| exit 1` after each critical command in the bloaty subshell, so
   a python crash actually aborts before the artifact-upload mv runs.

Also added workflow_dispatch to bloaty-size-comment.yml so the comment
poster can be invoked manually for verification before the workflow
lands on the default branch.
…oaty

Two more fixes from the second CI run:

1. `git worktree add` fails on the base SHA because actions/checkout's
   shallow fetch only has the head commit. Explicitly `git fetch --depth=1
   origin <base_sha>` before the worktree add.

2. Conda-forge's bloaty links against a newer libstdc++ than the docker
   image ships, so `bloaty --version` fails with CXXABI_1.3.15 not found.
   Prepend the conda env's lib dir to LD_LIBRARY_PATH so bloaty finds
   its own libstdc++ before the system one.
LD_LIBRARY_PATH override didn't help because the main conda env doesn't
ship a libstdc++.so.6, so the linker still picked the system one (too
old for conda-forge bloaty: CXXABI_1.3.15 missing).

Install bloaty into a dedicated conda env alongside libstdcxx-ng, then
invoke via `conda run --no-capture-output -p <env> bloaty`, which sets
LD_LIBRARY_PATH correctly. Pass the resolved command to bloaty_diff.py
via a new BLOATY env var.
First successful run on PR pytorch#19888 revealed two real issues:

1. bloaty defaults to -n 20 even without a flag, so metadata.json only
   contained the top 20 symbols + an aggregated [N Others] row. Diffing
   the resulting capped lists would produce phantom regressions when
   any symbol crossed the cutoff. -n 0 means unlimited.

2. The arm size_test binary's actual contents on bare-metal:
   - ~40% newlib stdio internals (_vfprintf_r, _svfprintf_r, _dtoa_r)
   - ~10% libsupc++ C++ demangler (d_print_*, cplus_demangle_*)
   - ~10% C++ unwind personality + section-level debug entries
   None of these matched our regexes. Added patterns so they bucket
   into libc/stdlib/metadata correctly.
The previous test bloater (ET_LOG format string) didn't survive Release
mode because ET_LOG_ENABLED=0 compiles log strings out entirely.

This version adds a ~265-byte string in .rodata, referenced through a
volatile static pointer to defeat the optimizer, inside a function
(get_execution_plan) that size_test actually links. Verified locally
that the string survives -Os + strip.

Look for [bloaty-ci-verify-string] in the bloaty PR comment.
Render the same markdown the PR-comment poster would produce, and
append it to $GITHUB_STEP_SUMMARY so it's visible on the workflow
run page without downloading the artifact.

Useful before the workflow_run-triggered comment poster is live
(it can't fire until the workflow file is on the default branch)
and as an always-on backup once the comment is the primary surface.
Adds two collapsed <details> blocks to $GITHUB_STEP_SUMMARY:
- Full per-symbol diff (-d sections,executorch,shortsymbols -n 100)
- Full bloaty report for the head binary (same, no diff)

Also writes head_only.txt to the artifact so the new-binary view is
accessible to the workflow_run poster without re-running bloaty.
Adds a small head-only table after the headline showing per-bucket
sizes for runtime/extension/backends/kernels/tokenizers/flatbuffer.
No deltas — just "how big is each source area in the new binary".

Excludes libc/stdlib/metadata/other so the table stays tight.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant