From 57d6ddd64973ff9cf95e131e657c8e41009becdb Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 29 Sep 2016 11:00:11 -0400 Subject: [PATCH 1/2] incr.comp.: Hide concrete hash algorithm used for ICH --- src/librustc/session/mod.rs | 8 ++++ .../calculate_svh/hasher.rs | 46 +++++++++++++++++++ src/librustc_incremental/calculate_svh/mod.rs | 20 ++++++-- .../calculate_svh/svh_visitor.rs | 6 +-- 4 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 src/librustc_incremental/calculate_svh/hasher.rs diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index f706bab32c80a..d002aba595bca 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -118,6 +118,8 @@ pub struct PerfStats { pub incr_comp_hashes_time: Cell, // The number of incr. comp. hash computations performed pub incr_comp_hashes_count: Cell, + // The number of bytes hashed when computing ICH values + pub incr_comp_bytes_hashed: Cell, // The accumulated time spent on computing symbol hashes pub symbol_hash_time: Cell, } @@ -439,6 +441,11 @@ impl Session { duration_to_secs_str(self.perf_stats.incr_comp_hashes_time.get())); println!("Total number of incr. comp. hashes computed: {}", self.perf_stats.incr_comp_hashes_count.get()); + println!("Total number of bytes hashed for incr. comp.: {}", + self.perf_stats.incr_comp_bytes_hashed.get()); + println!("Average bytes hashed per incr. comp. HIR node: {}", + self.perf_stats.incr_comp_bytes_hashed.get() / + self.perf_stats.incr_comp_hashes_count.get()); println!("Total time spent computing symbol hashes: {}", duration_to_secs_str(self.perf_stats.symbol_hash_time.get())); } @@ -571,6 +578,7 @@ pub fn build_session_(sopts: config::Options, svh_time: Cell::new(Duration::from_secs(0)), incr_comp_hashes_time: Cell::new(Duration::from_secs(0)), incr_comp_hashes_count: Cell::new(0), + incr_comp_bytes_hashed: Cell::new(0), symbol_hash_time: Cell::new(Duration::from_secs(0)), } }; diff --git a/src/librustc_incremental/calculate_svh/hasher.rs b/src/librustc_incremental/calculate_svh/hasher.rs new file mode 100644 index 0000000000000..28db39d667c4c --- /dev/null +++ b/src/librustc_incremental/calculate_svh/hasher.rs @@ -0,0 +1,46 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::hash::Hasher; +use std::collections::hash_map::DefaultHasher; + +#[derive(Debug)] +pub struct IchHasher { + // FIXME: this should use SHA1, not DefaultHasher. DefaultHasher is not + // built to avoid collisions. + state: DefaultHasher, + bytes_hashed: u64, +} + +impl IchHasher { + pub fn new() -> IchHasher { + IchHasher { + state: DefaultHasher::new(), + bytes_hashed: 0 + } + } + + pub fn bytes_hashed(&self) -> u64 { + self.bytes_hashed + } +} + +impl Hasher for IchHasher { + #[inline] + fn finish(&self) -> u64 { + self.state.finish() + } + + #[inline] + fn write(&mut self, bytes: &[u8]) { + self.state.write(bytes); + self.bytes_hashed += bytes.len() as u64; + } +} diff --git a/src/librustc_incremental/calculate_svh/mod.rs b/src/librustc_incremental/calculate_svh/mod.rs index a22b51ac04461..12627e02debd0 100644 --- a/src/librustc_incremental/calculate_svh/mod.rs +++ b/src/librustc_incremental/calculate_svh/mod.rs @@ -30,7 +30,6 @@ use syntax::ast; use std::cell::RefCell; use std::hash::{Hash, Hasher}; -use std::collections::hash_map::DefaultHasher; use rustc::dep_graph::DepNode; use rustc::hir; use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; @@ -43,10 +42,12 @@ use rustc::session::config::DebugInfoLevel::NoDebugInfo; use self::def_path_hash::DefPathHashes; use self::svh_visitor::StrictVersionHashVisitor; use self::caching_codemap_view::CachingCodemapView; +use self::hasher::IchHasher; mod def_path_hash; mod svh_visitor; mod caching_codemap_view; +mod hasher; pub struct IncrementalHashesMap { hashes: FnvHashMap, u64>, @@ -74,6 +75,10 @@ impl IncrementalHashesMap { pub fn iter<'a>(&'a self) -> ::std::collections::hash_map::Iter<'a, DepNode, u64> { self.hashes.iter() } + + pub fn len(&self) -> usize { + self.hashes.len() + } } impl<'a> ::std::ops::Index<&'a DepNode> for IncrementalHashesMap { @@ -102,6 +107,9 @@ pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) |v| visit::walk_crate(v, krate)); krate.visit_all_items(&mut visitor); }); + + tcx.sess.perf_stats.incr_comp_hashes_count.set(visitor.hashes.len() as u64); + record_time(&tcx.sess.perf_stats.svh_time, || visitor.compute_crate_hash()); visitor.hashes } @@ -127,9 +135,7 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { { assert!(def_id.is_local()); debug!("HashItemsVisitor::calculate(def_id={:?})", def_id); - // FIXME: this should use SHA1, not DefaultHasher. DefaultHasher is not - // built to avoid collisions. - let mut state = DefaultHasher::new(); + let mut state = IchHasher::new(); walk_op(&mut StrictVersionHashVisitor::new(&mut state, self.tcx, &mut self.def_path_hashes, @@ -138,12 +144,16 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> { let item_hash = state.finish(); self.hashes.insert(DepNode::Hir(def_id), item_hash); debug!("calculate_item_hash: def_id={:?} hash={:?}", def_id, item_hash); + + let bytes_hashed = self.tcx.sess.perf_stats.incr_comp_bytes_hashed.get() + + state.bytes_hashed(); + self.tcx.sess.perf_stats.incr_comp_bytes_hashed.set(bytes_hashed); } fn compute_crate_hash(&mut self) { let krate = self.tcx.map.krate(); - let mut crate_state = DefaultHasher::new(); + let mut crate_state = IchHasher::new(); let crate_disambiguator = self.tcx.sess.local_crate_disambiguator(); "crate_disambiguator".hash(&mut crate_state); diff --git a/src/librustc_incremental/calculate_svh/svh_visitor.rs b/src/librustc_incremental/calculate_svh/svh_visitor.rs index 3df68ac583d46..584e5598b9f9f 100644 --- a/src/librustc_incremental/calculate_svh/svh_visitor.rs +++ b/src/librustc_incremental/calculate_svh/svh_visitor.rs @@ -31,10 +31,10 @@ use rustc::hir::intravisit as visit; use rustc::ty::TyCtxt; use rustc_data_structures::fnv; use std::hash::Hash; -use std::collections::hash_map::DefaultHasher; use super::def_path_hash::DefPathHashes; use super::caching_codemap_view::CachingCodemapView; +use super::hasher::IchHasher; const IGNORED_ATTRIBUTES: &'static [&'static str] = &[ "cfg", @@ -48,7 +48,7 @@ const IGNORED_ATTRIBUTES: &'static [&'static str] = &[ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { pub tcx: TyCtxt<'hash, 'tcx, 'tcx>, - pub st: &'a mut DefaultHasher, + pub st: &'a mut IchHasher, // collect a deterministic hash of def-ids that we have seen def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, hash_spans: bool, @@ -56,7 +56,7 @@ pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { } impl<'a, 'hash, 'tcx> StrictVersionHashVisitor<'a, 'hash, 'tcx> { - pub fn new(st: &'a mut DefaultHasher, + pub fn new(st: &'a mut IchHasher, tcx: TyCtxt<'hash, 'tcx, 'tcx>, def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, codemap: &'a mut CachingCodemapView<'tcx>, From 954d89b75460d10cd96ec607d38cbf2fd028cf9c Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 29 Sep 2016 11:29:33 -0400 Subject: [PATCH 2/2] incr.comp.: Cover indirect changes in struct ICH test case --- src/test/incremental/hashes/struct_defs.rs | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/test/incremental/hashes/struct_defs.rs b/src/test/incremental/hashes/struct_defs.rs index 74c7797be2ab8..2d79987823f20 100644 --- a/src/test/incremental/hashes/struct_defs.rs +++ b/src/test/incremental/hashes/struct_defs.rs @@ -236,3 +236,74 @@ struct Visibility; #[rustc_clean(label="Hir", cfg="cfail3")] #[rustc_metadata_clean(cfg="cfail3")] pub struct Visibility; + + + + +struct ReferencedType1; +struct ReferencedType2; + +// Tuple Struct Change Field Type Indirectly ----------------------------------- +mod tuple_struct_change_field_type_indirectly { + #[cfg(cfail1)] + use super::ReferencedType1 as FieldType; + #[cfg(not(cfail1))] + use super::ReferencedType2 as FieldType; + + #[rustc_dirty(label="Hir", cfg="cfail2")] + #[rustc_clean(label="Hir", cfg="cfail3")] + #[rustc_metadata_dirty(cfg="cfail2")] + #[rustc_metadata_clean(cfg="cfail3")] + struct TupleStruct(FieldType); +} + + +// Record Struct Change Field Type Indirectly ----------------------------------- +mod record_struct_change_field_type_indirectly { + #[cfg(cfail1)] + use super::ReferencedType1 as FieldType; + #[cfg(not(cfail1))] + use super::ReferencedType2 as FieldType; + + #[rustc_dirty(label="Hir", cfg="cfail2")] + #[rustc_clean(label="Hir", cfg="cfail3")] + #[rustc_metadata_dirty(cfg="cfail2")] + #[rustc_metadata_clean(cfg="cfail3")] + struct RecordStruct { + _x: FieldType + } +} + + + + +trait ReferencedTrait1 {} +trait ReferencedTrait2 {} + +// Change Trait Bound Indirectly ----------------------------------------------- +mod change_trait_bound_indirectly { + #[cfg(cfail1)] + use super::ReferencedTrait1 as Trait; + #[cfg(not(cfail1))] + use super::ReferencedTrait2 as Trait; + + #[rustc_dirty(label="Hir", cfg="cfail2")] + #[rustc_clean(label="Hir", cfg="cfail3")] + #[rustc_metadata_dirty(cfg="cfail2")] + #[rustc_metadata_clean(cfg="cfail3")] + struct Struct(T); +} + +// Change Trait Bound Indirectly In Where Clause ------------------------------- +mod change_trait_bound_indirectly_in_where_clause { + #[cfg(cfail1)] + use super::ReferencedTrait1 as Trait; + #[cfg(not(cfail1))] + use super::ReferencedTrait2 as Trait; + + #[rustc_dirty(label="Hir", cfg="cfail2")] + #[rustc_clean(label="Hir", cfg="cfail3")] + #[rustc_metadata_dirty(cfg="cfail2")] + #[rustc_metadata_clean(cfg="cfail3")] + struct Struct(T) where T : Trait; +}