From 584a52096e207f7884d57da3d1357ce7f056f1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Tue, 25 Dec 2018 04:36:17 +0100 Subject: [PATCH] Clean up and optimize OpenTask / read_index --- src/librustc/dep_graph/graph.rs | 280 ++++++++++------------- src/librustc/dep_graph/mod.rs | 2 +- src/librustc/ty/context.rs | 10 +- src/librustc/ty/query/plumbing.rs | 2 +- src/librustc_incremental/persist/save.rs | 9 +- 5 files changed, 131 insertions(+), 172 deletions(-) diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index 961638151a2a8..71cacfe370658 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -122,12 +122,7 @@ impl DepGraph { if let Some(..) = self.data { ty::tls::with_context_opt(|icx| { let icx = if let Some(icx) = icx { icx } else { return }; - match *icx.task { - OpenTask::Ignore => { - // ignored - } - _ => panic!("expected an ignore context") - } + assert!(icx.task_deps.is_none(), "expected no task dependency tracking"); }) } } @@ -137,7 +132,7 @@ impl DepGraph { { ty::tls::with_context(|icx| { let icx = ty::tls::ImplicitCtxt { - task: &OpenTask::Ignore, + task_deps: None, ..icx.clone() }; @@ -184,12 +179,15 @@ impl DepGraph { R: HashStable>, { self.with_task_impl(key, cx, arg, false, task, - |key| OpenTask::Regular(Lock::new(RegularOpenTask { - node: key, + |_key| Some(TaskDeps { + #[cfg(debug_assertions)] + node: Some(_key), reads: SmallVec::new(), read_set: Default::default(), - })), - |data, key, fingerprint, task| data.borrow_mut().complete_task(key, task, fingerprint)) + }), + |data, key, fingerprint, task| { + data.borrow_mut().complete_task(key, task.unwrap(), fingerprint) + }) } /// Creates a new dep-graph input with value `input` @@ -206,7 +204,7 @@ impl DepGraph { } self.with_task_impl(key, cx, input, true, identity_fn, - |_| OpenTask::Ignore, + |_| None, |data, key, fingerprint, _| { data.borrow_mut().alloc_node(key, SmallVec::new(), fingerprint) }) @@ -219,18 +217,18 @@ impl DepGraph { arg: A, no_tcx: bool, task: fn(C, A) -> R, - create_task: fn(DepNode) -> OpenTask, + create_task: fn(DepNode) -> Option, finish_task_and_alloc_depnode: fn(&Lock, DepNode, Fingerprint, - OpenTask) -> DepNodeIndex + Option) -> DepNodeIndex ) -> (R, DepNodeIndex) where C: DepGraphSafe + StableHashingContextProvider<'gcx>, R: HashStable>, { if let Some(ref data) = self.data { - let open_task = create_task(key); + let task_deps = create_task(key).map(|deps| Lock::new(deps)); // In incremental mode, hash the result of the task. We don't // do anything with the hash yet, but we are computing it @@ -248,7 +246,7 @@ impl DepGraph { } else { ty::tls::with_context(|icx| { let icx = ty::tls::ImplicitCtxt { - task: &open_task, + task_deps: task_deps.as_ref(), ..icx.clone() }; @@ -271,7 +269,7 @@ impl DepGraph { &data.current, key, current_fingerprint, - open_task + task_deps.map(|lock| lock.into_inner()), ); // Determine the color of the new DepNode. @@ -304,15 +302,17 @@ impl DepGraph { where OP: FnOnce() -> R { if let Some(ref data) = self.data { - let (result, open_task) = ty::tls::with_context(|icx| { - let task = OpenTask::Anon(Lock::new(AnonOpenTask { + let (result, task_deps) = ty::tls::with_context(|icx| { + let task_deps = Lock::new(TaskDeps { + #[cfg(debug_assertions)] + node: None, reads: SmallVec::new(), read_set: Default::default(), - })); + }); let r = { let icx = ty::tls::ImplicitCtxt { - task: &task, + task_deps: Some(&task_deps), ..icx.clone() }; @@ -321,11 +321,11 @@ impl DepGraph { }) }; - (r, task) + (r, task_deps.into_inner()) }); let dep_node_index = data.current .borrow_mut() - .pop_anon_task(dep_kind, open_task); + .complete_anon_task(dep_kind, task_deps); (result, dep_node_index) } else { (op(), DepNodeIndex::INVALID) @@ -344,18 +344,23 @@ impl DepGraph { R: HashStable>, { self.with_task_impl(key, cx, arg, false, task, - |key| OpenTask::EvalAlways { node: key }, - |data, key, fingerprint, task| { - data.borrow_mut().complete_eval_always_task(key, task, fingerprint) + |_| None, + |data, key, fingerprint, _| { + let mut current = data.borrow_mut(); + let krate_idx = current.node_to_node_index[ + &DepNode::new_no_params(DepKind::Krate) + ]; + current.alloc_node(key, smallvec![krate_idx], fingerprint) }) } #[inline] pub fn read(&self, v: DepNode) { if let Some(ref data) = self.data { - let mut current = data.current.borrow_mut(); + let current = data.current.borrow_mut(); if let Some(&dep_node_index) = current.node_to_node_index.get(&v) { - current.read_index(dep_node_index); + std::mem::drop(current); + data.read_index(dep_node_index); } else { bug!("DepKind {:?} should be pre-allocated but isn't.", v.kind) } @@ -365,7 +370,7 @@ impl DepGraph { #[inline] pub fn read_index(&self, dep_node_index: DepNodeIndex) { if let Some(ref data) = self.data { - data.current.borrow_mut().read_index(dep_node_index); + data.read_index(dep_node_index); } } @@ -446,10 +451,15 @@ impl DepGraph { .cloned() } - pub fn edge_deduplication_data(&self) -> (u64, u64) { - let current_dep_graph = self.data.as_ref().unwrap().current.borrow(); + pub fn edge_deduplication_data(&self) -> Option<(u64, u64)> { + if cfg!(debug_assertions) { + let current_dep_graph = self.data.as_ref().unwrap().current.borrow(); - (current_dep_graph.total_read_count, current_dep_graph.total_duplicate_read_count) + Some((current_dep_graph.total_read_count, + current_dep_graph.total_duplicate_read_count)) + } else { + None + } } pub fn serialize(&self) -> SerializedDepGraph { @@ -827,6 +837,7 @@ struct DepNodeData { pub(super) struct CurrentDepGraph { data: IndexVec, node_to_node_index: FxHashMap, + #[allow(dead_code)] forbidden_edge: Option, // Anonymous DepNodes are nodes the ID of which we compute from the list of @@ -890,134 +901,60 @@ impl CurrentDepGraph { fn complete_task( &mut self, - key: DepNode, - task: OpenTask, + node: DepNode, + task_deps: TaskDeps, fingerprint: Fingerprint ) -> DepNodeIndex { - if let OpenTask::Regular(task) = task { - let RegularOpenTask { - node, - read_set: _, - reads - } = task.into_inner(); - assert_eq!(node, key); - - // If this is an input node, we expect that it either has no - // dependencies, or that it just depends on DepKind::CrateMetadata - // or DepKind::Krate. This happens for some "thin wrapper queries" - // like `crate_disambiguator` which sometimes have zero deps (for - // when called for LOCAL_CRATE) or they depend on a CrateMetadata - // node. - if cfg!(debug_assertions) { - if node.kind.is_input() && reads.len() > 0 && - // FIXME(mw): Special case for DefSpan until Spans are handled - // better in general. - node.kind != DepKind::DefSpan && - reads.iter().any(|&i| { - !(self.data[i].node.kind == DepKind::CrateMetadata || - self.data[i].node.kind == DepKind::Krate) - }) - { - bug!("Input node {:?} with unexpected reads: {:?}", - node, - reads.iter().map(|&i| self.data[i].node).collect::>()) - } + // If this is an input node, we expect that it either has no + // dependencies, or that it just depends on DepKind::CrateMetadata + // or DepKind::Krate. This happens for some "thin wrapper queries" + // like `crate_disambiguator` which sometimes have zero deps (for + // when called for LOCAL_CRATE) or they depend on a CrateMetadata + // node. + if cfg!(debug_assertions) { + if node.kind.is_input() && task_deps.reads.len() > 0 && + // FIXME(mw): Special case for DefSpan until Spans are handled + // better in general. + node.kind != DepKind::DefSpan && + task_deps.reads.iter().any(|&i| { + !(self.data[i].node.kind == DepKind::CrateMetadata || + self.data[i].node.kind == DepKind::Krate) + }) + { + bug!("Input node {:?} with unexpected reads: {:?}", + node, + task_deps.reads.iter().map(|&i| self.data[i].node).collect::>()) } - - self.alloc_node(node, reads, fingerprint) - } else { - bug!("complete_task() - Expected regular task to be popped") } - } - - fn pop_anon_task(&mut self, kind: DepKind, task: OpenTask) -> DepNodeIndex { - if let OpenTask::Anon(task) = task { - let AnonOpenTask { - read_set: _, - reads - } = task.into_inner(); - debug_assert!(!kind.is_input()); - - let mut fingerprint = self.anon_id_seed; - let mut hasher = StableHasher::new(); - for &read in reads.iter() { - let read_dep_node = self.data[read].node; + self.alloc_node(node, task_deps.reads, fingerprint) + } - ::std::mem::discriminant(&read_dep_node.kind).hash(&mut hasher); + fn complete_anon_task(&mut self, kind: DepKind, task_deps: TaskDeps) -> DepNodeIndex { + debug_assert!(!kind.is_input()); - // Fingerprint::combine() is faster than sending Fingerprint - // through the StableHasher (at least as long as StableHasher - // is so slow). - fingerprint = fingerprint.combine(read_dep_node.hash); - } + let mut fingerprint = self.anon_id_seed; + let mut hasher = StableHasher::new(); - fingerprint = fingerprint.combine(hasher.finish()); + for &read in task_deps.reads.iter() { + let read_dep_node = self.data[read].node; - let target_dep_node = DepNode { - kind, - hash: fingerprint, - }; + ::std::mem::discriminant(&read_dep_node.kind).hash(&mut hasher); - self.intern_node(target_dep_node, reads, Fingerprint::ZERO).0 - } else { - bug!("pop_anon_task() - Expected anonymous task to be popped") + // Fingerprint::combine() is faster than sending Fingerprint + // through the StableHasher (at least as long as StableHasher + // is so slow). + fingerprint = fingerprint.combine(read_dep_node.hash); } - } - fn complete_eval_always_task( - &mut self, - key: DepNode, - task: OpenTask, - fingerprint: Fingerprint - ) -> DepNodeIndex { - if let OpenTask::EvalAlways { - node, - } = task { - debug_assert_eq!(node, key); - let krate_idx = self.node_to_node_index[&DepNode::new_no_params(DepKind::Krate)]; - self.alloc_node(node, smallvec![krate_idx], fingerprint) - } else { - bug!("complete_eval_always_task() - Expected eval always task to be popped"); - } - } + fingerprint = fingerprint.combine(hasher.finish()); - fn read_index(&mut self, source: DepNodeIndex) { - ty::tls::with_context_opt(|icx| { - let icx = if let Some(icx) = icx { icx } else { return }; - match *icx.task { - OpenTask::Regular(ref task) => { - let mut task = task.lock(); - self.total_read_count += 1; - if task.read_set.insert(source) { - task.reads.push(source); - - if cfg!(debug_assertions) { - if let Some(ref forbidden_edge) = self.forbidden_edge { - let target = &task.node; - let source = self.data[source].node; - if forbidden_edge.test(&source, &target) { - bug!("forbidden edge {:?} -> {:?} created", - source, - target) - } - } - } - } else { - self.total_duplicate_read_count += 1; - } - } - OpenTask::Anon(ref task) => { - let mut task = task.lock(); - if task.read_set.insert(source) { - task.reads.push(source); - } - } - OpenTask::Ignore | OpenTask::EvalAlways { .. } => { - // ignore - } - } - }) + let target_dep_node = DepNode { + kind, + hash: fingerprint, + }; + + self.intern_node(target_dep_node, task_deps.reads, Fingerprint::ZERO).0 } fn alloc_node( @@ -1054,26 +991,47 @@ impl CurrentDepGraph { } } -pub struct RegularOpenTask { - node: DepNode, - reads: SmallVec<[DepNodeIndex; 8]>, - read_set: FxHashSet, +impl DepGraphData { + fn read_index(&self, source: DepNodeIndex) { + ty::tls::with_context_opt(|icx| { + let icx = if let Some(icx) = icx { icx } else { return }; + if let Some(task_deps) = icx.task_deps { + let mut task_deps = task_deps.lock(); + if cfg!(debug_assertions) { + self.current.lock().total_read_count += 1; + } + if task_deps.read_set.insert(source) { + task_deps.reads.push(source); + + #[cfg(debug_assertions)] + { + if let Some(target) = task_deps.node { + let graph = self.current.lock(); + if let Some(ref forbidden_edge) = graph.forbidden_edge { + let source = graph.data[source].node; + if forbidden_edge.test(&source, &target) { + bug!("forbidden edge {:?} -> {:?} created", + source, + target) + } + } + } + } + } else if cfg!(debug_assertions) { + self.current.lock().total_duplicate_read_count += 1; + } + } + }) + } } -pub struct AnonOpenTask { +pub struct TaskDeps { + #[cfg(debug_assertions)] + node: Option, reads: SmallVec<[DepNodeIndex; 8]>, read_set: FxHashSet, } -pub enum OpenTask { - Regular(Lock), - Anon(Lock), - Ignore, - EvalAlways { - node: DepNode, - }, -} - // A data structure that stores Option values as a contiguous // array, using one u32 per entry. struct DepNodeColorMap { diff --git a/src/librustc/dep_graph/mod.rs b/src/librustc/dep_graph/mod.rs index d153b7435c9b8..022caabcbf3a1 100644 --- a/src/librustc/dep_graph/mod.rs +++ b/src/librustc/dep_graph/mod.rs @@ -10,7 +10,7 @@ pub mod cgu_reuse_tracker; pub use self::dep_tracking_map::{DepTrackingMap, DepTrackingMapConfig}; pub use self::dep_node::{DepNode, DepKind, DepConstructor, WorkProductId, label_strs}; -pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, OpenTask}; +pub use self::graph::{DepGraph, WorkProduct, DepNodeIndex, DepNodeColor, TaskDeps}; pub use self::graph::WorkProductFileKind; pub use self::prev::PreviousDepGraph; pub use self::query::DepGraphQuery; diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index ea69d466ba6c5..7040f77135d03 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1632,7 +1632,7 @@ impl<'gcx> GlobalCtxt<'gcx> { tcx, query: icx.query.clone(), layout_depth: icx.layout_depth, - task: icx.task, + task_deps: icx.task_deps, }; ty::tls::enter_context(&new_icx, |_| { f(tcx) @@ -1889,7 +1889,7 @@ pub mod tls { use errors::{Diagnostic, TRACK_DIAGNOSTICS}; use rustc_data_structures::OnDrop; use rustc_data_structures::sync::{self, Lrc, Lock}; - use dep_graph::OpenTask; + use dep_graph::TaskDeps; #[cfg(not(parallel_queries))] use std::cell::Cell; @@ -1917,7 +1917,7 @@ pub mod tls { /// The current dep graph task. This is used to add dependencies to queries /// when executing them - pub task: &'a OpenTask, + pub task_deps: Option<&'a Lock>, } /// Sets Rayon's thread local variable which is preserved for Rayon jobs @@ -2046,7 +2046,7 @@ pub mod tls { tcx, query: None, layout_depth: 0, - task: &OpenTask::Ignore, + task_deps: None, }; enter_context(&icx, |_| { f(tcx) @@ -2076,7 +2076,7 @@ pub mod tls { query: None, tcx, layout_depth: 0, - task: &OpenTask::Ignore, + task_deps: None, }; enter_context(&icx, |_| f(tcx)) } diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 6887f480f72e0..88be99726df2b 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -212,7 +212,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> { tcx: tcx.global_tcx(), query: Some(self.job.clone()), layout_depth: current_icx.layout_depth, - task: current_icx.task, + task_deps: current_icx.task_deps, }; // Use the ImplicitCtxt while we execute the query diff --git a/src/librustc_incremental/persist/save.rs b/src/librustc_incremental/persist/save.rs index 5131b63115ae5..6a7553b388297 100644 --- a/src/librustc_incremental/persist/save.rs +++ b/src/librustc_incremental/persist/save.rs @@ -149,8 +149,6 @@ fn encode_dep_graph(tcx: TyCtxt, let total_node_count = serialized_graph.nodes.len(); let total_edge_count = serialized_graph.edge_list_data.len(); - let (total_edge_reads, total_duplicate_edge_reads) = - tcx.dep_graph.edge_deduplication_data(); let mut counts: FxHashMap<_, Stat> = FxHashMap::default(); @@ -188,8 +186,11 @@ fn encode_dep_graph(tcx: TyCtxt, println!("[incremental]"); println!("[incremental] Total Node Count: {}", total_node_count); println!("[incremental] Total Edge Count: {}", total_edge_count); - println!("[incremental] Total Edge Reads: {}", total_edge_reads); - println!("[incremental] Total Duplicate Edge Reads: {}", total_duplicate_edge_reads); + if let Some((total_edge_reads, + total_duplicate_edge_reads)) = tcx.dep_graph.edge_deduplication_data() { + println!("[incremental] Total Edge Reads: {}", total_edge_reads); + println!("[incremental] Total Duplicate Edge Reads: {}", total_duplicate_edge_reads); + } println!("[incremental]"); println!("[incremental] {:<36}| {:<17}| {:<12}| {:<17}|", "Node Kind",