[DRAFT][CI-VERIFY] Bloaty size reports on arm jobs - #1
Draft
rascani wants to merge 10 commits into
Draft
Conversation
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
force-pushed
the
test/bloaty-ci-verify
branch
from
May 29, 2026 20:53
1498925 to
04229ee
Compare
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.