Skip to content
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.

Sign up for GitHub

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

Optimize try_mark_green and eliminate the lock on dep node colors #57065

Merged
merged 2 commits into from
Jan 18, 2019
Merged

Optimize try_mark_green and eliminate the lock on dep node colors #57065

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8ef7413
Optimize try_mark_green and eliminate the lock on dep node colors
Zoxc Dec 22, 2018
1313678
Address comments
Zoxc Jan 15, 2019
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
162 changes: 99 additions & 63 deletions src/librustc/dep_graph/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use smallvec::SmallVec;
use rustc_data_structures::sync::{Lrc, Lock};
use rustc_data_structures::sync::{Lrc, Lock, AtomicU32, Ordering};
use std::env;
use std::hash::Hash;
use std::collections::hash_map::Entry;
Expand Down Expand Up @@ -58,7 +58,7 @@ struct DepGraphData {
/// nodes and edges as well as all fingerprints of nodes that have them.
previous: PreviousDepGraph,

colors: Lock<DepNodeColorMap>,
colors: DepNodeColorMap,

/// When we load, there may be `.o` files, cached mir, or other such
/// things available to us. If we find that they are not dirty, we
Expand All @@ -84,7 +84,7 @@ impl DepGraph {
dep_node_debug: Default::default(),
current: Lock::new(CurrentDepGraph::new(prev_graph_node_count)),
previous: prev_graph,
colors: Lock::new(DepNodeColorMap::new(prev_graph_node_count)),
colors: DepNodeColorMap::new(prev_graph_node_count),
loaded_from_cache: Default::default(),
})),
}
Expand Down Expand Up @@ -282,12 +282,11 @@ impl DepGraph {
DepNodeColor::Red
};

let mut colors = data.colors.borrow_mut();
debug_assert!(colors.get(prev_index).is_none(),
debug_assert!(data.colors.get(prev_index).is_none(),
"DepGraph::with_task() - Duplicate DepNodeColor \
insertion for {:?}", key);

colors.insert(prev_index, color);
data.colors.insert(prev_index, color);
}

(result, dep_node_index)
Expand Down Expand Up @@ -502,7 +501,7 @@ impl DepGraph {
pub fn node_color(&self, dep_node: &DepNode) -> Option<DepNodeColor> {
if let Some(ref data) = self.data {
if let Some(prev_index) = data.previous.node_to_index_opt(dep_node) {
return data.colors.borrow().get(prev_index)
return data.colors.get(prev_index)
} else {
// This is a node that did not exist in the previous compilation
// session, so we consider it to be red.
Expand All @@ -513,56 +512,89 @@ impl DepGraph {
None
}

pub fn try_mark_green<'tcx>(&self,
tcx: TyCtxt<'_, 'tcx, 'tcx>,
dep_node: &DepNode)
-> Option<DepNodeIndex> {
debug!("try_mark_green({:?}) - BEGIN", dep_node);
let data = self.data.as_ref().unwrap();
/// Try to read a node index for the node dep_node.
/// A node will have an index, when it's already been marked green, or when we can mark it
/// green. This function will mark the current task as a reader of the specified node, when
/// a node index can be found for that node.
pub fn try_mark_green_and_read(
&self,
tcx: TyCtxt<'_, '_, '_>,
dep_node: &DepNode
) -> Option<(SerializedDepNodeIndex, DepNodeIndex)> {
self.try_mark_green(tcx, dep_node).map(|(prev_index, dep_node_index)| {
debug_assert!(self.is_green(&dep_node));
self.read_index(dep_node_index);
(prev_index, dep_node_index)
})
}

#[cfg(not(parallel_queries))]
debug_assert!(!data.current.borrow().node_to_node_index.contains_key(dep_node));

if dep_node.kind.is_input() {
// We should only hit try_mark_green() for inputs that do not exist
// anymore in the current compilation session. Existing inputs are
// eagerly marked as either red/green before any queries are
// executed.
debug_assert!(dep_node.extract_def_id(tcx).is_none());
debug!("try_mark_green({:?}) - END - DepNode is deleted input", dep_node);
return None;
}
pub fn try_mark_green(
&self,
tcx: TyCtxt<'_, '_, '_>,
dep_node: &DepNode
) -> Option<(SerializedDepNodeIndex, DepNodeIndex)> {
debug_assert!(!dep_node.kind.is_input());

// Return None if the dep graph is disabled
let data = self.data.as_ref()?;

// Return None if the dep node didn't exist in the previous session
let prev_index = data.previous.node_to_index_opt(dep_node)?;

let (prev_deps, prev_dep_node_index) = match data.previous.edges_from(dep_node) {
Some(prev) => {
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.
prev
}
None => {
// This DepNode did not exist in the previous compilation session,
// so we cannot mark it as green.
debug!("try_mark_green({:?}) - END - DepNode does not exist in \
current compilation session anymore", dep_node);
return None
self.try_mark_previous_green(
tcx.global_tcx(),
data,
prev_index,
&dep_node
).map(|dep_node_index| {
(prev_index, dep_node_index)
})
}
};
}
}

/// Try to mark a dep-node which existed in the previous compilation session as green
fn try_mark_previous_green<'tcx>(
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a doc comment here? Something like "Try to mark a dep-node green for which we already know that it existed in the previous compilation session".

&self,
tcx: TyCtxt<'_, 'tcx, 'tcx>,
data: &DepGraphData,
prev_dep_node_index: SerializedDepNodeIndex,
dep_node: &DepNode
) -> Option<DepNodeIndex> {
debug!("try_mark_previous_green({:?}) - BEGIN", dep_node);

debug_assert!(data.colors.borrow().get(prev_dep_node_index).is_none());
#[cfg(not(parallel_queries))]
{
debug_assert!(!data.current.borrow().node_to_node_index.contains_key(dep_node));
debug_assert!(data.colors.get(prev_dep_node_index).is_none());
}

// We never try to mark inputs as green
debug_assert!(!dep_node.kind.is_input());

debug_assert_eq!(data.previous.index_to_node(prev_dep_node_index), *dep_node);

let prev_deps = data.previous.edge_targets_from(prev_dep_node_index);

let mut current_deps = SmallVec::new();

for &dep_dep_node_index in prev_deps {
let dep_dep_node_color = data.colors.borrow().get(dep_dep_node_index);
let dep_dep_node_color = data.colors.get(dep_dep_node_index);

match dep_dep_node_color {
Some(DepNodeColor::Green(node_index)) => {
// This dependency has been marked as green before, we are
// still fine and can continue with checking the other
// dependencies.
debug!("try_mark_green({:?}) --- found dependency {:?} to \
debug!("try_mark_previous_green({:?}) --- found dependency {:?} to \
be immediately green",
dep_node,
data.previous.index_to_node(dep_dep_node_index));
Expand All @@ -573,7 +605,7 @@ impl DepGraph {
// compared to the previous compilation session. We cannot
// mark the DepNode as green and also don't need to bother
// with checking any of the other dependencies.
debug!("try_mark_green({:?}) - END - dependency {:?} was \
debug!("try_mark_previous_green({:?}) - END - dependency {:?} was \
immediately red",
dep_node,
data.previous.index_to_node(dep_dep_node_index));
Expand All @@ -585,12 +617,18 @@ impl DepGraph {
// We don't know the state of this dependency. If it isn't
// an input node, let's try to mark it green recursively.
if !dep_dep_node.kind.is_input() {
debug!("try_mark_green({:?}) --- state of dependency {:?} \
debug!("try_mark_previous_green({:?}) --- state of dependency {:?} \
is unknown, trying to mark it green", dep_node,
dep_dep_node);

if let Some(node_index) = self.try_mark_green(tcx, dep_dep_node) {
debug!("try_mark_green({:?}) --- managed to MARK \
let node_index = self.try_mark_previous_green(
tcx,
data,
dep_dep_node_index,
dep_dep_node
);
if let Some(node_index) = node_index {
debug!("try_mark_previous_green({:?}) --- managed to MARK \
dependency {:?} as green", dep_node, dep_dep_node);
current_deps.push(node_index);
continue;
Expand Down Expand Up @@ -620,28 +658,28 @@ impl DepGraph {
}

// We failed to mark it green, so we try to force the query.
debug!("try_mark_green({:?}) --- trying to force \
debug!("try_mark_previous_green({:?}) --- trying to force \
dependency {:?}", dep_node, dep_dep_node);
if ::ty::query::force_from_dep_node(tcx, dep_dep_node) {
let dep_dep_node_color = data.colors.borrow().get(dep_dep_node_index);
let dep_dep_node_color = data.colors.get(dep_dep_node_index);

match dep_dep_node_color {
Some(DepNodeColor::Green(node_index)) => {
debug!("try_mark_green({:?}) --- managed to \
debug!("try_mark_previous_green({:?}) --- managed to \
FORCE dependency {:?} to green",
dep_node, dep_dep_node);
current_deps.push(node_index);
}
Some(DepNodeColor::Red) => {
debug!("try_mark_green({:?}) - END - \
debug!("try_mark_previous_green({:?}) - END - \
dependency {:?} was red after forcing",
dep_node,
dep_dep_node);
return None
}
None => {
if !tcx.sess.has_errors() {
bug!("try_mark_green() - Forcing the DepNode \
bug!("try_mark_previous_green() - Forcing the DepNode \
should have set its color")
} else {
// If the query we just forced has resulted
Expand All @@ -653,7 +691,7 @@ impl DepGraph {
}
} else {
// The DepNode could not be forced.
debug!("try_mark_green({:?}) - END - dependency {:?} \
debug!("try_mark_previous_green({:?}) - END - dependency {:?} \
could not be forced", dep_node, dep_dep_node);
return None
}
Expand Down Expand Up @@ -705,16 +743,15 @@ impl DepGraph {
}

// ... and finally storing a "Green" entry in the color map.
let mut colors = data.colors.borrow_mut();
// Multiple threads can all write the same color here
#[cfg(not(parallel_queries))]
debug_assert!(colors.get(prev_dep_node_index).is_none(),
"DepGraph::try_mark_green() - Duplicate DepNodeColor \
debug_assert!(data.colors.get(prev_dep_node_index).is_none(),
"DepGraph::try_mark_previous_green() - Duplicate DepNodeColor \
insertion for {:?}", dep_node);

colors.insert(prev_dep_node_index, DepNodeColor::Green(dep_node_index));
data.colors.insert(prev_dep_node_index, DepNodeColor::Green(dep_node_index));

debug!("try_mark_green({:?}) - END - successfully marked as green", dep_node);
debug!("try_mark_previous_green({:?}) - END - successfully marked as green", dep_node);
Some(dep_node_index)
}

Expand All @@ -735,9 +772,8 @@ impl DepGraph {
pub fn exec_cache_promotions<'a, 'tcx>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) {
let green_nodes: Vec<DepNode> = {
let data = self.data.as_ref().unwrap();
let colors = data.colors.borrow();
colors.values.indices().filter_map(|prev_index| {
match colors.get(prev_index) {
data.colors.values.indices().filter_map(|prev_index| {
match data.colors.get(prev_index) {
Some(DepNodeColor::Green(_)) => {
let dep_node = data.previous.index_to_node(prev_index);
if dep_node.cache_on_disk(tcx) {
Expand Down Expand Up @@ -1035,7 +1071,7 @@ pub struct TaskDeps {
// A data structure that stores Option<DepNodeColor> values as a contiguous
// array, using one u32 per entry.
struct DepNodeColorMap {
values: IndexVec<SerializedDepNodeIndex, u32>,
values: IndexVec<SerializedDepNodeIndex, AtomicU32>,
}

const COMPRESSED_NONE: u32 = 0;
Expand All @@ -1045,12 +1081,12 @@ const COMPRESSED_FIRST_GREEN: u32 = 2;
impl DepNodeColorMap {
fn new(size: usize) -> DepNodeColorMap {
DepNodeColorMap {
values: IndexVec::from_elem_n(COMPRESSED_NONE, size)
values: (0..size).map(|_| AtomicU32::new(COMPRESSED_NONE)).collect(),
}
}

fn get(&self, index: SerializedDepNodeIndex) -> Option<DepNodeColor> {
match self.values[index] {
match self.values[index].load(Ordering::Acquire) {
COMPRESSED_NONE => None,
COMPRESSED_RED => Some(DepNodeColor::Red),
value => Some(DepNodeColor::Green(DepNodeIndex::from_u32(
Expand All @@ -1059,10 +1095,10 @@ impl DepNodeColorMap {
}
}

fn insert(&mut self, index: SerializedDepNodeIndex, color: DepNodeColor) {
self.values[index] = match color {
fn insert(&self, index: SerializedDepNodeIndex, color: DepNodeColor) {
self.values[index].store(match color {
DepNodeColor::Red => COMPRESSED_RED,
DepNodeColor::Green(index) => index.as_u32() + COMPRESSED_FIRST_GREEN,
}
}, Ordering::Release)
}
}
13 changes: 5 additions & 8 deletions src/librustc/dep_graph/prev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@ impl PreviousDepGraph {
}

#[inline]
pub fn edges_from(&self,
dep_node: &DepNode)
-> Option<(&[SerializedDepNodeIndex], SerializedDepNodeIndex)> {
self.index
.get(dep_node)
.map(|&node_index| {
(self.data.edge_targets_from(node_index), node_index)
})
pub fn edge_targets_from(
&self,
dep_node_index: SerializedDepNodeIndex
) -> &[SerializedDepNodeIndex] {
self.data.edge_targets_from(dep_node_index)
}

#[inline]
Expand Down
Loading