Skip to content

perf(bpa): avoid per-bundle heap allocs in RIB route lookup - #767

Merged
ricktaylor merged 1 commit into
ricktaylor:mainfrom
g4dpz:perf/routing-lookup-allocs
Oct 19, 2026
Merged

ricktaylor merged 1 commit into
ricktaylor:mainfrom
g4dpz:perf/routing-lookup-allocs

Conversation

@g4dpz

@g4dpz g4dpz commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

Summary

The per-bundle RIB route lookup (RouteTable::find_recurse, run on every admitted and re-dispatched bundle) allocated two heap structures on every call:

  • a HashSet cycle-detection trail, and
  • a Vec of candidate next-hop peers.

This replaces both with inline SmallVec types so the common lookup is allocation-free:

  • Trail — SmallVec<[&Eid; 4]> instead of HashSet<&Eid>. Via chains are short, so the trail is used purely as a small set (membership check, push on descent, pop on return) and the inline capacity keeps the common walk allocation-free. Cycle-detection semantics are preserved exactly.
  • Peer vector — SmallVec<[(u32, &Eid); 2]> instead of Vec. The non-ECMP case resolves to zero or one peer and no longer heap-allocates. The rare ECMP branch converts to Vec via into_vec() so LookupResult::ForwardEcmp and select_peer are unchanged, keeping the diff minimal.

Adds smallvec (default features) as a direct hardy-bpa dependency; it was already present in Cargo.lock transitively, so the lock change is a single line. no_std-safe.

No public API or routing-semantics change — this is a pure internal allocation optimisation.

Testing

CI gates run locally, all green (except the storage-tests harness, which requires a live Postgres/S3 and is unrelated to this change):

  • cargo fmt --check — clean
  • cargo clippy --locked --all-targets --all-features -- -D warnings — zero warnings
  • cargo test --locked --all-features --workspace --exclude storage-tests — all pass

The existing routing tests cover the behaviour and pass unchanged: test_recursion_loop (cycle detection), test_reflection, test_reflection_no_double, test_union_route_member_specificity, and the ECMP tests (which exercise the multi-peer path and the into_vec() conversion).

The per-bundle RIB lookup (RouteTable::find_recurse, on every admitted and
re-dispatched bundle) allocated two heap structures on every call: a HashSet
cycle-detection trail and a Vec of candidate next-hop peers.

Replace both with inline SmallVec types:

- Trail: SmallVec<[&Eid; 4]> instead of HashSet<&Eid>. Via chains are short,
  so the trail is used purely as a small set (membership check, push on
  descent, pop on return) and the inline capacity keeps the common walk
  allocation-free. Cycle-detection semantics are preserved exactly.
- PeerVec: SmallVec<[(u32, &Eid); 2]> instead of Vec. The non-ECMP case
  resolves to zero or one peer and no longer heap-allocates; the rare ECMP
  branch converts to Vec via into_vec() to keep LookupResult::ForwardEcmp
  and select_peer unchanged for a minimal diff.

Adds smallvec (default features) as a direct bpa dependency; it was already
in the lock file transitively. no_std-safe. Existing routing tests
(test_recursion_loop, test_reflection*, test_union_route_member_specificity,
ECMP tests) cover the behaviour and pass unchanged.
Copilot AI balanced review requested due to automatic review settings October 18, 2026 19:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The localized changes preserve cycle detection, peer ordering, and ECMP behavior, with no blocking issues identified.

No open issues 🎉

What changed in this PR

Reduces common-path heap allocations in BPA route lookup without changing public APIs or routing behavior.

Changes:

  • Uses inline SmallVec storage for cycle detection and candidate peers.
  • Preserves ECMP results by converting multi-peer collections to Vec.
  • Adds smallvec as a direct BPA dependency.
File Description
Cargo.lock Records BPA’s direct smallvec dependency.
bpa/​src/​routing/​table.rs Replaces temporary collections with inline storage and updates test calls.
bpa/​src/​routing/​rib.rs Uses the new trail type for route lookups.
bpa/​Cargo.toml Declares the smallvec dependency.

Review effort: Balanced

@ricktaylor ricktaylor left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks Dave, we held off because routing was in a bit of flux., but this looks 👍

@ricktaylor
ricktaylor merged commit 1370ef3 into ricktaylor:main Oct 19, 2026
5 checks passed
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.

3 participants