Skip to content

fix(size-report): count linker-folded symbol addresses once - #10994

Closed
proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10571-report-size-icf
Closed

proggeramlug wants to merge 2 commits into
PerryTS:mainfrom
proggeramlug:fix/10571-report-size-icf

Conversation

@proggeramlug

Copy link
Copy Markdown
Contributor

Fixes #10571.

--report-size previously charged every symbol name at a folded address as another copy of the emitted bytes. That inflated crate and generic-family totals and made duplicate-function-body suggest savings the linker had already achieved.

This change keeps one deterministic representative per section/address before building any totals or duplicate-body groups. When aliases disagree about size, it prefers the largest real symbol size. The report now describes generic-family counts as distinct linked instantiations.

Validation:

  • cargo test -p perry --bin perry folded_symbol_aliases_are_charged_once on an isolated Linux checkout at perrymaster: 1 passed.
  • rustfmt --check --edition 2021 crates/perry/src/commands/compile/size_report.rs
  • ./scripts/check_file_size.sh
  • python3 scripts/check_test_registration.py
  • git diff --check

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 15 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 55ec56db-43b3-4bea-88d0-71972453eeae

📥 Commits

Reviewing files that changed from the base of the PR and between c7cbc3c and c3bd9f1.

📒 Files selected for processing (2)
  • changelog.d/10994-report-size-icf-aliases.md
  • crates/perry/src/commands/compile/size_report.rs
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main in merge train 257 (#11039, v0.5.1640), main 990b3eeada.

Carried at head c3bd9f1eb1. CI on the train head passed every job except the known public-baseline lint step: all 6 gap shards, cargo-test, e2e-scoped, gc-stress, check, warnings and security-audit green.

This train was split by blast radius after an earlier 35-PR assembly hit five gap regressions: it carries only PRs touching no lowering path. Trains rebase-merge, so commits get new SHAs and GitHub cannot mark this merged. Closed as landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--report-size over-counts by 2-3%: it sums symbol sizes without grouping by address, so ICF-folded aliases are charged once per name

1 participant