Skip to content

Binary size skill#17988

Merged
lucylq merged 2 commits into
mainfrom
binary-size-skill
Mar 7, 2026
Merged

Binary size skill#17988
lucylq merged 2 commits into
mainfrom
binary-size-skill

Conversation

@lucylq
Copy link
Copy Markdown
Contributor

@lucylq lucylq commented Mar 7, 2026

Summary

Add binary size skill and update ci with new binary size thresholds

Test plan

ci

Copilot AI review requested due to automatic review settings March 7, 2026 01:10
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot Bot commented Mar 7, 2026

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/17988

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 1 Awaiting Approval, 71 Pending

As of commit 997ff16 with merge base 6a1788b (image):

AWAITING APPROVAL - The following workflow needs approval before CI can run:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 7, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 7, 2026

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@lucylq lucylq force-pushed the binary-size-skill branch from 96f140d to 997ff16 Compare March 7, 2026 01:11
@lucylq lucylq requested a review from rascani March 7, 2026 01:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new “binary-size” skill to the .claude skill set and tightens CI binary-size thresholds to reflect current stripped size_test sizes.

Changes:

  • Document a new /binary-size skill and add it to the top-level skills index.
  • Update CI binary-size thresholds for GCC9 and Clang12 size test jobs.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
CLAUDE.md Adds /binary-size to the skills list.
.github/workflows/pull.yml Updates binary size thresholds and records current CI sizes.
.claude/skills/binary-size/SKILL.md New skill doc describing how to measure/analyze binary size and where to look for size wins.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 475 to 477
output=$(ls -la cmake-out/test/size_test)
arr=($output)
size=${arr[4]}
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow parses file size from ls -la output (arr=($output); size=${arr[4]}), which is brittle (format can vary with locale/ACL markers) and can misread the size. Prefer using stat (e.g., bytes via stat -c%s <file>) to make this check robust.

Copilot uses AI. Check for mistakes.
Comment on lines +478 to +479
# Current CI size: 48008 (gcc9-nopytorch, 2026-03-06)
threshold="48500"
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new gcc9 threshold (48,500) is very close to the observed size (48,008). This small buffer risks CI flakiness when toolchain/image deps change slightly; consider leaving a larger margin (or documenting why ~500 bytes is sufficient).

Suggested change
# Current CI size: 48008 (gcc9-nopytorch, 2026-03-06)
threshold="48500"
# Current CI size: 48008 (gcc9-nopytorch, 2026-03-06); leave ~2KB headroom to avoid CI flakiness
threshold="50000"

Copilot uses AI. Check for mistakes.
Comment on lines +42 to +43
- `EXECUTORCH_OPTIMIZE_SIZE=ON` — enables `-Os`, `-fno-exceptions`, `-fno-rtti`, unwind table suppression
- `CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror"`
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This “Key build flags” list is incomplete vs test/build_size_test.sh (it also sets -Wno-int-in-bool-context and -DET_HAVE_PREAD=0 via COMMON_CXXFLAGS). Consider updating this section to reflect the full flags actually used, and clarify which flags come from EXECUTORCH_OPTIMIZE_SIZE vs explicit CXXFLAGS.

Suggested change
- `EXECUTORCH_OPTIMIZE_SIZE=ON` — enables `-Os`, `-fno-exceptions`, `-fno-rtti`, unwind table suppression
- `CXXFLAGS="-fno-exceptions -fno-rtti -Wall -Werror"`
- `EXECUTORCH_OPTIMIZE_SIZE=ON`
- Typically enables: `-Os`, `-fno-exceptions`, `-fno-rtti`, and unwind table suppression
- Explicit compiler flags (`CXXFLAGS`/`COMMON_CXXFLAGS`):
- `-fno-exceptions -fno-rtti -Wall -Werror`
- `-Wno-int-in-bool-context`
- `-DET_HAVE_PREAD=0`

Copilot uses AI. Check for mistakes.
@lucylq lucylq requested a review from larryliu0820 March 7, 2026 01:13
@lucylq lucylq merged commit aa8c50d into main Mar 7, 2026
147 of 150 checks passed
@lucylq lucylq deleted the binary-size-skill branch March 7, 2026 01:33
jpiat pushed a commit to jpiat/executorch that referenced this pull request Mar 17, 2026
### Summary
Add binary size skill and update ci with new binary size thresholds

### Test plan
ci

---------

Co-authored-by: Github Executorch <github_executorch@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants