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.

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

Remove interior mutability from TraitDef by turning fields into queries #41911

Merged
merged 10 commits into from
May 18, 2017
Merged
3 changes: 3 additions & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ pub enum DepNode<D: Clone + Debug> {
// than changes in the impl body.
TraitImpls(D),

AllLocalTraitImpls,

// Nodes representing caches. To properly handle a true cache, we
// don't use a DepTrackingMap, but rather we push a task node.
// Otherwise the write into the map would be incorrectly
Expand Down Expand Up @@ -263,6 +265,7 @@ impl<D: Clone + Debug> DepNode<D> {
ConstEval(ref d) => op(d).map(ConstEval),
SymbolName(ref d) => op(d).map(SymbolName),
TraitImpls(ref d) => op(d).map(TraitImpls),
AllLocalTraitImpls => Some(AllLocalTraitImpls),
TraitItems(ref d) => op(d).map(TraitItems),
ReprHints(ref d) => op(d).map(ReprHints),
TraitSelect { ref trait_def_id, ref input_def_id } => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,15 +497,15 @@ impl<'hir> Map<'hir> {
}

pub fn trait_impls(&self, trait_did: DefId) -> &'hir [NodeId] {
self.dep_graph.read(DepNode::TraitImpls(trait_did));
self.dep_graph.read(DepNode::AllLocalTraitImpls);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why you converted this into a single dep-node, rather than keeping it per-trait. It seems like this is losing quite a bit of precision, no?

(That is, if any trait adds an impl, we will consider them all to have changed.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly for ease of implementation and symmetry with the situation in metadata. DepNode::TraitImpls has been re-purposed to represent local and remote impls and, with red-green, will take care of stopping invalidation short.

Without red-green though you are right, it's losing precision. Want me to make it per-item?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not a big deal.


// NB: intentionally bypass `self.forest.krate()` so that we
// do not trigger a read of the whole krate here
self.forest.krate.trait_impls.get(&trait_did).map_or(&[], |xs| &xs[..])
}

pub fn trait_default_impl(&self, trait_did: DefId) -> Option<NodeId> {
self.dep_graph.read(DepNode::TraitImpls(trait_did));
self.dep_graph.read(DepNode::AllLocalTraitImpls);

// NB: intentionally bypass `self.forest.krate()` so that we
// do not trigger a read of the whole krate here
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/ich/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,11 @@ impl stable_hasher::StableHasherResult for Fingerprint {
fingerprint
}
}

impl<CTX> stable_hasher::HashStable<CTX> for Fingerprint {
fn hash_stable<W: stable_hasher::StableHasherResult>(&self,
_: &mut CTX,
hasher: &mut stable_hasher::StableHasher<W>) {
::std::hash::Hash::hash(&self.0, hasher);
}
}
24 changes: 23 additions & 1 deletion src/librustc/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use ty;
use util::nodemap::NodeMap;

use std::hash as std_hash;
use std::collections::{HashMap, HashSet};
use std::collections::{HashMap, HashSet, BTreeMap};

use syntax::ast;
use syntax::attr;
Expand Down Expand Up @@ -348,3 +348,25 @@ pub fn hash_stable_nodemap<'a, 'tcx, V, W>(hcx: &mut StableHashingContext<'a, 't
hcx.tcx.hir.definitions().node_to_hir_id(*node_id).local_id
});
}


pub fn hash_stable_btreemap<'a, 'tcx, K, V, SK, F, W>(hcx: &mut StableHashingContext<'a, 'tcx>,
hasher: &mut StableHasher<W>,
map: &BTreeMap<K, V>,
extract_stable_key: F)
where K: Eq + Ord,
V: HashStable<StableHashingContext<'a, 'tcx>>,
SK: HashStable<StableHashingContext<'a, 'tcx>> + Ord + Clone,
F: Fn(&mut StableHashingContext<'a, 'tcx>, &K) -> SK,
W: StableHasherResult,
{
let mut keys: Vec<_> = map.keys()
.map(|k| (extract_stable_key(hcx, k), k))
.collect();
keys.sort_unstable_by_key(|&(ref stable_key, _)| stable_key.clone());
keys.len().hash_stable(hcx, hasher);
for (stable_key, key) in keys {
stable_key.hash_stable(hcx, hasher);
map[key].hash_stable(hcx, hasher);
}
}
3 changes: 2 additions & 1 deletion src/librustc/ich/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
pub use self::fingerprint::Fingerprint;
pub use self::caching_codemap_view::CachingCodemapView;
pub use self::hcx::{StableHashingContext, NodeIdHashingMode, hash_stable_hashmap,
hash_stable_hashset, hash_stable_nodemap};
hash_stable_hashset, hash_stable_nodemap,
hash_stable_btreemap};
mod fingerprint;
mod caching_codemap_view;
mod hcx;
Expand Down
55 changes: 54 additions & 1 deletion src/librustc_incremental/calculate_svh/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ use rustc::hir::def_id::{LOCAL_CRATE, CRATE_DEF_INDEX, DefId};
use rustc::hir::itemlikevisit::ItemLikeVisitor;
use rustc::ich::{Fingerprint, StableHashingContext};
use rustc::ty::TyCtxt;
use rustc::util::common::record_time;
use rustc_data_structures::stable_hasher::{StableHasher, HashStable};
use rustc_data_structures::fx::FxHashMap;
use rustc::util::common::record_time;
use rustc_data_structures::accumulate_vec::AccumulateVec;

pub type IchHasher = StableHasher<Fingerprint>;

Expand Down Expand Up @@ -159,6 +160,11 @@ impl<'a, 'tcx: 'a> ComputeItemHashesVisitor<'a, 'tcx> {
// difference, filter them out.
return None
}
DepNode::AllLocalTraitImpls => {
// These are already covered by hashing
// the HIR.
return None
}
ref other => {
bug!("Found unexpected DepNode during \
SVH computation: {:?}",
Expand Down Expand Up @@ -213,6 +219,49 @@ impl<'a, 'tcx: 'a> ComputeItemHashesVisitor<'a, 'tcx> {
true,
(module, (span, attrs)));
}

fn compute_and_store_ich_for_trait_impls(&mut self, krate: &'tcx hir::Crate)
{
let tcx = self.hcx.tcx();

let mut impls: Vec<(u64, Fingerprint)> = krate
.trait_impls
.iter()
.map(|(&trait_id, impls)| {
let trait_id = tcx.def_path_hash(trait_id);
let mut impls: AccumulateVec<[_; 32]> = impls
.iter()
.map(|&node_id| {
let def_id = tcx.hir.local_def_id(node_id);
tcx.def_path_hash(def_id)
})
.collect();

impls.sort_unstable();
let mut hasher = StableHasher::new();
impls.hash_stable(&mut self.hcx, &mut hasher);
(trait_id, hasher.finish())
})
.collect();

impls.sort_unstable();

let mut default_impls: AccumulateVec<[_; 32]> = krate
.trait_default_impl
.iter()
.map(|(&trait_def_id, &impl_node_id)| {
let impl_def_id = tcx.hir.local_def_id(impl_node_id);
(tcx.def_path_hash(trait_def_id), tcx.def_path_hash(impl_def_id))
})
.collect();

default_impls.sort_unstable();

let mut hasher = StableHasher::new();
impls.hash_stable(&mut self.hcx, &mut hasher);

self.hashes.insert(DepNode::AllLocalTraitImpls, hasher.finish());
}
}

impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for ComputeItemHashesVisitor<'a, 'tcx> {
Expand All @@ -235,6 +284,8 @@ impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for ComputeItemHashesVisitor<'a, 'tcx>
}
}



pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> IncrementalHashesMap {
let _ignore = tcx.dep_graph.in_ignore();
Expand Down Expand Up @@ -272,6 +323,8 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
let fingerprint = hasher.finish();
visitor.hashes.insert(dep_node, fingerprint);
}

visitor.compute_and_store_ich_for_trait_impls(krate);
});

tcx.sess.perf_stats.incr_comp_hashes_count.set(visitor.hashes.len() as u64);
Expand Down
9 changes: 9 additions & 0 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,20 @@ impl<'a> CrateLoader<'a> {
let exported_symbols = crate_root.exported_symbols
.map(|x| x.decode(&metadata).collect());

let trait_impls = crate_root
.impls
.map(|impls| {
impls.decode(&metadata)
.map(|trait_impls| (trait_impls.trait_id, trait_impls.impls))
.collect()
});

let mut cmeta = cstore::CrateMetadata {
name: name,
extern_crate: Cell::new(None),
def_path_table: def_path_table,
exported_symbols: exported_symbols,
trait_impls: trait_impls,
proc_macros: crate_root.macro_derive_registrar.map(|_| {
self.load_derive_macros(&crate_root, dylib.clone().map(|p| p.0), span)
}),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_metadata/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ pub struct CrateMetadata {

pub exported_symbols: Tracked<FxHashSet<DefIndex>>,

pub trait_impls: Tracked<FxHashMap<(u32, DefIndex), schema::LazySeq<DefIndex>>>,

pub dep_kind: Cell<DepKind>,
pub source: CrateSource,

Expand Down
4 changes: 1 addition & 3 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,8 @@ impl CrateStore for cstore::CStore {

fn implementations_of_trait(&self, filter: Option<DefId>) -> Vec<DefId>
{
if let Some(def_id) = filter {
self.dep_graph.read(DepNode::MetaData(def_id));
}
let mut result = vec![];

self.iter_crate_data(|_, cdata| {
cdata.get_implementations_for_trait(filter, &self.dep_graph, &mut result)
});
Expand Down
18 changes: 9 additions & 9 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,17 +961,17 @@ impl<'a, 'tcx> CrateMetadata {
None => None,
};

// FIXME(eddyb) Make this O(1) instead of O(n).
let dep_node = self.metadata_dep_node(GlobalMetaDataKind::Impls);
for trait_impls in self.root.impls.get(dep_graph, dep_node).decode(self) {
if filter.is_some() && filter != Some(trait_impls.trait_id) {
continue;
}

result.extend(trait_impls.impls.decode(self).map(|index| self.local_def_id(index)));

if filter.is_some() {
break;
if let Some(filter) = filter {
if let Some(impls) = self.trait_impls
.get(dep_graph, dep_node)
.get(&filter) {
result.extend(impls.decode(self).map(|idx| self.local_def_id(idx)));
}
} else {
for impls in self.trait_impls.get(dep_graph, dep_node).values() {
result.extend(impls.decode(self).map(|idx| self.local_def_id(idx)));
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions src/librustc_metadata/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,20 @@ impl<T> Tracked<T> {
}
}

impl<'a, 'tcx, T> HashStable<StableHashingContext<'a, 'tcx>> for Tracked<T>
where T: HashStable<StableHashingContext<'a, 'tcx>>
{
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a, 'tcx>,
hasher: &mut StableHasher<W>) {
let Tracked {
ref state
} = *self;

state.hash_stable(hcx, hasher);
}
}


#[derive(RustcEncodable, RustcDecodable)]
pub struct CrateRoot {
Expand Down