Skip to content

Commit 2e7df80

Browse files
committed
make metadata hashes determinstic
When we hash the inputs to a MetaData node, we have to hash them in a consistent order. We achieve this by sorting the stringfied `DefPath` entries. Also, micro-optimie by cache more results across the saving process.
1 parent 2797b2a commit 2e7df80

File tree

7 files changed

+79
-58
lines changed

7 files changed

+79
-58
lines changed

src/librustc/hir/map/definitions.rs

+24
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@ use middle::cstore::LOCAL_CRATE;
1212
use hir::def_id::{DefId, DefIndex};
1313
use hir::map::def_collector::DefCollector;
1414
use rustc_data_structures::fnv::FnvHashMap;
15+
use std::fmt::Write;
1516
use syntax::{ast, visit};
1617
use syntax::parse::token::InternedString;
18+
use ty::TyCtxt;
1719
use util::nodemap::NodeMap;
1820

1921
/// The definition table containing node definitions
@@ -109,6 +111,28 @@ impl DefPath {
109111
data.reverse();
110112
DefPath { data: data, krate: krate }
111113
}
114+
115+
pub fn to_string(&self, tcx: TyCtxt) -> String {
116+
let mut s = String::with_capacity(self.data.len() * 16);
117+
118+
if self.krate == LOCAL_CRATE {
119+
s.push_str(&tcx.crate_name(self.krate));
120+
} else {
121+
s.push_str(&tcx.sess.cstore.original_crate_name(self.krate));
122+
}
123+
s.push_str("/");
124+
s.push_str(&tcx.crate_disambiguator(self.krate));
125+
126+
for component in &self.data {
127+
write!(s,
128+
"::{}[{}]",
129+
component.data.as_interned_str(),
130+
component.disambiguator)
131+
.unwrap();
132+
}
133+
134+
s
135+
}
112136
}
113137

114138
/// Root of an inlined item. We track the `DefPath` of the item within

src/librustc_incremental/calculate_svh.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,7 @@ mod svh_visitor {
144144
}
145145

146146
fn hash_def_path(&mut self, path: &DefPath) {
147-
self.tcx.crate_name(path.krate).hash(self.st);
148-
self.tcx.crate_disambiguator(path.krate).hash(self.st);
149-
for data in &path.data {
150-
data.data.as_interned_str().hash(self.st);
151-
data.disambiguator.hash(self.st);
152-
}
147+
path.to_string(self.tcx).hash(self.st);
153148
}
154149
}
155150

src/librustc_incremental/persist/directory.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -159,12 +159,17 @@ impl<'a,'tcx> DefIdDirectoryBuilder<'a,'tcx> {
159159
.clone()
160160
}
161161

162+
pub fn lookup_def_path(&self, id: DefPathIndex) -> &DefPath {
163+
&self.directory.paths[id.index as usize]
164+
}
165+
166+
162167
pub fn map(&mut self, node: &DepNode<DefId>) -> DepNode<DefPathIndex> {
163168
node.map_def(|&def_id| Some(self.add(def_id))).unwrap()
164169
}
165170

166-
pub fn into_directory(self) -> DefIdDirectory {
167-
self.directory
171+
pub fn directory(&self) -> &DefIdDirectory {
172+
&self.directory
168173
}
169174
}
170175

src/librustc_incremental/persist/hash.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,19 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
3939
}
4040
}
4141

42-
pub fn hash(&mut self, dep_node: &DepNode<DefId>) -> Option<u64> {
42+
pub fn hash(&mut self, dep_node: &DepNode<DefId>) -> Option<(DefId, u64)> {
4343
match *dep_node {
4444
// HIR nodes (which always come from our crate) are an input:
4545
DepNode::Hir(def_id) => {
46-
Some(self.hir_hash(def_id))
46+
Some((def_id, self.hir_hash(def_id)))
4747
}
4848

4949
// MetaData from other crates is an *input* to us.
5050
// MetaData nodes from *our* crates are an *output*; we
5151
// don't hash them, but we do compute a hash for them and
5252
// save it for others to use.
5353
DepNode::MetaData(def_id) if !def_id.is_local() => {
54-
Some(self.metadata_hash(def_id))
54+
Some((def_id, self.metadata_hash(def_id)))
5555
}
5656

5757
_ => {

src/librustc_incremental/persist/load.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,14 @@ fn initial_dirty_nodes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
163163
let mut items_removed = false;
164164
let mut dirty_nodes = FnvHashSet();
165165
for hash in hashes {
166-
debug!("initial_dirty_nodes: retracing {:?}", hash);
167166
match hash.node.map_def(|&i| retraced.def_id(i)) {
168167
Some(dep_node) => {
169-
let current_hash = hcx.hash(&dep_node).unwrap();
168+
let (_, current_hash) = hcx.hash(&dep_node).unwrap();
170169
if current_hash != hash.hash {
171170
debug!("initial_dirty_nodes: {:?} is dirty as hash is {:?}, was {:?}",
172-
dep_node, current_hash, hash.hash);
171+
dep_node.map_def(|&def_id| Some(tcx.def_path(def_id))).unwrap(),
172+
current_hash,
173+
hash.hash);
173174
dirty_nodes.insert(dep_node);
174175
}
175176
}

src/librustc_incremental/persist/save.rs

+38-19
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use rustc::session::Session;
1616
use rustc::ty::TyCtxt;
1717
use rustc_data_structures::fnv::FnvHashMap;
1818
use rustc_serialize::{Encodable as RustcEncodable};
19-
use std::hash::{Hasher, SipHasher};
19+
use std::hash::{Hash, Hasher, SipHasher};
2020
use std::io::{self, Cursor, Write};
2121
use std::fs::{self, File};
2222
use std::path::PathBuf;
@@ -30,9 +30,14 @@ pub fn save_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
3030
debug!("save_dep_graph()");
3131
let _ignore = tcx.dep_graph.in_ignore();
3232
let sess = tcx.sess;
33+
if sess.opts.incremental.is_none() {
34+
return;
35+
}
3336
let mut hcx = HashContext::new(tcx);
34-
save_in(sess, dep_graph_path(tcx), |e| encode_dep_graph(&mut hcx, e));
35-
save_in(sess, metadata_hash_path(tcx, LOCAL_CRATE), |e| encode_metadata_hashes(&mut hcx, e));
37+
let mut builder = DefIdDirectoryBuilder::new(tcx);
38+
let query = tcx.dep_graph.query();
39+
save_in(sess, dep_graph_path(tcx), |e| encode_dep_graph(&mut hcx, &mut builder, &query, e));
40+
save_in(sess, metadata_hash_path(tcx, LOCAL_CRATE), |e| encode_metadata_hashes(&mut hcx, &mut builder, &query, e));
3641
}
3742

3843
pub fn save_work_products(sess: &Session, local_crate_name: &str) {
@@ -96,21 +101,19 @@ fn save_in<F>(sess: &Session,
96101
}
97102

98103
pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
104+
builder: &mut DefIdDirectoryBuilder,
105+
query: &DepGraphQuery<DefId>,
99106
encoder: &mut Encoder)
100107
-> io::Result<()>
101108
{
102-
let tcx = hcx.tcx;
103-
let query = tcx.dep_graph.query();
104109
let (nodes, edges) = post_process_graph(hcx, query);
105110

106-
let mut builder = DefIdDirectoryBuilder::new(tcx);
107-
108111
// Create hashes for inputs.
109112
let hashes =
110113
nodes.iter()
111114
.filter_map(|dep_node| {
112115
hcx.hash(dep_node)
113-
.map(|hash| {
116+
.map(|(_, hash)| {
114117
let node = builder.map(dep_node);
115118
SerializedHash { node: node, hash: hash }
116119
})
@@ -133,15 +136,14 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
133136
debug!("graph = {:#?}", graph);
134137

135138
// Encode the directory and then the graph data.
136-
let directory = builder.into_directory();
137-
try!(directory.encode(encoder));
139+
try!(builder.directory().encode(encoder));
138140
try!(graph.encode(encoder));
139141

140142
Ok(())
141143
}
142144

143145
pub fn post_process_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
144-
query: DepGraphQuery<DefId>)
146+
query: &DepGraphQuery<DefId>)
145147
-> (Vec<DepNode<DefId>>, Vec<(DepNode<DefId>, DepNode<DefId>)>)
146148
{
147149
let tcx = hcx.tcx;
@@ -192,11 +194,12 @@ pub fn post_process_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
192194

193195

194196
pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
197+
builder: &mut DefIdDirectoryBuilder,
198+
query: &DepGraphQuery<DefId>,
195199
encoder: &mut Encoder)
196200
-> io::Result<()>
197201
{
198202
let tcx = hcx.tcx;
199-
let query = tcx.dep_graph.query();
200203

201204
let serialized_hashes = {
202205
// Identify the `MetaData(X)` nodes where `X` is local. These are
@@ -224,16 +227,32 @@ pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
224227
let dep_node = DepNode::MetaData(def_id);
225228
let mut state = SipHasher::new();
226229
debug!("save: computing metadata hash for {:?}", dep_node);
227-
for node in query.transitive_predecessors(&dep_node) {
228-
if let Some(hash) = hcx.hash(&node) {
229-
debug!("save: predecessor {:?} has hash {}", node, hash);
230-
state.write_u64(hash.to_le());
231-
} else {
232-
debug!("save: predecessor {:?} cannot be hashed", node);
233-
}
230+
231+
let predecessors = query.transitive_predecessors(&dep_node);
232+
let mut hashes: Vec<_> =
233+
predecessors.iter()
234+
.filter_map(|node| hcx.hash(&node))
235+
.map(|(def_id, hash)| {
236+
let index = builder.add(def_id);
237+
let path = builder.lookup_def_path(index);
238+
(path.to_string(tcx), hash) // (*)
239+
})
240+
.collect();
241+
242+
// (*) creating a `String` from each def-path is a bit inefficient,
243+
// but it's the easiest way to get a deterministic ord/hash.
244+
245+
hashes.sort();
246+
state.write_usize(hashes.len());
247+
for (path, hash) in hashes {
248+
debug!("save: predecessor {:?} has hash {}", path, hash);
249+
path.hash(&mut state);
250+
state.write_u64(hash.to_le());
234251
}
252+
235253
let hash = state.finish();
236254
debug!("save: metadata hash for {:?} is {}", dep_node, hash);
255+
237256
SerializedMetadataHash {
238257
def_index: def_id.index,
239258
hash: hash,

src/librustc_trans/back/symbol_names.rs

+2-25
Original file line numberDiff line numberDiff line change
@@ -108,36 +108,13 @@ use rustc::ty::{self, TyCtxt, TypeFoldable};
108108
use rustc::ty::item_path::{self, ItemPathBuffer, RootMode};
109109
use rustc::hir::map::definitions::{DefPath, DefPathData};
110110

111-
use std::fmt::Write;
112111
use syntax::attr;
113112
use syntax::parse::token::{self, InternedString};
114113
use serialize::hex::ToHex;
115114

116115
pub fn def_id_to_string<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> String {
117116
let def_path = tcx.def_path(def_id);
118-
def_path_to_string(tcx, &def_path)
119-
}
120-
121-
fn def_path_to_string<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_path: &DefPath) -> String {
122-
let mut s = String::with_capacity(def_path.data.len() * 16);
123-
124-
if def_path.krate == cstore::LOCAL_CRATE {
125-
s.push_str(&tcx.crate_name(def_path.krate));
126-
} else {
127-
s.push_str(&tcx.sess.cstore.original_crate_name(def_path.krate));
128-
}
129-
s.push_str("/");
130-
s.push_str(&tcx.crate_disambiguator(def_path.krate));
131-
132-
for component in &def_path.data {
133-
write!(s,
134-
"::{}[{}]",
135-
component.data.as_interned_str(),
136-
component.disambiguator)
137-
.unwrap();
138-
}
139-
140-
s
117+
def_path.to_string(tcx)
141118
}
142119

143120
fn get_symbol_hash<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
@@ -167,7 +144,7 @@ fn get_symbol_hash<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
167144
// the main symbol name is not necessarily unique; hash in the
168145
// compiler's internal def-path, guaranteeing each symbol has a
169146
// truly unique path
170-
hash_state.input_str(&def_path_to_string(tcx, def_path));
147+
hash_state.input_str(&def_path.to_string(tcx));
171148

172149
// Include the main item-type. Note that, in this case, the
173150
// assertions about `needs_subst` may not hold, but this item-type

0 commit comments

Comments
 (0)