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

Jump to bottom

compute and cache HIR hashes at beginning #35854

Merged
merged 9 commits into from
Aug 23, 2016
Merged

compute and cache HIR hashes at beginning #35854

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6bd80d1
compute and cache HIR hashes at beginning
nikomatsakis Aug 19, 2016
0595a3a
hash the traits-in-scope determinstically
nikomatsakis Aug 20, 2016
226e1e5
add regression test for #35593
nikomatsakis Aug 23, 2016
484da37
rename HashesMap to IncrementalHashesMap
nikomatsakis Aug 23, 2016
f923083
cache def-path hashes across all items
nikomatsakis Aug 23, 2016
c42e0a3
make svh independent of item ordering
nikomatsakis Aug 23, 2016
ea2d90e
consider DepNode::Krate to be an input
nikomatsakis Aug 23, 2016
c21fa64
pacify the mercilous tidy
nikomatsakis Aug 23, 2016
1cc7c90
fix stray comment
nikomatsakis Aug 23, 2016
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
33 changes: 24 additions & 9 deletions src/librustc_incremental/calculate_svh/svh_visitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,33 @@ use rustc::hir::def::{Def, PathResolution};
use rustc::hir::def_id::DefId;
use rustc::hir::intravisit as visit;
use rustc::hir::intravisit::{Visitor, FnKind};
use rustc::hir::map::DefPath;
use rustc::ty::TyCtxt;
use rustc::util::nodemap::DefIdMap;

use std::hash::{Hash, SipHasher};

pub struct StrictVersionHashVisitor<'a, 'tcx: 'a> {
pub tcx: TyCtxt<'a, 'tcx, 'tcx>,
pub st: &'a mut SipHasher,

// collect a deterministic hash of def-ids that we have seen
def_id_hashes: DefIdMap<u64>,
Copy link
Member

Choose a reason for hiding this comment

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

I'd call this def_path_hashes_cache to make it clear, that this is a performance optimization.
It would make sense to share this map between invocations of the visitor (i.e. to allocate it once when the ICH-map is built and always give the visitors a reference to the same instance). That would reduce the number of memory allocations and presumably increase the number of cache hits.

More importantly, I'd document that this contains the hashes of just the DepPaths, not of the items the DefIds designate.

}

impl<'a, 'tcx> StrictVersionHashVisitor<'a, 'tcx> {
pub fn new(st: &'a mut SipHasher,
tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> Self {
StrictVersionHashVisitor { st: st, tcx: tcx }
StrictVersionHashVisitor { st: st, tcx: tcx, def_id_hashes: DefIdMap() }
}

fn hash_def_path(&mut self, path: &DefPath) {
path.deterministic_hash_to(self.tcx, self.st);
fn compute_def_id_hash(&mut self, def_id: DefId) -> u64 {
let tcx = self.tcx;
*self.def_id_hashes.entry(def_id)
.or_insert_with(|| {
let def_path = tcx.def_path(def_id);
def_path.deterministic_hash(tcx)
})
}
}

Expand Down Expand Up @@ -376,15 +384,22 @@ impl<'a, 'tcx> StrictVersionHashVisitor<'a, 'tcx> {
if let Some(traits) = self.tcx.trait_map.get(&id) {
debug!("hash_resolve: id={:?} traits={:?} st={:?}", id, traits, self.st);
traits.len().hash(self.st);
for candidate in traits {
self.hash_def_id(candidate.def_id);
}

// The ordering of the candidates is not fixed. So we hash
// the def-ids and then sort them and hash the collection.
let mut candidates: Vec<_> =
traits.iter()
.map(|&TraitCandidate { def_id, import_id: _ }| {
self.compute_def_id_hash(def_id)
})
.collect();
candidates.sort();
candidates.hash(self.st);
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a fine way to make it deterministic. One of the million cases though where I wish we had SmallVec available in the compiler :)

}
}

fn hash_def_id(&mut self, def_id: DefId) {
let def_path = self.tcx.def_path(def_id);
self.hash_def_path(&def_path);
self.compute_def_id_hash(def_id).hash(self.st);
}

fn hash_partial_def(&mut self, def: &PathResolution) {
Expand Down