From 87644d85a2e91c285af9e3159a70581afd6a70bd Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 10 Dec 2021 11:34:05 +0100 Subject: [PATCH 1/3] Print a backtrace when query forcing fails. --- .../rustc_query_system/src/dep_graph/graph.rs | 56 +++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 47b2fd8f8f4..11c10b9c6ef 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -671,15 +671,45 @@ impl DepGraph { let prev_index = data.previous.node_to_index_opt(dep_node)?; match data.colors.get(prev_index) { - Some(DepNodeColor::Green(dep_node_index)) => Some((prev_index, dep_node_index)), - Some(DepNodeColor::Red) => None, - None => { - // This DepNode and the corresponding query invocation existed - // in the previous compilation session too, so we can try to - // mark it as green by recursively marking all of its - // dependencies green. - self.try_mark_previous_green(qcx, data, prev_index, &dep_node) - .map(|dep_node_index| (prev_index, dep_node_index)) + Some(DepNodeColor::Green(dep_node_index)) => return Some((prev_index, dep_node_index)), + Some(DepNodeColor::Red) => return None, + None => {} + } + + let mut stack = + MarkingStack { stack: vec![prev_index], sess: qcx.dep_context().sess(), graph: data }; + + // This DepNode and the corresponding query invocation existed + // in the previous compilation session too, so we can try to + // mark it as green by recursively marking all of its + // dependencies green. + return self + .try_mark_previous_green(qcx, data, prev_index, &dep_node, &mut stack.stack) + .map(|dep_node_index| (prev_index, dep_node_index)); + + /// Remember the stack of queries we are forcing in the event of an incr. comp. panic. + struct MarkingStack<'a, K: DepKind> { + stack: Vec, + sess: &'a rustc_session::Session, + graph: &'a DepGraphData, + } + + impl<'a, K: DepKind> Drop for MarkingStack<'a, K> { + /// Print the forcing backtrace. + fn drop(&mut self) { + for &frame in self.stack.iter().skip(1).rev() { + let node = self.graph.previous.index_to_node(frame); + // Do not try to rely on DepNode's Debug implementation, + // since it may panic. + let diag = rustc_errors::Diagnostic::new( + rustc_errors::Level::FailureNote, + &format!( + "encountered while trying to mark dependency green: {:?}({})", + node.kind, node.hash + ), + ); + self.sess.diagnostic().force_print_diagnostic(diag); + } } } } @@ -691,6 +721,7 @@ impl DepGraph { data: &DepGraphData, parent_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, + stack: &mut Vec, ) -> Option<()> { let dep_dep_node_color = data.colors.get(parent_dep_node_index); let dep_dep_node = &data.previous.index_to_node(parent_dep_node_index); @@ -723,7 +754,7 @@ impl DepGraph { ); let node_index = - self.try_mark_previous_green(qcx, data, parent_dep_node_index, dep_dep_node); + self.try_mark_previous_green(qcx, data, parent_dep_node_index, dep_dep_node, stack); if node_index.is_some() { debug!("managed to MARK dependency {dep_dep_node:?} as green",); @@ -779,6 +810,7 @@ impl DepGraph { data: &DepGraphData, prev_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, + stack: &mut Vec, ) -> Option { #[cfg(not(parallel_compiler))] { @@ -794,7 +826,9 @@ impl DepGraph { let prev_deps = data.previous.edge_targets_from(prev_dep_node_index); for &dep_dep_node_index in prev_deps { - self.try_mark_parent_green(qcx, data, dep_dep_node_index, dep_node)? + stack.push(dep_dep_node_index); + self.try_mark_parent_green(qcx, data, dep_dep_node_index, dep_node, stack)?; + stack.pop(); } // If we got here without hitting a `return` that means that all From e33e2d6e9e29a159b1774cc47775d19fc74bbc80 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 11 Dec 2021 10:27:59 +0100 Subject: [PATCH 2/3] Attempt to reduce perf impact. --- .../rustc_query_system/src/dep_graph/graph.rs | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 11c10b9c6ef..b7381326fcb 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -676,28 +676,35 @@ impl DepGraph { None => {} } - let mut stack = - MarkingStack { stack: vec![prev_index], sess: qcx.dep_context().sess(), graph: data }; + let mut stack = smallvec![prev_index]; + let _backtrace_print = + MarkingStack { stack: &mut stack, sess: qcx.dep_context().sess(), graph: data }; // This DepNode and the corresponding query invocation existed // in the previous compilation session too, so we can try to // mark it as green by recursively marking all of its // dependencies green. - return self - .try_mark_previous_green(qcx, data, prev_index, &dep_node, &mut stack.stack) + let ret = self + .try_mark_previous_green(qcx, data, prev_index, &dep_node, _backtrace_print.stack) .map(|dep_node_index| (prev_index, dep_node_index)); + // We succeeded, no backtrace. + std::mem::forget(_backtrace_print); + return ret; + /// Remember the stack of queries we are forcing in the event of an incr. comp. panic. - struct MarkingStack<'a, K: DepKind> { - stack: Vec, + struct MarkingStack<'a, 'v, K: DepKind> { + stack: &'v mut SmallVec<[SerializedDepNodeIndex; 8]>, sess: &'a rustc_session::Session, graph: &'a DepGraphData, } - impl<'a, K: DepKind> Drop for MarkingStack<'a, K> { + impl<'a, 'v, K: DepKind> Drop for MarkingStack<'a, 'v, K> { /// Print the forcing backtrace. + #[inline(never)] + #[cold] fn drop(&mut self) { - for &frame in self.stack.iter().skip(1).rev() { + for &frame in self.stack.iter().rev() { let node = self.graph.previous.index_to_node(frame); // Do not try to rely on DepNode's Debug implementation, // since it may panic. @@ -721,7 +728,7 @@ impl DepGraph { data: &DepGraphData, parent_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, - stack: &mut Vec, + stack: &mut SmallVec<[SerializedDepNodeIndex; 8]>, ) -> Option<()> { let dep_dep_node_color = data.colors.get(parent_dep_node_index); let dep_dep_node = &data.previous.index_to_node(parent_dep_node_index); @@ -810,7 +817,7 @@ impl DepGraph { data: &DepGraphData, prev_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, - stack: &mut Vec, + stack: &mut SmallVec<[SerializedDepNodeIndex; 8]>, ) -> Option { #[cfg(not(parallel_compiler))] { From 870dd1678ecf8c8ea39d67a1147f02fca5be4a6d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 28 Jan 2023 12:38:22 +0000 Subject: [PATCH 3/3] Use OnDrop. --- .../rustc_query_system/src/dep_graph/graph.rs | 71 +++++++++---------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index b7381326fcb..29f94304da1 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -6,6 +6,7 @@ use rustc_data_structures::sharded::{self, Sharded}; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_data_structures::steal::Steal; use rustc_data_structures::sync::{AtomicU32, AtomicU64, Lock, Lrc, Ordering}; +use rustc_data_structures::OnDrop; use rustc_index::vec::IndexVec; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use smallvec::{smallvec, SmallVec}; @@ -676,49 +677,19 @@ impl DepGraph { None => {} } - let mut stack = smallvec![prev_index]; - let _backtrace_print = - MarkingStack { stack: &mut stack, sess: qcx.dep_context().sess(), graph: data }; + let backtrace = backtrace_printer(qcx.dep_context().sess(), data, prev_index); // This DepNode and the corresponding query invocation existed // in the previous compilation session too, so we can try to // mark it as green by recursively marking all of its // dependencies green. let ret = self - .try_mark_previous_green(qcx, data, prev_index, &dep_node, _backtrace_print.stack) + .try_mark_previous_green(qcx, data, prev_index, &dep_node) .map(|dep_node_index| (prev_index, dep_node_index)); // We succeeded, no backtrace. - std::mem::forget(_backtrace_print); + backtrace.disable(); return ret; - - /// Remember the stack of queries we are forcing in the event of an incr. comp. panic. - struct MarkingStack<'a, 'v, K: DepKind> { - stack: &'v mut SmallVec<[SerializedDepNodeIndex; 8]>, - sess: &'a rustc_session::Session, - graph: &'a DepGraphData, - } - - impl<'a, 'v, K: DepKind> Drop for MarkingStack<'a, 'v, K> { - /// Print the forcing backtrace. - #[inline(never)] - #[cold] - fn drop(&mut self) { - for &frame in self.stack.iter().rev() { - let node = self.graph.previous.index_to_node(frame); - // Do not try to rely on DepNode's Debug implementation, - // since it may panic. - let diag = rustc_errors::Diagnostic::new( - rustc_errors::Level::FailureNote, - &format!( - "encountered while trying to mark dependency green: {:?}({})", - node.kind, node.hash - ), - ); - self.sess.diagnostic().force_print_diagnostic(diag); - } - } - } } #[instrument(skip(self, qcx, data, parent_dep_node_index), level = "debug")] @@ -728,7 +699,6 @@ impl DepGraph { data: &DepGraphData, parent_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, - stack: &mut SmallVec<[SerializedDepNodeIndex; 8]>, ) -> Option<()> { let dep_dep_node_color = data.colors.get(parent_dep_node_index); let dep_dep_node = &data.previous.index_to_node(parent_dep_node_index); @@ -761,7 +731,7 @@ impl DepGraph { ); let node_index = - self.try_mark_previous_green(qcx, data, parent_dep_node_index, dep_dep_node, stack); + self.try_mark_previous_green(qcx, data, parent_dep_node_index, dep_dep_node); if node_index.is_some() { debug!("managed to MARK dependency {dep_dep_node:?} as green",); @@ -817,7 +787,6 @@ impl DepGraph { data: &DepGraphData, prev_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, - stack: &mut SmallVec<[SerializedDepNodeIndex; 8]>, ) -> Option { #[cfg(not(parallel_compiler))] { @@ -833,9 +802,10 @@ impl DepGraph { let prev_deps = data.previous.edge_targets_from(prev_dep_node_index); for &dep_dep_node_index in prev_deps { - stack.push(dep_dep_node_index); - self.try_mark_parent_green(qcx, data, dep_dep_node_index, dep_node, stack)?; - stack.pop(); + let backtrace = backtrace_printer(qcx.dep_context().sess(), data, dep_dep_node_index); + let success = self.try_mark_parent_green(qcx, data, dep_dep_node_index, dep_node); + backtrace.disable(); + success?; } // If we got here without hitting a `return` that means that all @@ -1405,3 +1375,26 @@ impl DepNodeColorMap { ) } } + +fn backtrace_printer<'a, K: DepKind>( + sess: &'a rustc_session::Session, + graph: &'a DepGraphData, + node: SerializedDepNodeIndex, +) -> OnDrop { + OnDrop( + #[inline(never)] + #[cold] + move || { + let node = graph.previous.index_to_node(node); + // Do not try to rely on DepNode's Debug implementation, since it may panic. + let diag = rustc_errors::Diagnostic::new( + rustc_errors::Level::FailureNote, + &format!( + "encountered while trying to mark dependency green: {:?}({})", + node.kind, node.hash + ), + ); + sess.diagnostic().force_print_diagnostic(diag); + }, + ) +}