Skip to content

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

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

Merged
merged 2 commits into from
Jan 18, 2019
+117 −113
Merged

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

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
Original file line number Diff line number Diff line change
@@ -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;
@@ -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
@@ -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(),
})),
}
@@ -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)
@@ -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.
@@ -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));
@@ -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));
@@ -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;
@@ -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
@@ -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
}
@@ -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)
}

@@ -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) {
@@ -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;
@@ -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(
@@ -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
Original file line number Diff line number Diff line change
@@ -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]
49 changes: 8 additions & 41 deletions src/librustc/ty/query/plumbing.rs
View file
Original file line number Diff line number Diff line change
@@ -2,7 +2,7 @@
//! that generate the actual methods on tcx which find and execute the
//! provider, manage the caches, and so forth.
use dep_graph::{DepNodeIndex, DepNode, DepKind, DepNodeColor};
use dep_graph::{DepNodeIndex, DepNode, DepKind, SerializedDepNodeIndex};
use errors::DiagnosticBuilder;
use errors::Level;
use errors::Diagnostic;
@@ -335,40 +335,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
eprintln!("end of query stack");
}

/// 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(super) fn try_mark_green_and_read(self, dep_node: &DepNode) -> Option<DepNodeIndex> {
match self.dep_graph.node_color(dep_node) {
Some(DepNodeColor::Green(dep_node_index)) => {
self.dep_graph.read_index(dep_node_index);
Some(dep_node_index)
}
Some(DepNodeColor::Red) => {
None
}
None => {
// try_mark_green (called below) will panic when full incremental
// compilation is disabled. If that's the case, we can't try to mark nodes
// as green anyway, so we can safely return None here.
if !self.dep_graph.is_fully_enabled() {
return None;
}
match self.dep_graph.try_mark_green(self.global_tcx(), &dep_node) {
Some(dep_node_index) => {
debug_assert!(self.dep_graph.is_green(&dep_node));
self.dep_graph.read_index(dep_node_index);
Some(dep_node_index)
}
None => {
None
}
}
}
}
}

#[inline(never)]
fn try_get_with<Q: QueryDescription<'gcx>>(
self,
@@ -435,10 +401,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}

if !dep_node.kind.is_input() {
if let Some(dep_node_index) = self.try_mark_green_and_read(&dep_node) {
if let Some((prev_dep_node_index,
dep_node_index)) = self.dep_graph.try_mark_green_and_read(self,
&dep_node) {
return Ok(self.load_from_disk_and_cache_in_memory::<Q>(
key,
job,
prev_dep_node_index,
dep_node_index,
&dep_node
))
@@ -454,6 +423,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self,
key: Q::Key,
job: JobOwner<'a, 'gcx, Q>,
prev_dep_node_index: SerializedDepNodeIndex,
dep_node_index: DepNodeIndex,
dep_node: &DepNode
) -> Q::Value
@@ -466,10 +436,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// First we try to load the result from the on-disk cache
let result = if Q::cache_on_disk(key.clone()) &&
self.sess.opts.debugging_opts.incremental_queries {
let prev_dep_node_index =
self.dep_graph.prev_dep_node_index_of(dep_node);
let result = Q::try_load_from_disk(self.global_tcx(),
prev_dep_node_index);
let result = Q::try_load_from_disk(self.global_tcx(), prev_dep_node_index);

// We always expect to find a cached result for things that
// can be forced from DepNode.
@@ -624,7 +591,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// Ensuring an "input" or anonymous query makes no sense
assert!(!dep_node.kind.is_anon());
assert!(!dep_node.kind.is_input());
if self.try_mark_green_and_read(&dep_node).is_none() {
if self.dep_graph.try_mark_green_and_read(self, &dep_node).is_none() {
// A None return from `try_mark_green_and_read` means that this is either
// a new dep node or that the dep node has already been marked red.
// Either way, we can't call `dep_graph.read()` as we don't have the
6 changes: 5 additions & 1 deletion src/librustc_data_structures/sync.rs
View file
Original file line number Diff line number Diff line change
@@ -70,6 +70,7 @@ cfg_if! {
pub struct Atomic<T: Copy>(Cell<T>);

impl<T: Copy> Atomic<T> {
#[inline]
pub fn new(v: T) -> Self {
Atomic(Cell::new(v))
}
@@ -80,10 +81,12 @@ cfg_if! {
self.0.into_inner()
}

#[inline]
pub fn load(&self, _: Ordering) -> T {
self.0.get()
}

#[inline]
pub fn store(&self, val: T, _: Ordering) {
self.0.set(val)
}
@@ -118,6 +121,7 @@ cfg_if! {

pub type AtomicUsize = Atomic<usize>;
pub type AtomicBool = Atomic<bool>;
pub type AtomicU32 = Atomic<u32>;
pub type AtomicU64 = Atomic<u64>;

pub use self::serial_join as join;
@@ -223,7 +227,7 @@ cfg_if! {
pub use parking_lot::MutexGuard as LockGuard;
pub use parking_lot::MappedMutexGuard as MappedLockGuard;

pub use std::sync::atomic::{AtomicBool, AtomicUsize, AtomicU64};
pub use std::sync::atomic::{AtomicBool, AtomicUsize, AtomicU32, AtomicU64};

pub use std::sync::Arc as Lrc;
pub use std::sync::Weak as Weak;