From 8dd0f20ee65883d12775ae4d552596b815ac9977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Mon, 27 Feb 2023 05:56:48 +0100 Subject: [PATCH 1/3] Optimize dep node backtrace and ignore fatal errors --- .../rustc_query_system/src/dep_graph/graph.rs | 90 +++++++++---------- .../rustc_query_system/src/dep_graph/mod.rs | 23 ++++- 2 files changed, 63 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 8a9a12386064f..5604da4696211 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -6,7 +6,6 @@ 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}; @@ -54,6 +53,11 @@ impl From for QueryInvocationId { } } +pub struct MarkFrame<'a> { + index: SerializedDepNodeIndex, + parent: Option<&'a MarkFrame<'a>>, +} + #[derive(PartialEq)] pub enum DepNodeColor { Red, @@ -710,32 +714,26 @@ impl DepGraphData { let prev_index = self.previous.node_to_index_opt(dep_node)?; match self.colors.get(prev_index) { - Some(DepNodeColor::Green(dep_node_index)) => return Some((prev_index, dep_node_index)), - Some(DepNodeColor::Red) => return None, - None => {} + 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, prev_index, &dep_node, None) + .map(|dep_node_index| (prev_index, dep_node_index)) + } } - - let backtrace = backtrace_printer(qcx.dep_context().sess(), self, 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, prev_index, &dep_node) - .map(|dep_node_index| (prev_index, dep_node_index)); - - // We succeeded, no backtrace. - backtrace.disable(); - return ret; } - #[instrument(skip(self, qcx, parent_dep_node_index), level = "debug")] + #[instrument(skip(self, qcx, parent_dep_node_index, frame), level = "debug")] fn try_mark_parent_green>( &self, qcx: Qcx, parent_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, + frame: Option<&MarkFrame<'_>>, ) -> Option<()> { let dep_dep_node_color = self.colors.get(parent_dep_node_index); let dep_dep_node = &self.previous.index_to_node(parent_dep_node_index); @@ -767,7 +765,8 @@ impl DepGraphData { dep_dep_node, dep_dep_node.hash, ); - let node_index = self.try_mark_previous_green(qcx, parent_dep_node_index, dep_dep_node); + let node_index = + self.try_mark_previous_green(qcx, parent_dep_node_index, dep_dep_node, frame); if node_index.is_some() { debug!("managed to MARK dependency {dep_dep_node:?} as green",); @@ -777,7 +776,7 @@ impl DepGraphData { // We failed to mark it green, so we try to force the query. debug!("trying to force dependency {dep_dep_node:?}"); - if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node) { + if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, data, frame) { // The DepNode could not be forced. debug!("dependency {dep_dep_node:?} could not be forced"); return None; @@ -816,13 +815,16 @@ impl DepGraphData { } /// Try to mark a dep-node which existed in the previous compilation session as green. - #[instrument(skip(self, qcx, prev_dep_node_index), level = "debug")] + #[instrument(skip(self, qcx, prev_dep_node_index, frame), level = "debug")] fn try_mark_previous_green>( &self, qcx: Qcx, prev_dep_node_index: SerializedDepNodeIndex, dep_node: &DepNode, + frame: Option<&MarkFrame<'_>>, ) -> Option { + let frame = MarkFrame { index: prev_dep_node_index, parent: frame }; + #[cfg(not(parallel_compiler))] { debug_assert!(!self.dep_node_exists(dep_node)); @@ -837,10 +839,7 @@ impl DepGraphData { let prev_deps = self.previous.edge_targets_from(prev_dep_node_index); for &dep_dep_node_index in prev_deps { - let backtrace = backtrace_printer(qcx.dep_context().sess(), self, dep_dep_node_index); - let success = self.try_mark_parent_green(qcx, dep_dep_node_index, dep_node); - backtrace.disable(); - success?; + self.try_mark_parent_green(qcx, dep_dep_node_index, dep_node, Some(&frame))?; } // If we got here without hitting a `return` that means that all @@ -1414,25 +1413,24 @@ 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); +#[inline(never)] +#[cold] +pub(crate) fn print_markframe_trace( + graph: &DepGraphData, + frame: Option<&MarkFrame<'_>>, +) { + eprintln!("there was a panic while trying to force a dep node"); + eprintln!("try_mark_green dep node stack:"); + + let mut i = 0; + let mut current = frame; + while let Some(frame) = current { // 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); - }, - ) + let node = graph.previous.index_to_node(frame.index); + eprintln!("#{i} {:?} ({})", node.kind, node.hash); + current = frame.parent; + i += 1; + } + + eprintln!("end of try_mark_green dep node stack"); } diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs index 5a7b9ae2ab436..f70a913974362 100644 --- a/compiler/rustc_query_system/src/dep_graph/mod.rs +++ b/compiler/rustc_query_system/src/dep_graph/mod.rs @@ -17,8 +17,10 @@ use rustc_data_structures::profiling::SelfProfilerRef; use rustc_serialize::{opaque::FileEncoder, Encodable}; use rustc_session::Session; -use std::fmt; use std::hash::Hash; +use std::{fmt, panic}; + +use self::graph::{print_markframe_trace, DepGraphData, MarkFrame}; pub trait DepContext: Copy { type DepKind: self::DepKind; @@ -53,11 +55,24 @@ pub trait DepContext: Copy { } /// Try to force a dep node to execute and see if it's green. - #[instrument(skip(self), level = "debug")] - fn try_force_from_dep_node(self, dep_node: DepNode) -> bool { + #[inline] + #[instrument(skip(self, graph, frame), level = "debug")] + fn try_force_from_dep_node( + self, + dep_node: DepNode, + graph: &DepGraphData, + frame: Option<&MarkFrame<'_>>, + ) -> bool { let cb = self.dep_kind_info(dep_node.kind); if let Some(f) = cb.force_from_dep_node { - f(self, dep_node); + if let Err(value) = panic::catch_unwind(panic::AssertUnwindSafe(|| { + f(self, dep_node); + })) { + if !value.is::() { + print_markframe_trace(graph, frame); + } + panic::resume_unwind(value) + } true } else { false From 867de8bbb87667a721dd409cefc294b0a1c303a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 9 Mar 2023 08:35:01 +0100 Subject: [PATCH 2/3] Remove `graph` parameter from `try_force_from_dep_node` --- compiler/rustc_query_system/src/dep_graph/graph.rs | 8 +++++--- compiler/rustc_query_system/src/dep_graph/mod.rs | 7 +++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 5604da4696211..0963e8e8bdfc0 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -776,7 +776,7 @@ impl DepGraphData { // We failed to mark it green, so we try to force the query. debug!("trying to force dependency {dep_dep_node:?}"); - if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, data, frame) { + if !qcx.dep_context().try_force_from_dep_node(*dep_dep_node, frame) { // The DepNode could not be forced. debug!("dependency {dep_dep_node:?} could not be forced"); return None; @@ -1416,9 +1416,11 @@ impl DepNodeColorMap { #[inline(never)] #[cold] pub(crate) fn print_markframe_trace( - graph: &DepGraphData, + graph: &DepGraph, frame: Option<&MarkFrame<'_>>, ) { + let data = graph.data.as_ref().unwrap(); + eprintln!("there was a panic while trying to force a dep node"); eprintln!("try_mark_green dep node stack:"); @@ -1426,7 +1428,7 @@ pub(crate) fn print_markframe_trace( let mut current = frame; while let Some(frame) = current { // Do not try to rely on DepNode's Debug implementation, since it may panic. - let node = graph.previous.index_to_node(frame.index); + let node = data.previous.index_to_node(frame.index); eprintln!("#{i} {:?} ({})", node.kind, node.hash); current = frame.parent; i += 1; diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs index f70a913974362..40e7131987fab 100644 --- a/compiler/rustc_query_system/src/dep_graph/mod.rs +++ b/compiler/rustc_query_system/src/dep_graph/mod.rs @@ -20,7 +20,7 @@ use rustc_session::Session; use std::hash::Hash; use std::{fmt, panic}; -use self::graph::{print_markframe_trace, DepGraphData, MarkFrame}; +use self::graph::{print_markframe_trace, MarkFrame}; pub trait DepContext: Copy { type DepKind: self::DepKind; @@ -56,11 +56,10 @@ pub trait DepContext: Copy { /// Try to force a dep node to execute and see if it's green. #[inline] - #[instrument(skip(self, graph, frame), level = "debug")] + #[instrument(skip(self, frame), level = "debug")] fn try_force_from_dep_node( self, dep_node: DepNode, - graph: &DepGraphData, frame: Option<&MarkFrame<'_>>, ) -> bool { let cb = self.dep_kind_info(dep_node.kind); @@ -69,7 +68,7 @@ pub trait DepContext: Copy { f(self, dep_node); })) { if !value.is::() { - print_markframe_trace(graph, frame); + print_markframe_trace(self.dep_graph(), frame); } panic::resume_unwind(value) } From f48ff4a2cf440088ef7f8a806190956ad4d38607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 9 Mar 2023 08:37:51 +0100 Subject: [PATCH 3/3] Use `Debug` for formatting the dep nodes --- compiler/rustc_query_system/src/dep_graph/graph.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 0963e8e8bdfc0..02cb9c452a8c1 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -1427,9 +1427,8 @@ pub(crate) fn print_markframe_trace( let mut i = 0; let mut current = frame; while let Some(frame) = current { - // Do not try to rely on DepNode's Debug implementation, since it may panic. let node = data.previous.index_to_node(frame.index); - eprintln!("#{i} {:?} ({})", node.kind, node.hash); + eprintln!("#{i} {:?}", node); current = frame.parent; i += 1; }