Skip to content

Optimize dep node backtrace and ignore fatal errors #108524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 46 additions & 47 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -54,6 +53,11 @@ impl From<DepNodeIndex> for QueryInvocationId {
}
}

pub struct MarkFrame<'a> {
index: SerializedDepNodeIndex,
parent: Option<&'a MarkFrame<'a>>,
}

#[derive(PartialEq)]
pub enum DepNodeColor {
Red,
Expand Down Expand Up @@ -710,32 +714,26 @@ impl<K: DepKind> DepGraphData<K> {
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<Qcx: QueryContext<DepKind = K>>(
&self,
qcx: Qcx,
parent_dep_node_index: SerializedDepNodeIndex,
dep_node: &DepNode<K>,
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);
Expand Down Expand Up @@ -767,7 +765,8 @@ impl<K: DepKind> DepGraphData<K> {
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",);
Expand All @@ -777,7 +776,7 @@ impl<K: DepKind> DepGraphData<K> {

// 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, frame) {
// The DepNode could not be forced.
debug!("dependency {dep_dep_node:?} could not be forced");
return None;
Expand Down Expand Up @@ -816,13 +815,16 @@ impl<K: DepKind> DepGraphData<K> {
}

/// 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<Qcx: QueryContext<DepKind = K>>(
&self,
qcx: Qcx,
prev_dep_node_index: SerializedDepNodeIndex,
dep_node: &DepNode<K>,
frame: Option<&MarkFrame<'_>>,
) -> Option<DepNodeIndex> {
let frame = MarkFrame { index: prev_dep_node_index, parent: frame };

#[cfg(not(parallel_compiler))]
{
debug_assert!(!self.dep_node_exists(dep_node));
Expand All @@ -837,10 +839,7 @@ impl<K: DepKind> DepGraphData<K> {
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
Expand Down Expand Up @@ -1414,25 +1413,25 @@ impl DepNodeColorMap {
}
}

fn backtrace_printer<'a, K: DepKind>(
sess: &'a rustc_session::Session,
graph: &'a DepGraphData<K>,
node: SerializedDepNodeIndex,
) -> OnDrop<impl Fn() + 'a> {
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);
},
)
#[inline(never)]
#[cold]
pub(crate) fn print_markframe_trace<K: DepKind>(
graph: &DepGraph<K>,
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:");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to use handler.force_print_diagnostic to play nice with the rest of the infra, like print_query_stack does.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

report_ice -> try_print_query_stack -> print_query_stack just prints to stderr like this. It does not rely on the session emitter at all.


let mut i = 0;
let mut current = frame;
while let Some(frame) = current {
let node = data.previous.index_to_node(frame.index);
eprintln!("#{i} {:?}", node);
current = frame.parent;
i += 1;
}

eprintln!("end of try_mark_green dep node stack");
}
22 changes: 18 additions & 4 deletions compiler/rustc_query_system/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, MarkFrame};

pub trait DepContext: Copy {
type DepKind: self::DepKind;
Expand Down Expand Up @@ -53,11 +55,23 @@ 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<Self::DepKind>) -> bool {
#[inline]
#[instrument(skip(self, frame), level = "debug")]
fn try_force_from_dep_node(
self,
dep_node: DepNode<Self::DepKind>,
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::<rustc_errors::FatalErrorMarker>() {
print_markframe_trace(self.dep_graph(), frame);
}
panic::resume_unwind(value)
}
true
} else {
false
Expand Down