Uh oh!
There was an error while loading. Please reload this page.
feat(bloat): per-symbol back-references + single-symbol lookup (#478) - #480
Merged
Conversation
Adds two complementary capabilities to the bloat-analysis pipeline so
AI optimisation passes can answer "who calls this specific symbol?"
and "what does this specific symbol cost?" with per-symbol precision.
Schema
- `FineGrainedSymbol.called_by: Vec<String>` — per-symbol inverse of
`references_to`, populated by inverting the existing objdump-derived
forward map (`callgraph::invert`) in `run_objdump_and_attribute`.
Single objdump pass produces both directions.
- `SymbolQuery` + `SymbolLookup` + `FineGrainedSymbolMap::find_symbol`:
resolves one symbol by exact demangled / substring demangled / exact
mangled. Substring falls back from exact-demangled-first → ambiguous
list when multiple match; mangled is always exact.
Graph
- `NodeKind::Caller { demangled, size, callees_count }` — per-symbol
inverse of `Callee`, mirroring the `walk_forward` layout.
- `walk_backward_per_symbol`: BFS on `called_by`, ranked by caller
flash size descending, same fan-out + max-depth + overflow node
rendering as the forward walker.
- `BackrefGraph::build_with_index` prefers per-symbol back-references
when `called_by` is non-empty (more precise), falls back to the
TU-level walker when it's empty (older reports, vtable/fn-ptr-only
call sites). No double-rendering.
Markdown
- New `Top callers (dual ranking)` sub-table sibling to the existing
callees one. `rank_callers_dual` ranks by caller size and by caller
breadth (callees-count, proxy for "downstream leverage if this caller
were eliminated"), surfacing what an AI bloat reducer most wants.
CLI
- `fbuild bloat lookup <input> --symbol <demangled>` (or
`--symbol-mangled`, or `--json`): emits a focused per-symbol block
with size, archive, object, region, per-symbol callers + callees,
and TU-level referencers — without re-rendering the whole report.
Tests
- `find_symbol_dispatches_correctly` covers exact / substring /
ambiguous-substring / unique-substring / miss / mangled.
- `called_by_roundtrips_via_serde_with_default` proves the new field
serialises and that older reports (without it) still parse.
- `backward_uses_per_symbol_when_called_by_populated` and
`backward_falls_back_to_tu_when_called_by_empty` lock in the
per-symbol-vs-TU branch in the graph walker.
- `rank_callers_dual_sorts_each_axis_independently` proves the
markdown sub-table's two rankings independently surface "biggest"
vs. "broadest".
Closes#478
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>📝 WalkthroughWalkthroughThis PR adds per-symbol backward callgraph edges to the fbuild bloat analyzer by inverting objdump-derived forward calls, introduces a symbol lookup API with multiple query modes, replaces callee dual-ranking with caller dual-ranking, integrates backward graph traversal with a new Caller node type, emits "Top callers" tables in markdown reports, and implements a new ChangesPer-Symbol Backward Edges and Symbol Lookup Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/fbuild-build/src/symbol_analyzer/tests.rs (1)
120-121: ⚡ Quick winAdd a dedicated markdown regression test for the callers sub-table path.
All updated fixtures still keep
called_byempty, so this suite doesn’t assert the new#### Top callers (dual ranking)rendering/overflow behavior. Adding one focused non-emptycalled_bycase here would guard the new report contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/fbuild-build/src/symbol_analyzer/tests.rs` around lines 120 - 121, Add a focused regression test that populates the called_by field (instead of leaving Vec::new()) and asserts the new sub-table rendering; update the test in tests.rs to create a non-empty called_by entry (use the same struct used in the surrounding fixtures) and assert the generated markdown contains the "#### Top callers (dual ranking)" header and expected overflow/entry rows so the callers sub-table path is exercised (name the test e.g. callers_subtable_regression to clearly reference it).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fbuild-core/src/symbol_analysis/graph/mod.rs`:
- Around line 369-375: The current logic sets use_per_symbol_backward =
want_backward && !root.called_by.is_empty() and then disables want_tu_backward,
causing TU-level backward results to be skipped whenever root.called_by has
entries; change the control flow so that when both per-symbol callers
(root.called_by) and TU-level referencers exist you run both walkers instead of
choosing one: update the conditions around want_tu_backward /
use_per_symbol_backward and the subsequent emission (where CappedReferencer and
the per-symbol walker are produced) to always include the TU-level
CappedReferencer list alongside the per-symbol results when both datasets are
present, and ensure the rendering/emission code distinguishes the two views
visually (e.g., different labels or sections) so both per-symbol and TU-level
backward callers are shown side-by-side.
In `@crates/fbuild-core/src/symbol_analysis/mod.rs`:
- Around line 238-251: find_symbol currently uses .find(...) for ExactDemangled
and ExactMangled (and the other exact branches), which returns only the first
match; change these branches in the find_symbol function to collect all matching
entries from self.symbols (use .iter().filter(...) and collect or count) and: if
zero matches return SymbolLookup::Miss, if exactly one return SymbolLookup::Hit
with that entry, and if more than one return SymbolLookup::Ambiguous (including
the matching entries or enough info per SymbolLookup::Ambiguous variant). Update
both ExactDemangled and ExactMangled branches (and the similar exact-mode
branches around the 253-265 region) to follow this collect->switch-on-count
pattern so duplicate exact matches are detected and reported as Ambiguous.
---
Nitpick comments:
In `@crates/fbuild-build/src/symbol_analyzer/tests.rs`:
- Around line 120-121: Add a focused regression test that populates the
called_by field (instead of leaving Vec::new()) and asserts the new sub-table
rendering; update the test in tests.rs to create a non-empty called_by entry
(use the same struct used in the surrounding fixtures) and assert the generated
markdown contains the "#### Top callers (dual ranking)" header and expected
overflow/entry rows so the callers sub-table path is exercised (name the test
e.g. callers_subtable_regression to clearly reference it).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2d9983ea-54e3-462a-a4ab-3ba4f2f929e5
📒 Files selected for processing (12)
crates/fbuild-build/src/symbol_analyzer/markdown.rscrates/fbuild-build/src/symbol_analyzer/mod.rscrates/fbuild-build/src/symbol_analyzer/tests.rscrates/fbuild-cli/src/cli/args.rscrates/fbuild-cli/src/cli/bloat_lookup.rscrates/fbuild-cli/src/cli/dispatch.rscrates/fbuild-cli/src/cli/mod.rscrates/fbuild-core/src/symbol_analysis/graph/mod.rscrates/fbuild-core/src/symbol_analysis/graph/tests.rscrates/fbuild-core/src/symbol_analysis/graph/walker.rscrates/fbuild-core/src/symbol_analysis/mod.rscrates/fbuild-core/src/symbol_analysis/tests.rs
Comment on lines
369
to
+375
| let want_backward = matches!( | ||
| config.direction, | ||
| Direction::Backward | Direction::Bidirectional | ||
| ); | ||
| let level1: Vec<CappedReferencer> = if want_backward { | ||
| let use_per_symbol_backward = want_backward && !root.called_by.is_empty(); | ||
| let want_tu_backward = want_backward && !use_per_symbol_backward; | ||
| let level1: Vec<CappedReferencer> = if want_tu_backward { |
There was a problem hiding this comment.
Render both backward views when both datasets exist
Line 374 disables TU-level backward expansion whenever called_by is non-empty, and Line 553 then emits only the per-symbol walker. This drops cref-only callers (indirect/unresolved call paths) instead of showing both views side-by-side, so backward graphs can lose valid call-in evidence. Please run both views when both sources are present and distinguish them visually.
Also applies to: 547-566
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/fbuild-core/src/symbol_analysis/graph/mod.rs` around lines 369 - 375,
The current logic sets use_per_symbol_backward = want_backward &&
!root.called_by.is_empty() and then disables want_tu_backward, causing TU-level
backward results to be skipped whenever root.called_by has entries; change the
control flow so that when both per-symbol callers (root.called_by) and TU-level
referencers exist you run both walkers instead of choosing one: update the
conditions around want_tu_backward / use_per_symbol_backward and the subsequent
emission (where CappedReferencer and the per-symbol walker are produced) to
always include the TU-level CappedReferencer list alongside the per-symbol
results when both datasets are present, and ensure the rendering/emission code
distinguishes the two views visually (e.g., different labels or sections) so
both per-symbol and TU-level backward callers are shown side-by-side.
Comment on lines
+238
to
+251
| pub fn find_symbol(&self, query: &SymbolQuery<'_>) -> SymbolLookup<'_> { | ||
| match query { | ||
| SymbolQuery::ExactDemangled(name) => self | ||
| .symbols | ||
| .iter() | ||
| .find(|s| s.demangled == *name) | ||
| .map(SymbolLookup::Hit) | ||
| .unwrap_or(SymbolLookup::Miss), | ||
| SymbolQuery::ExactMangled(name) => self | ||
| .symbols | ||
| .iter() | ||
| .find(|s| s.mangled == *name) | ||
| .map(SymbolLookup::Hit) | ||
| .unwrap_or(SymbolLookup::Miss), |
There was a problem hiding this comment.
Handle duplicate exact matches in find_symbol.
Line 240 and Line 246 use .find(...), so exact queries return only the first matching row. This can hide valid duplicates (e.g., multiple map-derived rows sharing one owner symbol), yielding partial/unstable lookup results for bloat lookup --symbol. Exact modes should collect all exact matches and return Ambiguous when count > 1 (or apply a documented deterministic merge rule).
Suggested fix
pub fn find_symbol(&self, query: &SymbolQuery<'_>) -> SymbolLookup<'_> {
match query {
- SymbolQuery::ExactDemangled(name) => self- .symbols- .iter()- .find(|s| s.demangled == *name)- .map(SymbolLookup::Hit)- .unwrap_or(SymbolLookup::Miss),- SymbolQuery::ExactMangled(name) => self- .symbols- .iter()- .find(|s| s.mangled == *name)- .map(SymbolLookup::Hit)- .unwrap_or(SymbolLookup::Miss),+ SymbolQuery::ExactDemangled(name) => {+ let mut hits: Vec<&FineGrainedSymbol> =+ self.symbols.iter().filter(|s| s.demangled == *name).collect();+ match hits.len() {+ 0 => SymbolLookup::Miss,+ 1 => SymbolLookup::Hit(hits.remove(0)),+ _ => SymbolLookup::Ambiguous(hits),+ }+ }+ SymbolQuery::ExactMangled(name) => {+ let mut hits: Vec<&FineGrainedSymbol> =+ self.symbols.iter().filter(|s| s.mangled == *name).collect();+ match hits.len() {+ 0 => SymbolLookup::Miss,+ 1 => SymbolLookup::Hit(hits.remove(0)),+ _ => SymbolLookup::Ambiguous(hits),+ }+ }
SymbolQuery::SubstringDemangled(needle) => {Also applies to: 253-265
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/fbuild-core/src/symbol_analysis/mod.rs` around lines 238 - 251,
find_symbol currently uses .find(...) for ExactDemangled and ExactMangled (and
the other exact branches), which returns only the first match; change these
branches in the find_symbol function to collect all matching entries from
self.symbols (use .iter().filter(...) and collect or count) and: if zero matches
return SymbolLookup::Miss, if exactly one return SymbolLookup::Hit with that
entry, and if more than one return SymbolLookup::Ambiguous (including the
matching entries or enough info per SymbolLookup::Ambiguous variant). Update
both ExactDemangled and ExactMangled branches (and the similar exact-mode
branches around the 253-265 region) to follow this collect->switch-on-count
pattern so duplicate exact matches are detected and reported as Ambiguous.
Uh oh!
There was an error while loading. Please reload this page.
zackees added a commit
that referenced
this pull request
Jun 7, 2026
Picks up: - #480 — per-symbol back-references (`called_by`) + `fbuild bloat lookup` - #486 — drop intra-function jump labels (`<funcname+0xNN>`) from the call graph so `references_to` / `called_by` stay honest - #482 — truncate subprocess output fed back to Claude in hooks Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes#478.
Summary
Two complementary capabilities so AI optimisation passes can answer
"who calls this specific symbol?" and "what does this specific
symbol cost?" with per-symbol precision (not TU-level).
FineGrainedSymbol.called_by— per-symbol inverse ofreferences_to, populated by inverting the existing objdump-derivedforward map (
callgraph::invert) inrun_objdump_and_attribute.Single objdump pass produces both directions.
NodeKind::Caller+walk_backward_per_symbol— graph walkermirror of
walk_forward. Whencalled_byis non-empty the graphuses per-symbol back-edges (more precise); falls back to the
TU-level walker when empty (older reports, vtable-only calls).
Top callersmarkdown sub-table — sibling of the existingTop calleestable, dual-ranked by caller flash size and by callerbreadth (callees-count, proxy for downstream-leverage).
fbuild bloat lookup <input> --symbol <demangled>— focusedper-symbol block (size, archive, object, region, per-symbol callers,
per-symbol callees, TU-level referencers).
--symbol-mangledforexact mangled lookup,
--jsonfor machine consumption. Substringsearch falls back from exact-demangled-first → ambiguous list.
FineGrainedSymbolMap::find_symbolwithSymbolQuery/SymbolLookup— programmatic API behind the CLI.Test plan
find_symbol_dispatches_correctly— exact/substring/ambiguous/unique/miss/mangledcalled_by_roundtrips_via_serde_with_default— JSON round-trip + back-compat for older reportsbackward_uses_per_symbol_when_called_by_populated— graph walker takes per-symbol pathbackward_falls_back_to_tu_when_called_by_empty— TU walker still runs as fallbackrank_callers_dual_sorts_each_axis_independently— both rankings worksoldr cargo check --workspace --all-targetssoldr cargo clippy --workspace --all-targets -- -D warningssoldr cargo fmt --all -- --checkRUSTDOCFLAGS=-D warnings soldr cargo doc --workspace --no-depssoldr cargo test -p fbuild-core --lib symbol_analysis::(72 passed)soldr cargo test -p fbuild-build --lib symbol_analyzer(17 passed)🤖 Generated with Claude Code
Summary by CodeRabbit
fbuild bloat lookupsubcommand to search and analyze specific symbols