Skip to content

Commit

Permalink
perf: Don't track specific live points for promoteds
Browse files Browse the repository at this point in the history
We don't query this information out of the promoted (it's basically a
single "unit" regardless of the complexity within it) and this saves on
re-initializing the SparseIntervalMatrix's backing IndexVec with mostly
empty rows for all of the leading regions in the function. Typical
promoteds will only contain a few regions that need up be uplifted,
while the parent function can have thousands.

For a simple function repeating println!("Hello world"); 50,000 times
this reduces compile times from 90 to 15 seconds in debug mode. The
previous implementations re-initialization led to an overall roughly n^2
runtime as each promoted initialized slots for ~n regions, now we scale
closer to linearly (5000 hello worlds takes 1.1 seconds).
  • Loading branch information
Mark-Simulacrum committed Jan 15, 2024
1 parent 665d2c6 commit 5d68fa6
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 16 deletions.
68 changes: 55 additions & 13 deletions compiler/rustc_borrowck/src/region_infer/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use rustc_index::Idx;
use rustc_index::IndexVec;
use rustc_middle::mir::{BasicBlock, Body, Location};
use rustc_middle::ty::{self, RegionVid};
use std::collections::BTreeSet;
use std::fmt::Debug;
use std::rc::Rc;

Expand Down Expand Up @@ -125,8 +126,13 @@ pub(crate) struct LivenessValues {
/// The map from locations to points.
elements: Rc<RegionValueElements>,

live_regions: BTreeSet<RegionVid>,

/// For each region: the points where it is live.
points: SparseIntervalMatrix<RegionVid, PointIndex>,
///
/// This is not initialized for promoteds, because we don't care *where* within a promoted a
/// region is live, only that it is.
points: Option<SparseIntervalMatrix<RegionVid, PointIndex>>,

/// When using `-Zpolonius=next`, for each point: the loans flowing into the live regions at
/// that point.
Expand Down Expand Up @@ -155,24 +161,38 @@ impl LiveLoans {

impl LivenessValues {
/// Create an empty map of regions to locations where they're live.
pub(crate) fn new(elements: Rc<RegionValueElements>) -> Self {
pub(crate) fn with_specific_points(elements: Rc<RegionValueElements>) -> Self {
LivenessValues {
points: SparseIntervalMatrix::new(elements.num_points),
live_regions: BTreeSet::new(),
points: Some(SparseIntervalMatrix::new(elements.num_points)),
elements,
loans: None,
}
}

/// Create an empty map of regions to locations where they're live.
///
/// Unlike `with_specific_points`, does not track exact locations where something is live, only
/// which regions are live.
pub(crate) fn without_specific_points(elements: Rc<RegionValueElements>) -> Self {
LivenessValues { live_regions: BTreeSet::new(), points: None, elements, loans: None }
}

/// Iterate through each region that has a value in this set.
pub(crate) fn regions(&self) -> impl Iterator<Item = RegionVid> {
self.points.rows()
pub(crate) fn regions(&self) -> impl Iterator<Item = RegionVid> + '_ {
self.live_regions.iter().copied()
}

/// Records `region` as being live at the given `location`.
pub(crate) fn add_location(&mut self, region: RegionVid, location: Location) {
debug!("LivenessValues::add_location(region={:?}, location={:?})", region, location);
let point = self.elements.point_from_location(location);
self.points.insert(region, point);
debug!("LivenessValues::add_location(region={:?}, location={:?})", region, location);
if self.elements.point_in_range(point) {
self.live_regions.insert(region);
}
if let Some(points) = &mut self.points {
points.insert(region, point);
}

// When available, record the loans flowing into this region as live at the given point.
if let Some(loans) = self.loans.as_mut() {
Expand All @@ -185,7 +205,12 @@ impl LivenessValues {
/// Records `region` as being live at all the given `points`.
pub(crate) fn add_points(&mut self, region: RegionVid, points: &IntervalSet<PointIndex>) {
debug!("LivenessValues::add_points(region={:?}, points={:?})", region, points);
self.points.union_row(region, points);
if points.iter().any(|point| self.elements.point_in_range(point)) {
self.live_regions.insert(region);
}
if let Some(this) = &mut self.points {
this.union_row(region, points);
}

// When available, record the loans flowing into this region as live at the given points.
if let Some(loans) = self.loans.as_mut() {
Expand All @@ -201,23 +226,37 @@ impl LivenessValues {

/// Records `region` as being live at all the control-flow points.
pub(crate) fn add_all_points(&mut self, region: RegionVid) {
self.points.insert_all_into_row(region);
if let Some(points) = &mut self.points {
points.insert_all_into_row(region);
}
self.live_regions.insert(region);
}

/// Returns whether `region` is marked live at the given `location`.
pub(crate) fn is_live_at(&self, region: RegionVid, location: Location) -> bool {
let point = self.elements.point_from_location(location);
self.points.row(region).is_some_and(|r| r.contains(point))
if let Some(points) = &self.points {
points.row(region).is_some_and(|r| r.contains(point))
} else {
unreachable!(
"Should be using LivenessValues::with_specific_points to ask whether live at a location"
)
}
}

/// Returns whether `region` is marked live at any location.
pub(crate) fn is_live_anywhere(&self, region: RegionVid) -> bool {
self.live_points(region).next().is_some()
self.live_regions.contains(&region)
}

/// Returns an iterator of all the points where `region` is live.
fn live_points(&self, region: RegionVid) -> impl Iterator<Item = PointIndex> + '_ {
self.points
let Some(points) = &self.points else {
unreachable!(
"Should be using LivenessValues::with_specific_points to ask whether live at a location"
)
};
points
.row(region)
.into_iter()
.flat_map(|set| set.iter())
Expand Down Expand Up @@ -372,7 +411,10 @@ impl<N: Idx> RegionValues<N> {
/// elements for the region `from` from `values` and add them to
/// the region `to` in `self`.
pub(crate) fn merge_liveness(&mut self, to: N, from: RegionVid, values: &LivenessValues) {
if let Some(set) = values.points.row(from) {
let Some(value_points) = &values.points else {
panic!("LivenessValues must track specific points for use in merge_liveness");
};
if let Some(set) = value_points.row(from) {
self.points.union_row(to, set);
}
}
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
let mut constraints = MirTypeckRegionConstraints {
placeholder_indices: PlaceholderIndices::default(),
placeholder_index_to_region: IndexVec::default(),
liveness_constraints: LivenessValues::new(elements.clone()),
liveness_constraints: LivenessValues::with_specific_points(elements.clone()),
outlives_constraints: OutlivesConstraintSet::default(),
member_constraints: MemberConstraintSet::default(),
type_tests: Vec::default(),
Expand Down Expand Up @@ -544,8 +544,13 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
// modify their locations.
let all_facts = &mut None;
let mut constraints = Default::default();
let mut liveness_constraints =
LivenessValues::new(Rc::new(RegionValueElements::new(promoted_body)));
// We care which regions are used in the promoted, but not where in the promoted those
// regions are used. So we skip tracking the specific points - that can be expensive, since
// each promoted has its own LivenessValues but the regions are indexed globally within the
// body. As a result we'd spend lots of time re-initializing the values.
let mut liveness_constraints = LivenessValues::without_specific_points(Rc::new(
RegionValueElements::new(promoted_body),
));
// Don't try to add borrow_region facts for the promoted MIR

let mut swap_constraints = |this: &mut Self| {
Expand Down

0 comments on commit 5d68fa6

Please sign in to comment.