Skip to content

Commit 3c5f850

Browse files
committed
Auto merge of #49472 - nikomatsakis:nll-optimize-constraint-prop-1, r=pnkfelix
optimize NLL constraint propagation a little Removes a bone-headed hot spot in NLL constant propagation; we were re-allocating the stack vector and hashmap as we repeated the DFS. This change shares those resources across each call. It also modifies the constraint list to be a linked list; arguably I should revert that, though, as this didn't turn out to be a perf hit and perhaps the old code was clearer? (Still, the new style appeals to me.) r? @pnkfelix
2 parents 1c5283b + ca8176d commit 3c5f850

File tree

4 files changed

+136
-48
lines changed

4 files changed

+136
-48
lines changed

src/librustc_mir/borrow_check/nll/region_infer/dfs.rs

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,26 @@ use borrow_check::nll::region_infer::values::{RegionElementIndex, RegionValueEle
1818
use syntax::codemap::Span;
1919
use rustc::mir::{Location, Mir};
2020
use rustc::ty::RegionVid;
21-
use rustc_data_structures::fx::FxHashSet;
21+
use rustc_data_structures::bitvec::BitVector;
22+
use rustc_data_structures::indexed_vec::Idx;
23+
24+
pub(super) struct DfsStorage {
25+
stack: Vec<Location>,
26+
visited: BitVector,
27+
}
2228

2329
impl<'tcx> RegionInferenceContext<'tcx> {
30+
/// Creates dfs storage for use by dfs; this should be shared
31+
/// across as many calls to dfs as possible to amortize allocation
32+
/// costs.
33+
pub(super) fn new_dfs_storage(&self) -> DfsStorage {
34+
let num_elements = self.elements.num_elements();
35+
DfsStorage {
36+
stack: vec![],
37+
visited: BitVector::new(num_elements),
38+
}
39+
}
40+
2441
/// Function used to satisfy or test a `R1: R2 @ P`
2542
/// constraint. The core idea is that it performs a DFS starting
2643
/// from `P`. The precise actions *during* that DFS depend on the
@@ -34,25 +51,29 @@ impl<'tcx> RegionInferenceContext<'tcx> {
3451
/// - `Ok(false)` if the walk was completed with no changes;
3552
/// - `Err(early)` if the walk was existed early by `op`. `earlyelem` is the
3653
/// value that `op` returned.
37-
pub(super) fn dfs<C>(&self, mir: &Mir<'tcx>, mut op: C) -> Result<bool, C::Early>
54+
#[inline(never)] // ensure dfs is identifiable in profiles
55+
pub(super) fn dfs<C>(
56+
&self,
57+
mir: &Mir<'tcx>,
58+
dfs: &mut DfsStorage,
59+
mut op: C,
60+
) -> Result<bool, C::Early>
3861
where
3962
C: DfsOp,
4063
{
4164
let mut changed = false;
4265

43-
let mut stack = vec![];
44-
let mut visited = FxHashSet();
45-
46-
stack.push(op.start_point());
47-
while let Some(p) = stack.pop() {
66+
dfs.visited.clear();
67+
dfs.stack.push(op.start_point());
68+
while let Some(p) = dfs.stack.pop() {
4869
let point_index = self.elements.index(p);
4970

5071
if !op.source_region_contains(point_index) {
5172
debug!(" not in from-region");
5273
continue;
5374
}
5475

55-
if !visited.insert(p) {
76+
if !dfs.visited.insert(point_index.index()) {
5677
debug!(" already visited");
5778
continue;
5879
}
@@ -62,25 +83,27 @@ impl<'tcx> RegionInferenceContext<'tcx> {
6283

6384
let block_data = &mir[p.block];
6485

65-
let start_stack_len = stack.len();
86+
let start_stack_len = dfs.stack.len();
6687

6788
if p.statement_index < block_data.statements.len() {
68-
stack.push(Location {
89+
dfs.stack.push(Location {
6990
statement_index: p.statement_index + 1,
7091
..p
7192
});
7293
} else {
73-
stack.extend(block_data.terminator().successors().iter().map(
74-
|&basic_block| {
75-
Location {
94+
dfs.stack.extend(
95+
block_data
96+
.terminator()
97+
.successors()
98+
.iter()
99+
.map(|&basic_block| Location {
76100
statement_index: 0,
77101
block: basic_block,
78-
}
79-
},
80-
));
102+
}),
103+
);
81104
}
82105

83-
if stack.len() == start_stack_len {
106+
if dfs.stack.len() == start_stack_len {
84107
// If we reach the END point in the graph, then copy
85108
// over any skolemized end points in the `from_region`
86109
// and make sure they are included in the `to_region`.
@@ -229,9 +252,8 @@ impl<'v, 'tcx> DfsOp for TestTargetOutlivesSource<'v, 'tcx> {
229252
// `X: ur_in_source`, OK.
230253
if self.inferred_values
231254
.universal_regions_outlived_by(self.target_region)
232-
.any(|ur_in_target| {
233-
self.universal_regions.outlives(ur_in_target, ur_in_source)
234-
}) {
255+
.any(|ur_in_target| self.universal_regions.outlives(ur_in_target, ur_in_source))
256+
{
235257
continue;
236258
}
237259

src/librustc_mir/borrow_check/nll/region_infer/dump_mir.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
8484
sub,
8585
point,
8686
span,
87+
next: _,
8788
} = constraint;
8889
with_msg(&format!(
8990
"{:?}: {:?} @ {:?} due to {:?}",

src/librustc_mir/borrow_check/nll/region_infer/graphviz.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ impl<'this, 'tcx> dot::GraphWalk<'this> for RegionInferenceContext<'tcx> {
5555
vids.into_cow()
5656
}
5757
fn edges(&'this self) -> dot::Edges<'this, Constraint> {
58-
(&self.constraints[..]).into_cow()
58+
(&self.constraints.raw[..]).into_cow()
5959
}
6060

6161
// Render `a: b` as `a <- b`, indicating the flow

0 commit comments

Comments
 (0)