From 343eee6082a90016b315b82b048e5a6774472afe Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 7 Jan 2020 21:45:49 +0100 Subject: [PATCH 1/2] perf: Filter out and process fixed constraints first in region expansion Should reduce the number of elements as well as branches in the extremely hot loop and process_constraint in benchmarks such as unicode_normalization --- .../infer/lexical_region_resolve/mod.rs | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/librustc/infer/lexical_region_resolve/mod.rs b/src/librustc/infer/lexical_region_resolve/mod.rs index 0bc49a29015..f4c1965a041 100644 --- a/src/librustc/infer/lexical_region_resolve/mod.rs +++ b/src/librustc/infer/lexical_region_resolve/mod.rs @@ -295,30 +295,49 @@ fn enforce_member_constraint( } fn expansion(&self, var_values: &mut LexicalRegionResolutions<'tcx>) { - let mut process_constraint = |constraint: &Constraint<'tcx>| { - let (a_region, b_vid, b_data, retain) = match *constraint { + let mut changed = false; + let mut constraints = Vec::new(); + for constraint in self.data.constraints.keys() { + let (a_region, b_vid, b_data) = match *constraint { Constraint::RegSubVar(a_region, b_vid) => { let b_data = var_values.value_mut(b_vid); - (a_region, b_vid, b_data, false) + (a_region, b_vid, b_data) } Constraint::VarSubVar(a_vid, b_vid) => match *var_values.value(a_vid) { - VarValue::ErrorValue => return (false, false), + VarValue::ErrorValue => continue, VarValue::Value(a_region) => { let b_data = var_values.value_mut(b_vid); - let retain = match *b_data { - VarValue::Value(ReStatic) | VarValue::ErrorValue => false, - _ => true, - }; - (a_region, b_vid, b_data, retain) + match *b_data { + VarValue::Value(ReStatic) | VarValue::ErrorValue => (), + _ => constraints.push((a_vid, b_vid)), + } + (a_region, b_vid, b_data) } }, Constraint::RegSubReg(..) | Constraint::VarSubReg(..) => { // These constraints are checked after expansion // is done, in `collect_errors`. - return (false, false); + continue; } }; + let edge_changed = self.expand_node(a_region, b_vid, b_data); + if edge_changed { + changed = true + } + } + let mut process_constraint = |a_vid, b_vid| { + let (a_region, b_data, retain) = match *var_values.value(a_vid) { + VarValue::ErrorValue => return (false, false), + VarValue::Value(a_region) => { + let b_data = var_values.value_mut(b_vid); + let retain = match *b_data { + VarValue::Value(ReStatic) | VarValue::ErrorValue => false, + _ => true, + }; + (a_region, b_data, retain) + } + }; let changed = self.expand_node(a_region, b_vid, b_data); (changed, retain) }; @@ -326,15 +345,13 @@ fn expansion(&self, var_values: &mut LexicalRegionResolutions<'tcx>) { // Using bitsets to track the remaining elements is faster than using a // `Vec` by itself (which requires removing elements, which requires // element shuffling, which is slow). - let constraints: Vec<_> = self.data.constraints.keys().collect(); let mut live_indices: BitSet = BitSet::new_filled(constraints.len()); let mut killed_indices: BitSet = BitSet::new_empty(constraints.len()); - let mut changed = true; while changed { changed = false; for index in live_indices.iter() { - let constraint = constraints[index]; - let (edge_changed, retain) = process_constraint(constraint); + let (a_vid, b_vid) = constraints[index]; + let (edge_changed, retain) = process_constraint(a_vid, b_vid); changed |= edge_changed; if !retain { let changed = killed_indices.insert(index); @@ -790,8 +807,8 @@ fn region_order_key(x: &RegionAndOrigin<'_>) -> u8 { self.var_infos[node_idx].origin.span(), &format!( "collect_error_for_expanding_node() could not find \ - error for var {:?} in universe {:?}, lower_bounds={:#?}, \ - upper_bounds={:#?}", + error for var {:?} in universe {:?}, lower_bounds={:#?}, \ + upper_bounds={:#?}", node_idx, node_universe, lower_bounds, upper_bounds ), ); From 917eb187905591c5470cca3e742c17b0fbb495a2 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 8 Jan 2020 10:35:51 +0100 Subject: [PATCH 2/2] perf: Only search the potentially changed constraints in lexical_region_resolve --- .../infer/lexical_region_resolve/mod.rs | 79 +++++++------------ 1 file changed, 29 insertions(+), 50 deletions(-) diff --git a/src/librustc/infer/lexical_region_resolve/mod.rs b/src/librustc/infer/lexical_region_resolve/mod.rs index f4c1965a041..18c25ef0dd9 100644 --- a/src/librustc/infer/lexical_region_resolve/mod.rs +++ b/src/librustc/infer/lexical_region_resolve/mod.rs @@ -19,7 +19,6 @@ Direction, Graph, NodeIndex, INCOMING, OUTGOING, }; use rustc_hir::def_id::DefId; -use rustc_index::bit_set::BitSet; use rustc_index::vec::{Idx, IndexVec}; use rustc_span::Span; use std::fmt; @@ -295,23 +294,19 @@ fn enforce_member_constraint( } fn expansion(&self, var_values: &mut LexicalRegionResolutions<'tcx>) { - let mut changed = false; - let mut constraints = Vec::new(); + let mut constraints = IndexVec::from_elem_n(Vec::new(), var_values.values.len()); + let mut changes = Vec::new(); for constraint in self.data.constraints.keys() { - let (a_region, b_vid, b_data) = match *constraint { + let (a_vid, a_region, b_vid, b_data) = match *constraint { Constraint::RegSubVar(a_region, b_vid) => { let b_data = var_values.value_mut(b_vid); - (a_region, b_vid, b_data) + (None, a_region, b_vid, b_data) } Constraint::VarSubVar(a_vid, b_vid) => match *var_values.value(a_vid) { VarValue::ErrorValue => continue, VarValue::Value(a_region) => { let b_data = var_values.value_mut(b_vid); - match *b_data { - VarValue::Value(ReStatic) | VarValue::ErrorValue => (), - _ => constraints.push((a_vid, b_vid)), - } - (a_region, b_vid, b_data) + (Some(a_vid), a_region, b_vid, b_data) } }, Constraint::RegSubReg(..) | Constraint::VarSubReg(..) => { @@ -320,54 +315,38 @@ fn expansion(&self, var_values: &mut LexicalRegionResolutions<'tcx>) { continue; } }; - let edge_changed = self.expand_node(a_region, b_vid, b_data); - if edge_changed { - changed = true + if self.expand_node(a_region, b_vid, b_data) { + changes.push(b_vid); + } + if let Some(a_vid) = a_vid { + match *b_data { + VarValue::Value(ReStatic) | VarValue::ErrorValue => (), + _ => { + constraints[a_vid].push((a_vid, b_vid)); + constraints[b_vid].push((a_vid, b_vid)); + } + } } } - let mut process_constraint = |a_vid, b_vid| { - let (a_region, b_data, retain) = match *var_values.value(a_vid) { - VarValue::ErrorValue => return (false, false), - VarValue::Value(a_region) => { - let b_data = var_values.value_mut(b_vid); - let retain = match *b_data { - VarValue::Value(ReStatic) | VarValue::ErrorValue => false, - _ => true, - }; - (a_region, b_data, retain) + while let Some(vid) = changes.pop() { + constraints[vid].retain(|&(a_vid, b_vid)| { + let a_region = match *var_values.value(a_vid) { + VarValue::ErrorValue => return false, + VarValue::Value(a_region) => a_region, + }; + let b_data = var_values.value_mut(b_vid); + if self.expand_node(a_region, b_vid, b_data) { + changes.push(b_vid); } - }; - let changed = self.expand_node(a_region, b_vid, b_data); - (changed, retain) - }; - - // Using bitsets to track the remaining elements is faster than using a - // `Vec` by itself (which requires removing elements, which requires - // element shuffling, which is slow). - let mut live_indices: BitSet = BitSet::new_filled(constraints.len()); - let mut killed_indices: BitSet = BitSet::new_empty(constraints.len()); - while changed { - changed = false; - for index in live_indices.iter() { - let (a_vid, b_vid) = constraints[index]; - let (edge_changed, retain) = process_constraint(a_vid, b_vid); - changed |= edge_changed; - if !retain { - let changed = killed_indices.insert(index); - debug_assert!(changed); + match *b_data { + VarValue::Value(ReStatic) | VarValue::ErrorValue => false, + _ => true, } - } - live_indices.subtract(&killed_indices); - - // We could clear `killed_indices` here, but we don't need to and - // it's cheaper not to. + }); } } - // This function is very hot in some workloads. There's a single callsite - // so always inlining is ok even though it's large. - #[inline(always)] fn expand_node( &self, a_region: Region<'tcx>,