Skip to content

Commit

Permalink
Rollup merge of rust-lang#41056 - michaelwoerister:central-defpath-ha…
Browse files Browse the repository at this point in the history
…shes, r=nikomatsakis

Handle DefPath hashing centrally as part of DefPathTable (+ save work during SVH calculation)

In almost all cases where we construct a `DefPath`, we just hash it and throw it away again immediately.
With this PR, the compiler will immediately compute and store the hash for each `DefPath` as it is allocated. This way we
+ can get rid of any subsequent `DefPath` hash caching (e.g. the `DefPathHashes`),
+ don't need to allocate a transient `Vec` for holding the `DefPath` (although I'm always surprised how little these small, dynamic allocations seem to hurt performance), and
+ we don't hash `DefPath` prefixes over and over again.

That last part is because we construct the hash for `prefix::foo` by hashing `(hash(prefix), foo)` instead of hashing every component of prefix.

The last commit of this PR is pretty neat, I think:
```
The SVH (Strict Version Hash) of a crate is currently computed
by hashing the ICHes (Incremental Computation Hashes) of the
crate's HIR. This is fine, expect that for incr. comp. we compute
two ICH values for each HIR item, one for the complete item and
one that just includes the item's interface. The two hashes are
are needed for dependency tracking but if we are compiling
non-incrementally and just need the ICH values for the SVH,
one of them is enough, giving us the opportunity to save some
work in this case.
```

r? @nikomatsakis

This PR depends on rust-lang#40878 to be merged first (you can ignore the first commit for reviewing, that's just rust-lang#40878).
  • Loading branch information
frewsxcv authored Apr 7, 2017
2 parents ef9eee7 + bb63872 commit 88e97f0
Show file tree
Hide file tree
Showing 19 changed files with 161 additions and 131 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2697,7 +2697,7 @@ impl<'a> LoweringContext<'a> {
fn pat_ident_binding_mode(&mut self, span: Span, name: Name, bm: hir::BindingMode)
-> P<hir::Pat> {
let id = self.next_id();
let parent_def = self.parent_def;
let parent_def = self.parent_def.unwrap();
let def_id = {
let defs = self.resolver.definitions();
let def_path_data = DefPathData::Binding(name.as_str());
Expand Down
19 changes: 4 additions & 15 deletions src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ impl<'a> DefCollector<'a> {
}
}

pub fn collect_root(&mut self) {
let root = self.create_def_with_parent(None,
CRATE_NODE_ID,
DefPathData::CrateRoot,
ITEM_LIKE_SPACE);
pub fn collect_root(&mut self, crate_name: &str, crate_disambiguator: &str) {
let root = self.definitions.create_root_def(crate_name,
crate_disambiguator);
assert_eq!(root, CRATE_DEF_INDEX);
self.parent_def = Some(root);
}
Expand All @@ -54,20 +52,11 @@ impl<'a> DefCollector<'a> {
data: DefPathData,
address_space: DefIndexAddressSpace)
-> DefIndex {
let parent_def = self.parent_def;
let parent_def = self.parent_def.unwrap();
debug!("create_def(node_id={:?}, data={:?}, parent_def={:?})", node_id, data, parent_def);
self.definitions.create_def_with_parent(parent_def, node_id, data, address_space)
}

fn create_def_with_parent(&mut self,
parent: Option<DefIndex>,
node_id: NodeId,
data: DefPathData,
address_space: DefIndexAddressSpace)
-> DefIndex {
self.definitions.create_def_with_parent(parent, node_id, data, address_space)
}

pub fn with_parent<F: FnOnce(&mut Self)>(&mut self, parent_def: DefIndex, f: F) {
let parent = self.parent_def;
self.parent_def = Some(parent_def);
Expand Down
115 changes: 93 additions & 22 deletions src/librustc/hir/map/definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::stable_hasher::StableHasher;
use serialize::{Encodable, Decodable, Encoder, Decoder};
use std::fmt::Write;
use std::hash::{Hash, Hasher};
use std::hash::Hash;
use syntax::ast;
use syntax::symbol::{Symbol, InternedString};
use ty::TyCtxt;
Expand All @@ -34,6 +34,7 @@ use util::nodemap::NodeMap;
pub struct DefPathTable {
index_to_key: [Vec<DefKey>; 2],
key_to_index: FxHashMap<DefKey, DefIndex>,
def_path_hashes: [Vec<u64>; 2],
}

// Unfortunately we have to provide a manual impl of Clone because of the
Expand All @@ -44,6 +45,8 @@ impl Clone for DefPathTable {
index_to_key: [self.index_to_key[0].clone(),
self.index_to_key[1].clone()],
key_to_index: self.key_to_index.clone(),
def_path_hashes: [self.def_path_hashes[0].clone(),
self.def_path_hashes[1].clone()],
}
}
}
Expand All @@ -52,6 +55,7 @@ impl DefPathTable {

fn allocate(&mut self,
key: DefKey,
def_path_hash: u64,
address_space: DefIndexAddressSpace)
-> DefIndex {
let index = {
Expand All @@ -62,6 +66,9 @@ impl DefPathTable {
index
};
self.key_to_index.insert(key, index);
self.def_path_hashes[address_space.index()].push(def_path_hash);
debug_assert!(self.def_path_hashes[address_space.index()].len() ==
self.index_to_key[address_space.index()].len());
index
}

Expand All @@ -71,6 +78,12 @@ impl DefPathTable {
[index.as_array_index()].clone()
}

#[inline(always)]
pub fn def_path_hash(&self, index: DefIndex) -> u64 {
self.def_path_hashes[index.address_space().index()]
[index.as_array_index()]
}

#[inline(always)]
pub fn def_index_for_def_key(&self, key: &DefKey) -> Option<DefIndex> {
self.key_to_index.get(key).cloned()
Expand Down Expand Up @@ -116,17 +129,28 @@ impl DefPathTable {

impl Encodable for DefPathTable {
fn encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
// Index to key
self.index_to_key[DefIndexAddressSpace::Low.index()].encode(s)?;
self.index_to_key[DefIndexAddressSpace::High.index()].encode(s)
self.index_to_key[DefIndexAddressSpace::High.index()].encode(s)?;

// DefPath hashes
self.def_path_hashes[DefIndexAddressSpace::Low.index()].encode(s)?;
self.def_path_hashes[DefIndexAddressSpace::High.index()].encode(s)?;

Ok(())
}
}

impl Decodable for DefPathTable {
fn decode<D: Decoder>(d: &mut D) -> Result<DefPathTable, D::Error> {
let index_to_key_lo: Vec<DefKey> = Decodable::decode(d)?;
let index_to_key_high: Vec<DefKey> = Decodable::decode(d)?;
let index_to_key_hi: Vec<DefKey> = Decodable::decode(d)?;

let index_to_key = [index_to_key_lo, index_to_key_high];
let def_path_hashes_lo: Vec<u64> = Decodable::decode(d)?;
let def_path_hashes_hi: Vec<u64> = Decodable::decode(d)?;

let index_to_key = [index_to_key_lo, index_to_key_hi];
let def_path_hashes = [def_path_hashes_lo, def_path_hashes_hi];

let mut key_to_index = FxHashMap();

Expand All @@ -141,6 +165,7 @@ impl Decodable for DefPathTable {
Ok(DefPathTable {
index_to_key: index_to_key,
key_to_index: key_to_index,
def_path_hashes: def_path_hashes,
})
}
}
Expand Down Expand Up @@ -184,6 +209,29 @@ pub struct DefKey {
pub disambiguated_data: DisambiguatedDefPathData,
}

impl DefKey {
fn compute_stable_hash(&self, parent_hash: u64) -> u64 {
let mut hasher = StableHasher::new();

// We hash a 0u8 here to disambiguate between regular DefPath hashes,
// and the special "root_parent" below.
0u8.hash(&mut hasher);
parent_hash.hash(&mut hasher);
self.disambiguated_data.hash(&mut hasher);
hasher.finish()
}

fn root_parent_stable_hash(crate_name: &str, crate_disambiguator: &str) -> u64 {
let mut hasher = StableHasher::new();
// Disambiguate this from a regular DefPath hash,
// see compute_stable_hash() above.
1u8.hash(&mut hasher);
crate_name.hash(&mut hasher);
crate_disambiguator.hash(&mut hasher);
hasher.finish()
}
}

/// Pair of `DefPathData` and an integer disambiguator. The integer is
/// normally 0, but in the event that there are multiple defs with the
/// same `parent` and `data`, we use this field to disambiguate
Expand Down Expand Up @@ -271,19 +319,6 @@ impl DefPath {

s
}

pub fn deterministic_hash(&self, tcx: TyCtxt) -> u64 {
debug!("deterministic_hash({:?})", self);
let mut state = StableHasher::new();
self.deterministic_hash_to(tcx, &mut state);
state.finish()
}

pub fn deterministic_hash_to<H: Hasher>(&self, tcx: TyCtxt, state: &mut H) {
tcx.original_crate_name(self.krate).as_str().hash(state);
tcx.crate_disambiguator(self.krate).as_str().hash(state);
self.data.hash(state);
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
Expand Down Expand Up @@ -338,6 +373,7 @@ impl Definitions {
table: DefPathTable {
index_to_key: [vec![], vec![]],
key_to_index: FxHashMap(),
def_path_hashes: [vec![], vec![]],
},
node_to_def_index: NodeMap(),
def_index_to_node: [vec![], vec![]],
Expand All @@ -359,6 +395,11 @@ impl Definitions {
self.table.def_key(index)
}

#[inline(always)]
pub fn def_path_hash(&self, index: DefIndex) -> u64 {
self.table.def_path_hash(index)
}

pub fn def_index_for_def_key(&self, key: DefKey) -> Option<DefIndex> {
self.table.def_index_for_def_key(&key)
}
Expand Down Expand Up @@ -398,12 +439,38 @@ impl Definitions {
self.node_to_hir_id[node_id]
}

/// Add a definition with a parent definition.
pub fn create_root_def(&mut self,
crate_name: &str,
crate_disambiguator: &str)
-> DefIndex {
let key = DefKey {
parent: None,
disambiguated_data: DisambiguatedDefPathData {
data: DefPathData::CrateRoot,
disambiguator: 0
}
};

let parent_hash = DefKey::root_parent_stable_hash(crate_name,
crate_disambiguator);
let def_path_hash = key.compute_stable_hash(parent_hash);

// Create the definition.
let address_space = super::ITEM_LIKE_SPACE;
let index = self.table.allocate(key, def_path_hash, address_space);
assert!(self.def_index_to_node[address_space.index()].is_empty());
self.def_index_to_node[address_space.index()].push(ast::CRATE_NODE_ID);
self.node_to_def_index.insert(ast::CRATE_NODE_ID, index);

index
}

/// Add a definition with a parent definition.
pub fn create_def_with_parent(&mut self,
parent: Option<DefIndex>,
parent: DefIndex,
node_id: ast::NodeId,
data: DefPathData,
// is_owner: bool)
address_space: DefIndexAddressSpace)
-> DefIndex {
debug!("create_def_with_parent(parent={:?}, node_id={:?}, data={:?})",
Expand All @@ -415,12 +482,13 @@ impl Definitions {
data,
self.table.def_key(self.node_to_def_index[&node_id]));

assert_eq!(parent.is_some(), data != DefPathData::CrateRoot);
// The root node must be created with create_root_def()
assert!(data != DefPathData::CrateRoot);

// Find a unique DefKey. This basically means incrementing the disambiguator
// until we get no match.
let mut key = DefKey {
parent: parent,
parent: Some(parent),
disambiguated_data: DisambiguatedDefPathData {
data: data,
disambiguator: 0
Expand All @@ -431,10 +499,13 @@ impl Definitions {
key.disambiguated_data.disambiguator += 1;
}

let parent_hash = self.table.def_path_hash(parent);
let def_path_hash = key.compute_stable_hash(parent_hash);

debug!("create_def_with_parent: after disambiguation, key = {:?}", key);

// Create the definition.
let index = self.table.allocate(key, address_space);
let index = self.table.allocate(key, def_path_hash, address_space);
assert_eq!(index.as_array_index(),
self.def_index_to_node[address_space.index()].len());
self.def_index_to_node[address_space.index()].push(node_id);
Expand Down
36 changes: 0 additions & 36 deletions src/librustc/ich/def_path_hash.rs

This file was deleted.

6 changes: 2 additions & 4 deletions src/librustc/ich/hcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use hir;
use hir::def_id::DefId;
use ich::{self, CachingCodemapView, DefPathHashes};
use ich::{self, CachingCodemapView};
use session::config::DebugInfoLevel::NoDebugInfo;
use ty;

Expand All @@ -32,7 +32,6 @@ use rustc_data_structures::accumulate_vec::AccumulateVec;
/// things (e.g. each DefId/DefPath is only hashed once).
pub struct StableHashingContext<'a, 'tcx: 'a> {
tcx: ty::TyCtxt<'a, 'tcx, 'tcx>,
def_path_hashes: DefPathHashes<'a, 'tcx>,
codemap: CachingCodemapView<'tcx>,
hash_spans: bool,
hash_bodies: bool,
Expand Down Expand Up @@ -64,7 +63,6 @@ impl<'a, 'tcx: 'a> StableHashingContext<'a, 'tcx> {

StableHashingContext {
tcx: tcx,
def_path_hashes: DefPathHashes::new(tcx),
codemap: CachingCodemapView::new(tcx),
hash_spans: hash_spans_initial,
hash_bodies: true,
Expand Down Expand Up @@ -111,7 +109,7 @@ impl<'a, 'tcx: 'a> StableHashingContext<'a, 'tcx> {

#[inline]
pub fn def_path_hash(&mut self, def_id: DefId) -> u64 {
self.def_path_hashes.hash(def_id)
self.tcx.def_path_hash(def_id)
}

#[inline]
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/ich/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@
//! ICH - Incremental Compilation Hash

pub use self::fingerprint::Fingerprint;
pub use self::def_path_hash::DefPathHashes;
pub use self::caching_codemap_view::CachingCodemapView;
pub use self::hcx::{StableHashingContext, NodeIdHashingMode};

mod fingerprint;
mod def_path_hash;
mod caching_codemap_view;
mod hcx;

Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ pub trait CrateStore {
-> Option<DefId>;
fn def_key(&self, def: DefId) -> DefKey;
fn def_path(&self, def: DefId) -> hir_map::DefPath;
fn def_path_hash(&self, def: DefId) -> u64;
fn struct_field_names(&self, def: DefId) -> Vec<ast::Name>;
fn item_children(&self, did: DefId) -> Vec<def::Export>;
fn load_macro(&self, did: DefId, sess: &Session) -> LoadedMacro;
Expand Down Expand Up @@ -377,6 +378,9 @@ impl CrateStore for DummyCrateStore {
fn def_path(&self, def: DefId) -> hir_map::DefPath {
bug!("relative_def_path")
}
fn def_path_hash(&self, def: DefId) -> u64 {
bug!("wa")
}
fn struct_field_names(&self, def: DefId) -> Vec<ast::Name> { bug!("struct_field_names") }
fn item_children(&self, did: DefId) -> Vec<def::Export> { bug!("item_children") }
fn load_macro(&self, did: DefId, sess: &Session) -> LoadedMacro { bug!("load_macro") }
Expand Down
Loading

0 comments on commit 88e97f0

Please sign in to comment.