Skip to content

Store LiveLoans more densely packed #161850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rust-bors merged 1 commit into from
Sep 2, 2026
+28 −3
Merged

Store LiveLoans more densely packed #161850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_borrowck/src/nll.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ pub(crate) fn compute_regions<'tcx>(
&lowered_constraints,
);

let num_points = location_map.num_points();

@panstromek panstromek Aug 27, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure where is the right place to get this information from location_map. The way location_map is threaded through the code and the fact that it's in Rc makes it unclear at what point in time it is ready to use.

View changes since the review

@jackh726 jackh726 Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is fine. I was realizing too that currently things are all jumbled up and it might make sense to take a cleanup pass at some point.

I end up touching this in #161938 anyways, so not a big deal.


// If requested for `-Zpolonius=next`, compute loan liveness information.
// This is done prior to `RegionInferenceContext::new`, because we may add
// additional liveness constraints.
Expand All @@ -155,6 +157,7 @@ pub(crate) fn compute_regions<'tcx>(
&universal_region_relations.universal_regions,
body,
borrow_set,
num_points,
);
}

Expand Down
28 changes: 25 additions & 3 deletions compiler/rustc_borrowck/src/polonius/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ mod liveness_constraints;
use std::collections::BTreeMap;

use rustc_data_structures::fx::FxHashSet;
use rustc_index::bit_set::SparseBitMatrix;
use rustc_index::bit_set::DenseBitSet;
use rustc_middle::mir::{Body, Local};
use rustc_middle::ty::RegionVid;
use rustc_mir_dataflow::points::PointIndex;
Expand All @@ -55,7 +55,28 @@ use crate::dataflow::BorrowIndex;
use crate::region_infer::values::LivenessValues;
use crate::universal_regions::UniversalRegions;

pub(crate) type LiveLoans = SparseBitMatrix<PointIndex, BorrowIndex>;
#[derive(Clone)]
pub(crate) struct LiveLoans {

@panstromek panstromek Aug 27, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if it's worth making this a broadly available datastructure or if it's enough to keep it here for now?

View changes since the review

@jackh726 jackh726 Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think leaving this here for now is fine.

@lqd lqd Aug 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's only used in a small part of borrowck, and we will have to tweak it in the future anyways: the current behavior of filling this depends on how the graph is traversed. When traversing multiple loans at a time, the exact iteration and filling behavior will be different, and the best live loans representation will likely need to be reinvestigated. This matches your comment

There's also the tradeoff with iteration, though

num_points: usize,
// This matrix always has more rows (PointIndex) than columns (BorrowIndex),
// and the borrow dimension is usually very low (single digit in 90% of cases in our benchmark suite),
// so we store it packed in a single bitset. Rows are points, columns are borrows.
flat_matrix: DenseBitSet<usize>,
}

impl LiveLoans {
pub(crate) fn new(num_points: usize, num_borrows: usize) -> Self {
Self { num_points, flat_matrix: DenseBitSet::new_empty(num_points * num_borrows) }
}
pub(crate) fn insert(&mut self, row: PointIndex, col: BorrowIndex) {
let bit_index = row.index() + self.num_points * col.index();
self.flat_matrix.insert(bit_index);
}
pub(crate) fn contains(&self, row: PointIndex, col: BorrowIndex) -> bool {
let bit_index = row.index() + self.num_points * col.index();
self.flat_matrix.contains(bit_index)
}
}

/// This struct holds the necessary
/// - liveness data, created during MIR typeck, and which will be used to lazily compute the
Expand Down Expand Up @@ -109,6 +130,7 @@ impl PoloniusContext {
universal_regions: &UniversalRegions<'tcx>,
body: &Body<'tcx>,
borrow_set: &BorrowSet<'tcx>,
num_points: usize,
) {
// We don't need to prepare the graph (index NLL constraints, etc.) if we have no loans to
// trace throughout localized constraints.
Expand All @@ -118,7 +140,7 @@ impl PoloniusContext {
// step in the chain (the NLL loan scope and active loans computations).
let graph = LocalizedConstraintGraph::new(liveness, outlives_constraints);

let mut live_loans = LiveLoans::new(borrow_set.len());
let mut live_loans = LiveLoans::new(num_points, borrow_set.len());
let mut visitor = LoanLivenessVisitor { liveness, live_loans: &mut live_loans };
graph.traverse(
body,
Expand Down
Loading