From 476e75ded71ad6f573daabe2ca7a1be9fe3ce3cb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Sep 2019 12:08:24 +1000 Subject: [PATCH 1/5] Move a `Node`'s parent into the descendents list. `Node` has an optional parent and a list of other descendents. Most of the time the parent is treated the same as the other descendents -- error-handling is the exception -- and chaining them together for iteration has a non-trivial cost. This commit changes the representation. There is now a single list of descendants, and a boolean flag that indicates if there is a parent (in which case it is first descendent). This representation encodes the same information, in a way that is less idiomatic but cheaper to iterate over for the common case where the parent doesn't need special treatment. As a result, some benchmark workloads are up to 2% faster. --- .../obligation_forest/graphviz.rs | 4 +- .../obligation_forest/mod.rs | 75 +++++++++---------- 2 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/librustc_data_structures/obligation_forest/graphviz.rs b/src/librustc_data_structures/obligation_forest/graphviz.rs index b2120b182fa..fe0a3cb0500 100644 --- a/src/librustc_data_structures/obligation_forest/graphviz.rs +++ b/src/librustc_data_structures/obligation_forest/graphviz.rs @@ -74,9 +74,7 @@ impl<'a, O: ForestObligation + 'a> dot::GraphWalk<'a> for &'a ObligationForest { obligation: O, state: Cell, - /// The parent of a node - the original obligation of which it is a - /// subobligation. Except for error reporting, it is just like any member - /// of `dependents`. - /// - /// Unlike `ObligationForest::nodes`, this uses `NodeIndex` rather than - /// `usize` for the index, because keeping the size down is more important - /// than the cost of converting to a `usize` for indexing. - parent: Option, - /// Obligations that depend on this obligation for their completion. They /// must all be in a non-pending state. - /// - /// This uses `NodeIndex` for the same reason as `parent`. dependents: Vec, + /// If true, dependents[0] points to a "parent" node, which requires + /// special treatment upon error but is otherwise treated the same. + /// (It would be more idiomatic to store the parent node in a separate + /// `Option` field, but that slows down the common case of + /// iterating over the parent and other descendants together.) + has_parent: bool, + /// Identifier of the obligation tree to which this node belongs. obligation_tree_id: ObligationTreeId, } @@ -205,8 +201,13 @@ impl Node { Node { obligation, state: Cell::new(NodeState::Pending), - parent, - dependents: vec![], + dependents: + if let Some(parent_index) = parent { + vec![parent_index] + } else { + vec![] + }, + has_parent: parent.is_some(), obligation_tree_id, } } @@ -315,13 +316,11 @@ impl ObligationForest { obligation, parent, o.get()); let node = &mut self.nodes[o.get().index()]; if let Some(parent_index) = parent { - // If the node is already in `waiting_cache`, it's already - // been marked with a parent. (It's possible that parent - // has been cleared by `apply_rewrites`, though.) So just - // dump `parent` into `node.dependents`... unless it's - // already in `node.dependents` or `node.parent`. - if !node.dependents.contains(&parent_index) && - Some(parent_index) != node.parent { + // If the node is already in `waiting_cache`, it has + // already had its chance to be marked with a parent. So if + // it's not already present, just dump `parent` into the + // dependents as a non-parent. + if !node.dependents.contains(&parent_index) { node.dependents.push(parent_index); } } @@ -523,7 +522,7 @@ impl ObligationForest { NodeState::Success => { node.state.set(NodeState::OnDfsStack); stack.push(i); - for index in node.parent.iter().chain(node.dependents.iter()) { + for index in node.dependents.iter() { self.find_cycles_from_node(stack, processor, index.index()); } stack.pop(); @@ -549,12 +548,15 @@ impl ObligationForest { let node = &self.nodes[i]; node.state.set(NodeState::Error); trace.push(node.obligation.clone()); - error_stack.extend(node.dependents.iter().map(|index| index.index())); - - // Loop to the parent. - match node.parent { - Some(parent_index) => i = parent_index.index(), - None => break + if node.has_parent { + // The first dependent is the parent, which is treated + // specially. + error_stack.extend(node.dependents.iter().skip(1).map(|index| index.index())); + i = node.dependents[0].index(); + } else { + // No parent; treat all dependents non-specially. + error_stack.extend(node.dependents.iter().map(|index| index.index())); + break; } } @@ -565,9 +567,7 @@ impl ObligationForest { _ => node.state.set(NodeState::Error), } - error_stack.extend( - node.parent.iter().chain(node.dependents.iter()).map(|index| index.index()) - ); + error_stack.extend(node.dependents.iter().map(|index| index.index())); } self.scratch.replace(error_stack); @@ -577,7 +577,7 @@ impl ObligationForest { // This always-inlined function is for the hot call site. #[inline(always)] fn inlined_mark_neighbors_as_waiting_from(&self, node: &Node) { - for dependent in node.parent.iter().chain(node.dependents.iter()) { + for dependent in node.dependents.iter() { self.mark_as_waiting_from(&self.nodes[dependent.index()]); } } @@ -706,20 +706,15 @@ impl ObligationForest { let nodes_len = node_rewrites.len(); for node in &mut self.nodes { - if let Some(index) = node.parent { - let new_i = node_rewrites[index.index()]; - if new_i >= nodes_len { - node.parent = None; - } else { - node.parent = Some(NodeIndex::new(new_i)); - } - } - let mut i = 0; while i < node.dependents.len() { let new_i = node_rewrites[node.dependents[i].index()]; if new_i >= nodes_len { node.dependents.swap_remove(i); + if i == 0 && node.has_parent { + // We just removed the parent. + node.has_parent = false; + } } else { node.dependents[i] = NodeIndex::new(new_i); i += 1; From cf3a562c981252f2f4e4c89d7cae353ea00529e3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Sep 2019 14:19:44 +1000 Subject: [PATCH 2/5] Remove `NodeIndex`. The size of the indices doesn't matter much here, and having a `newtype_index!` index type without also using `IndexVec` requires lots of conversions. So this commit removes `NodeIndex` in favour of uniform use of `usize` as the index type. As well as making the code slightly more concise, it also slightly speeds things up. --- .../obligation_forest/graphviz.rs | 2 +- .../obligation_forest/mod.rs | 61 ++++++++----------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/librustc_data_structures/obligation_forest/graphviz.rs b/src/librustc_data_structures/obligation_forest/graphviz.rs index fe0a3cb0500..ddf89d99621 100644 --- a/src/librustc_data_structures/obligation_forest/graphviz.rs +++ b/src/librustc_data_structures/obligation_forest/graphviz.rs @@ -74,7 +74,7 @@ impl<'a, O: ForestObligation + 'a> dot::GraphWalk<'a> for &'a ObligationForest { /// At the end of processing, those nodes will be removed by a /// call to `compress`. /// - /// Ideally, this would be an `IndexVec>`. But that is - /// slower, because this vector is accessed so often that the - /// `u32`-to-`usize` conversions required for accesses are significant. + /// `usize` indices are used here and throughout this module, rather than + /// `newtype_index!` indices, because this code is hot enough that the + /// `u32`-to-`usize` conversions that would be required are significant, + /// and space considerations are not important. nodes: Vec>, /// A cache of predicates that have been successfully completed. @@ -154,7 +149,7 @@ pub struct ObligationForest { /// A cache of the nodes in `nodes`, indexed by predicate. Unfortunately, /// its contents are not guaranteed to match those of `nodes`. See the /// comments in `process_obligation` for details. - waiting_cache: FxHashMap, + waiting_cache: FxHashMap, /// A scratch vector reused in various operations, to avoid allocating new /// vectors. @@ -179,12 +174,12 @@ struct Node { /// Obligations that depend on this obligation for their completion. They /// must all be in a non-pending state. - dependents: Vec, + dependents: Vec, /// If true, dependents[0] points to a "parent" node, which requires /// special treatment upon error but is otherwise treated the same. /// (It would be more idiomatic to store the parent node in a separate - /// `Option` field, but that slows down the common case of + /// `Option` field, but that slows down the common case of /// iterating over the parent and other descendants together.) has_parent: bool, @@ -194,7 +189,7 @@ struct Node { impl Node { fn new( - parent: Option, + parent: Option, obligation: O, obligation_tree_id: ObligationTreeId ) -> Node { @@ -303,9 +298,7 @@ impl ObligationForest { } // Returns Err(()) if we already know this obligation failed. - fn register_obligation_at(&mut self, obligation: O, parent: Option) - -> Result<(), ()> - { + fn register_obligation_at(&mut self, obligation: O, parent: Option) -> Result<(), ()> { if self.done_cache.contains(obligation.as_predicate()) { return Ok(()); } @@ -314,7 +307,7 @@ impl ObligationForest { Entry::Occupied(o) => { debug!("register_obligation_at({:?}, {:?}) - duplicate of {:?}!", obligation, parent, o.get()); - let node = &mut self.nodes[o.get().index()]; + let node = &mut self.nodes[*o.get()]; if let Some(parent_index) = parent { // If the node is already in `waiting_cache`, it has // already had its chance to be marked with a parent. So if @@ -335,10 +328,8 @@ impl ObligationForest { obligation, parent, self.nodes.len()); let obligation_tree_id = match parent { - Some(parent_index) => { - self.nodes[parent_index.index()].obligation_tree_id - } - None => self.obligation_tree_id_generator.next().unwrap() + Some(parent_index) => self.nodes[parent_index].obligation_tree_id, + None => self.obligation_tree_id_generator.next().unwrap(), }; let already_failed = @@ -351,7 +342,7 @@ impl ObligationForest { if already_failed { Err(()) } else { - v.insert(NodeIndex::new(self.nodes.len())); + v.insert(self.nodes.len()); self.nodes.push(Node::new(parent, obligation, obligation_tree_id)); Ok(()) } @@ -437,7 +428,7 @@ impl ObligationForest { for child in children { let st = self.register_obligation_at( child, - Some(NodeIndex::new(i)) + Some(i) ); if let Err(()) = st { // Error already reported - propagate it @@ -522,8 +513,8 @@ impl ObligationForest { NodeState::Success => { node.state.set(NodeState::OnDfsStack); stack.push(i); - for index in node.dependents.iter() { - self.find_cycles_from_node(stack, processor, index.index()); + for &index in node.dependents.iter() { + self.find_cycles_from_node(stack, processor, index); } stack.pop(); node.state.set(NodeState::Done); @@ -551,11 +542,11 @@ impl ObligationForest { if node.has_parent { // The first dependent is the parent, which is treated // specially. - error_stack.extend(node.dependents.iter().skip(1).map(|index| index.index())); - i = node.dependents[0].index(); + error_stack.extend(node.dependents.iter().skip(1)); + i = node.dependents[0]; } else { // No parent; treat all dependents non-specially. - error_stack.extend(node.dependents.iter().map(|index| index.index())); + error_stack.extend(node.dependents.iter()); break; } } @@ -567,7 +558,7 @@ impl ObligationForest { _ => node.state.set(NodeState::Error), } - error_stack.extend(node.dependents.iter().map(|index| index.index())); + error_stack.extend(node.dependents.iter()); } self.scratch.replace(error_stack); @@ -577,8 +568,8 @@ impl ObligationForest { // This always-inlined function is for the hot call site. #[inline(always)] fn inlined_mark_neighbors_as_waiting_from(&self, node: &Node) { - for dependent in node.dependents.iter() { - self.mark_as_waiting_from(&self.nodes[dependent.index()]); + for &dependent in node.dependents.iter() { + self.mark_as_waiting_from(&self.nodes[dependent]); } } @@ -708,7 +699,7 @@ impl ObligationForest { for node in &mut self.nodes { let mut i = 0; while i < node.dependents.len() { - let new_i = node_rewrites[node.dependents[i].index()]; + let new_i = node_rewrites[node.dependents[i]]; if new_i >= nodes_len { node.dependents.swap_remove(i); if i == 0 && node.has_parent { @@ -716,7 +707,7 @@ impl ObligationForest { node.has_parent = false; } } else { - node.dependents[i] = NodeIndex::new(new_i); + node.dependents[i] = new_i; i += 1; } } @@ -725,11 +716,11 @@ impl ObligationForest { // This updating of `self.waiting_cache` is necessary because the // removal of nodes within `compress` can fail. See above. self.waiting_cache.retain(|_predicate, index| { - let new_i = node_rewrites[index.index()]; + let new_i = node_rewrites[*index]; if new_i >= nodes_len { false } else { - *index = NodeIndex::new(new_i); + *index = new_i; true } }); From 7f6e160875f375bb71d1ae761cb5cab8f0b02e19 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Sep 2019 14:23:32 +1000 Subject: [PATCH 3/5] Rename some index variables. Now that all indices have type `usize`, it makes sense to be more consistent about their naming. This commit removes all uses of `i` in favour of `index`. --- .../obligation_forest/mod.rs | 88 +++++++++---------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/src/librustc_data_structures/obligation_forest/mod.rs b/src/librustc_data_structures/obligation_forest/mod.rs index afec7a24b17..98ae1a58324 100644 --- a/src/librustc_data_structures/obligation_forest/mod.rs +++ b/src/librustc_data_structures/obligation_forest/mod.rs @@ -353,9 +353,9 @@ impl ObligationForest { /// Converts all remaining obligations to the given error. pub fn to_errors(&mut self, error: E) -> Vec> { let mut errors = vec![]; - for (i, node) in self.nodes.iter().enumerate() { + for (index, node) in self.nodes.iter().enumerate() { if let NodeState::Pending = node.state.get() { - let backtrace = self.error_at(i); + let backtrace = self.error_at(index); errors.push(Error { error: error.clone(), backtrace, @@ -399,10 +399,10 @@ impl ObligationForest { let mut errors = vec![]; let mut stalled = true; - for i in 0..self.nodes.len() { - let node = &mut self.nodes[i]; + for index in 0..self.nodes.len() { + let node = &mut self.nodes[index]; - debug!("process_obligations: node {} == {:?}", i, node); + debug!("process_obligations: node {} == {:?}", index, node); // `processor.process_obligation` can modify the predicate within // `node.obligation`, and that predicate is the key used for @@ -414,7 +414,7 @@ impl ObligationForest { _ => continue }; - debug!("process_obligations: node {} got result {:?}", i, result); + debug!("process_obligations: node {} got result {:?}", index, result); match result { ProcessResult::Unchanged => { @@ -428,18 +428,18 @@ impl ObligationForest { for child in children { let st = self.register_obligation_at( child, - Some(i) + Some(index) ); if let Err(()) = st { // Error already reported - propagate it // to our node. - self.error_at(i); + self.error_at(index); } } } ProcessResult::Error(err) => { stalled = false; - let backtrace = self.error_at(i); + let backtrace = self.error_at(index); errors.push(Error { error: err, backtrace, @@ -483,14 +483,14 @@ impl ObligationForest { debug!("process_cycles()"); - for (i, node) in self.nodes.iter().enumerate() { + for (index, node) in self.nodes.iter().enumerate() { // For rustc-benchmarks/inflate-0.1.0 this state test is extremely // hot and the state is almost always `Pending` or `Waiting`. It's // a win to handle the no-op cases immediately to avoid the cost of // the function call. match node.state.get() { NodeState::Waiting | NodeState::Pending | NodeState::Done | NodeState::Error => {}, - _ => self.find_cycles_from_node(&mut stack, processor, i), + _ => self.find_cycles_from_node(&mut stack, processor, index), } } @@ -500,19 +500,19 @@ impl ObligationForest { self.scratch.replace(stack); } - fn find_cycles_from_node

(&self, stack: &mut Vec, processor: &mut P, i: usize) + fn find_cycles_from_node

(&self, stack: &mut Vec, processor: &mut P, index: usize) where P: ObligationProcessor { - let node = &self.nodes[i]; + let node = &self.nodes[index]; match node.state.get() { NodeState::OnDfsStack => { - let i = stack.iter().rposition(|n| *n == i).unwrap(); - processor.process_backedge(stack[i..].iter().map(GetObligation(&self.nodes)), + let index = stack.iter().rposition(|&n| n == index).unwrap(); + processor.process_backedge(stack[index..].iter().map(GetObligation(&self.nodes)), PhantomData); } NodeState::Success => { node.state.set(NodeState::OnDfsStack); - stack.push(i); + stack.push(index); for &index in node.dependents.iter() { self.find_cycles_from_node(stack, processor, index); } @@ -531,19 +531,19 @@ impl ObligationForest { /// Returns a vector of obligations for `p` and all of its /// ancestors, putting them into the error state in the process. - fn error_at(&self, mut i: usize) -> Vec { + fn error_at(&self, mut index: usize) -> Vec { let mut error_stack = self.scratch.replace(vec![]); let mut trace = vec![]; loop { - let node = &self.nodes[i]; + let node = &self.nodes[index]; node.state.set(NodeState::Error); trace.push(node.obligation.clone()); if node.has_parent { // The first dependent is the parent, which is treated // specially. error_stack.extend(node.dependents.iter().skip(1)); - i = node.dependents[0]; + index = node.dependents[0]; } else { // No parent; treat all dependents non-specially. error_stack.extend(node.dependents.iter()); @@ -551,8 +551,8 @@ impl ObligationForest { } } - while let Some(i) = error_stack.pop() { - let node = &self.nodes[i]; + while let Some(index) = error_stack.pop() { + let node = &self.nodes[index]; match node.state.get() { NodeState::Error => continue, _ => node.state.set(NodeState::Error), @@ -568,8 +568,8 @@ impl ObligationForest { // This always-inlined function is for the hot call site. #[inline(always)] fn inlined_mark_neighbors_as_waiting_from(&self, node: &Node) { - for &dependent in node.dependents.iter() { - self.mark_as_waiting_from(&self.nodes[dependent]); + for &index in node.dependents.iter() { + self.mark_as_waiting_from(&self.nodes[index]); } } @@ -622,16 +622,16 @@ impl ObligationForest { // Now move all popped nodes to the end. Try to keep the order. // // LOOP INVARIANT: - // self.nodes[0..i - dead_nodes] are the first remaining nodes - // self.nodes[i - dead_nodes..i] are all dead - // self.nodes[i..] are unchanged - for i in 0..self.nodes.len() { - let node = &self.nodes[i]; + // self.nodes[0..index - dead_nodes] are the first remaining nodes + // self.nodes[index - dead_nodes..index] are all dead + // self.nodes[index..] are unchanged + for index in 0..self.nodes.len() { + let node = &self.nodes[index]; match node.state.get() { NodeState::Pending | NodeState::Waiting => { if dead_nodes > 0 { - self.nodes.swap(i, i - dead_nodes); - node_rewrites[i] -= dead_nodes; + self.nodes.swap(index, index - dead_nodes); + node_rewrites[index] -= dead_nodes; } } NodeState::Done => { @@ -646,7 +646,7 @@ impl ObligationForest { } else { self.done_cache.insert(node.obligation.as_predicate().clone()); } - node_rewrites[i] = nodes_len; + node_rewrites[index] = nodes_len; dead_nodes += 1; } NodeState::Error => { @@ -654,9 +654,9 @@ impl ObligationForest { // tests must come up with a different type on every type error they // check against. self.waiting_cache.remove(node.obligation.as_predicate()); - node_rewrites[i] = nodes_len; + node_rewrites[index] = nodes_len; dead_nodes += 1; - self.insert_into_error_cache(i); + self.insert_into_error_cache(index); } NodeState::OnDfsStack | NodeState::Success => unreachable!() } @@ -697,18 +697,18 @@ impl ObligationForest { let nodes_len = node_rewrites.len(); for node in &mut self.nodes { - let mut i = 0; - while i < node.dependents.len() { - let new_i = node_rewrites[node.dependents[i]]; - if new_i >= nodes_len { - node.dependents.swap_remove(i); - if i == 0 && node.has_parent { + let mut index = 0; + while index < node.dependents.len() { + let new_index = node_rewrites[node.dependents[index]]; + if new_index >= nodes_len { + node.dependents.swap_remove(index); + if index == 0 && node.has_parent { // We just removed the parent. node.has_parent = false; } } else { - node.dependents[i] = new_i; - i += 1; + node.dependents[index] = new_index; + index += 1; } } } @@ -716,11 +716,11 @@ impl ObligationForest { // This updating of `self.waiting_cache` is necessary because the // removal of nodes within `compress` can fail. See above. self.waiting_cache.retain(|_predicate, index| { - let new_i = node_rewrites[*index]; - if new_i >= nodes_len { + let new_index = node_rewrites[*index]; + if new_index >= nodes_len { false } else { - *index = new_i; + *index = new_index; true } }); From 82e5c6ee6597e7d8926b4f2fea89d02d77f978d4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Sep 2019 16:48:21 +1000 Subject: [PATCH 4/5] Use explicit iteration instead of `all()` in `process_obligation()`. Amazingly enough, this is a 3.5% instruction count win on `keccak`. --- src/librustc/traits/fulfill.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 4494c034d51..5eaaeca82f2 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -256,15 +256,22 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { &mut self, pending_obligation: &mut Self::Obligation, ) -> ProcessResult { - // if we were stalled on some unresolved variables, first check + // If we were stalled on some unresolved variables, first check // whether any of them have been resolved; if not, don't bother // doing more work yet if !pending_obligation.stalled_on.is_empty() { - if pending_obligation.stalled_on.iter().all(|&ty| { + let mut changed = false; + // This `for` loop was once a call to `all()`, but this lower-level + // form was a perf win. See #64545 for details. + for &ty in &pending_obligation.stalled_on { // Use the force-inlined variant of shallow_resolve() because this code is hot. let resolved = ShallowResolver::new(self.selcx.infcx()).inlined_shallow_resolve(ty); - resolved == ty // nothing changed here - }) { + if resolved != ty { + changed = true; + break; + } + } + if !changed { debug!("process_predicate: pending obligation {:?} still stalled on {:?}", self.selcx.infcx() .resolve_vars_if_possible(&pending_obligation.obligation), From 3b85597d22dc9dc9226445a95e275b8130880e63 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 17 Sep 2019 18:05:57 +1000 Subject: [PATCH 5/5] Add a specialized version of `shallow_resolve()`. The super-hot call site of `inlined_shallow_resolve()` basically does `r.inlined_shallow_resolve(ty) != ty`. This commit introduces a version of that function specialized for that particular call pattern, `shallow_resolve_changed()`. Incredibly, this reduces the instruction count for `keccak` by 5%. The commit also renames `inlined_shallow_resolve()` as `shallow_resolve()` and removes the `inline(always)` annotation, as it's no longer nearly so hot. --- src/librustc/infer/mod.rs | 44 +++++++++++++++++++++++++++++----- src/librustc/traits/fulfill.rs | 4 +--- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 8638f42976f..46a364cfe99 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -1558,11 +1558,7 @@ impl<'a, 'tcx> ShallowResolver<'a, 'tcx> { ShallowResolver { infcx } } - // We have this force-inlined variant of `shallow_resolve` for the one - // callsite that is extremely hot. All other callsites use the normal - // variant. - #[inline(always)] - pub fn inlined_shallow_resolve(&mut self, typ: Ty<'tcx>) -> Ty<'tcx> { + pub fn shallow_resolve(&mut self, typ: Ty<'tcx>) -> Ty<'tcx> { match typ.sty { ty::Infer(ty::TyVar(v)) => { // Not entirely obvious: if `typ` is a type variable, @@ -1597,6 +1593,42 @@ impl<'a, 'tcx> ShallowResolver<'a, 'tcx> { _ => typ, } } + + // `resolver.shallow_resolve_changed(ty)` is equivalent to + // `resolver.shallow_resolve(ty) != ty`, but more efficient. It's always + // inlined, despite being large, because it has a single call site that is + // extremely hot. + #[inline(always)] + pub fn shallow_resolve_changed(&mut self, typ: Ty<'tcx>) -> bool { + match typ.sty { + ty::Infer(ty::TyVar(v)) => { + use self::type_variable::TypeVariableValue; + + // See the comment in `shallow_resolve()`. + match self.infcx.type_variables.borrow_mut().probe(v) { + TypeVariableValue::Known { value: t } => self.fold_ty(t) != typ, + TypeVariableValue::Unknown { .. } => false, + } + } + + ty::Infer(ty::IntVar(v)) => { + match self.infcx.int_unification_table.borrow_mut().probe_value(v) { + Some(v) => v.to_type(self.infcx.tcx) != typ, + None => false, + } + } + + ty::Infer(ty::FloatVar(v)) => { + match self.infcx.float_unification_table.borrow_mut().probe_value(v) { + Some(v) => v.to_type(self.infcx.tcx) != typ, + None => false, + } + } + + _ => false, + } + } + } impl<'a, 'tcx> TypeFolder<'tcx> for ShallowResolver<'a, 'tcx> { @@ -1605,7 +1637,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for ShallowResolver<'a, 'tcx> { } fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> { - self.inlined_shallow_resolve(ty) + self.shallow_resolve(ty) } fn fold_const(&mut self, ct: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> { diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 5eaaeca82f2..805727b6ce0 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -264,9 +264,7 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> { // This `for` loop was once a call to `all()`, but this lower-level // form was a perf win. See #64545 for details. for &ty in &pending_obligation.stalled_on { - // Use the force-inlined variant of shallow_resolve() because this code is hot. - let resolved = ShallowResolver::new(self.selcx.infcx()).inlined_shallow_resolve(ty); - if resolved != ty { + if ShallowResolver::new(self.selcx.infcx()).shallow_resolve_changed(ty) { changed = true; break; }