Auto merge of rust-lang#36025 - michaelwoerister:incr-comp-hash-spans, r=nikomatsakis

incr. comp.: Take spans into account for ICH

This PR makes the ICH (incr. comp. hash) take spans into account when debuginfo is enabled.

A side-effect of this is that the SVH (which is based on the ICHs of all items in the crate) becomes sensitive to the tiniest change in a code base if debuginfo is enabled. Since we are not trying to model ABI compatibility via the SVH anymore (this is done via the crate disambiguator now), this should be not be a problem.

Fixes rust-lang#33888.
Fixes rust-lang#32753.

Author: bors

src/librustc/session/config.rs

@@ -913,6 +913,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
           "dump MIR state at various points in translation"),
     dump_mir_dir: Option<String> = (None, parse_opt_string, [UNTRACKED],
           "the directory the MIR is dumped into"),
+    perf_stats: bool = (false, parse_bool, [UNTRACKED],
+          "print some performance-related statistics"),
 }
 
 pub fn default_lib_output() -> CrateType {

src/librustc/session/mod.rs

@@ -18,6 +18,7 @@ use session::search_paths::PathKind;
 use session::config::{DebugInfoLevel, PanicStrategy};
 use ty::tls;
 use util::nodemap::{NodeMap, FnvHashMap};
+use util::common::duration_to_secs_str;
 use mir::transform as mir_pass;
 
 use syntax::ast::{NodeId, Name};
@@ -43,6 +44,7 @@ use std::env;
 use std::ffi::CString;
 use std::rc::Rc;
 use std::fmt;
+use std::time::Duration;
 use libc::c_int;
 
 pub mod config;
@@ -105,9 +107,23 @@ pub struct Session {
 
     incr_comp_session: RefCell<IncrCompSession>,
 
+    /// Some measurements that are being gathered during compilation.
+    pub perf_stats: PerfStats,
+
     next_node_id: Cell<ast::NodeId>,
 }
 
+pub struct PerfStats {
+    // The accumulated time needed for computing the SVH of the crate
+    pub svh_time: Cell<Duration>,
+    // The accumulated time spent on computing incr. comp. hashes
+    pub incr_comp_hashes_time: Cell<Duration>,
+    // The number of incr. comp. hash computations performed
+    pub incr_comp_hashes_count: Cell<u64>,
+    // The accumulated time spent on computing symbol hashes
+    pub symbol_hash_time: Cell<Duration>,
+}
+
 impl Session {
     pub fn local_crate_disambiguator(&self) -> token::InternedString {
         self.crate_disambiguator.borrow().clone()
@@ -411,6 +427,17 @@ impl Session {
             None
         }
     }
+
+    pub fn print_perf_stats(&self) {
+        println!("Total time spent computing SVHs:               {}",
+                 duration_to_secs_str(self.perf_stats.svh_time.get()));
+        println!("Total time spent computing incr. comp. hashes: {}",
+                 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 time spent computing symbol hashes:      {}",
+                 duration_to_secs_str(self.perf_stats.symbol_hash_time.get()));
+    }
 }
 
 pub fn build_session(sopts: config::Options,
@@ -528,6 +555,12 @@ pub fn build_session_(sopts: config::Options,
         available_macros: RefCell::new(HashSet::new()),
         imported_macro_spans: RefCell::new(HashMap::new()),
         incr_comp_session: RefCell::new(IncrCompSession::NotInitialized),
+        perf_stats: PerfStats {
+            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),
+            symbol_hash_time: Cell::new(Duration::from_secs(0)),
+        }
     };
 
     init_llvm(&sess);

src/librustc/util/common.rs

@@ -17,7 +17,7 @@ use std::fmt::Debug;
 use std::hash::{Hash, BuildHasher};
 use std::iter::repeat;
 use std::path::Path;
-use std::time::Instant;
+use std::time::{Duration, Instant};
 
 use hir;
 use hir::intravisit;
@@ -47,27 +47,44 @@ pub fn time<T, F>(do_it: bool, what: &str, f: F) -> T where
     let rv = f();
     let dur = start.elapsed();
 
-    // Hack up our own formatting for the duration to make it easier for scripts
-    // to parse (always use the same number of decimal places and the same unit).
-    const NANOS_PER_SEC: f64 = 1_000_000_000.0;
-    let secs = dur.as_secs() as f64;
-    let secs = secs + dur.subsec_nanos() as f64 / NANOS_PER_SEC;
-
     let mem_string = match get_resident() {
         Some(n) => {
             let mb = n as f64 / 1_000_000.0;
             format!("; rss: {}MB", mb.round() as usize)
         }
         None => "".to_owned(),
     };
-    println!("{}time: {:.3}{}\t{}", repeat("  ").take(old).collect::<String>(),
-             secs, mem_string, what);
+    println!("{}time: {}{}\t{}",
+             repeat("  ").take(old).collect::<String>(),
+             duration_to_secs_str(dur),
+             mem_string,
+             what);
 
     DEPTH.with(|slot| slot.set(old));
 
     rv
 }
 
+// Hack up our own formatting for the duration to make it easier for scripts
+// to parse (always use the same number of decimal places and the same unit).
+pub fn duration_to_secs_str(dur: Duration) -> String {
+    const NANOS_PER_SEC: f64 = 1_000_000_000.0;
+    let secs = dur.as_secs() as f64 +
+               dur.subsec_nanos() as f64 / NANOS_PER_SEC;
+
+    format!("{:.3}", secs)
+}
+
+pub fn record_time<T, F>(accu: &Cell<Duration>, f: F) -> T where
+    F: FnOnce() -> T,
+{
+    let start = Instant::now();
+    let rv = f();
+    let duration = start.elapsed();
+    accu.set(duration + accu.get());
+    rv
+}
+
 // Like std::macros::try!, but for Option<>.
 macro_rules! option_try(
     ($e:expr) => (match $e { Some(e) => e, None => return None })

src/librustc_data_structures/fnv.rs

@@ -57,3 +57,9 @@ impl Hasher for FnvHasher {
         self.0
     }
 }
+
+pub fn hash<T: Hash>(v: &T) -> u64 {
+    let mut state = FnvHasher::default();
+    v.hash(&mut state);
+    state.finish()
+}

src/librustc_driver/driver.rs

@@ -235,6 +235,10 @@ pub fn compile_input(sess: &Session,
     // any more, we can finalize it (which involves renaming it)
     rustc_incremental::finalize_session_directory(sess, trans.link.crate_hash);
 
+    if sess.opts.debugging_opts.perf_stats {
+        sess.print_perf_stats();
+    }
+
     controller_entry_point!(compilation_done,
                             sess,
                             CompileState::state_when_compilation_done(input, sess, outdir, output),

src/librustc_incremental/calculate_svh/caching_codemap_view.rs

@@ -0,0 +1,115 @@
+// 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use rustc::ty::TyCtxt;
+use std::rc::Rc;
+use syntax::codemap::CodeMap;
+use syntax_pos::{BytePos, FileMap};
+
+#[derive(Clone)]
+struct CacheEntry {
+    time_stamp: usize,
+    line_number: usize,
+    line_start: BytePos,
+    line_end: BytePos,
+    file: Rc<FileMap>,
+}
+
+pub struct CachingCodemapView<'tcx> {
+    codemap: &'tcx CodeMap,
+    line_cache: [CacheEntry; 3],
+    time_stamp: usize,
+}
+
+impl<'tcx> CachingCodemapView<'tcx> {
+    pub fn new<'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CachingCodemapView<'tcx> {
+        let codemap = tcx.sess.codemap();
+        let first_file = codemap.files.borrow()[0].clone();
+        let entry = CacheEntry {
+            time_stamp: 0,
+            line_number: 0,
+            line_start: BytePos(0),
+            line_end: BytePos(0),
+            file: first_file,
+        };
+
+        CachingCodemapView {
+            codemap: codemap,
+            line_cache: [entry.clone(), entry.clone(), entry.clone()],
+            time_stamp: 0,
+        }
+    }
+
+    pub fn codemap(&self) -> &'tcx CodeMap {
+        self.codemap
+    }
+
+    pub fn byte_pos_to_line_and_col(&mut self,
+                                    pos: BytePos)
+                                    -> Option<(Rc<FileMap>, usize, BytePos)> {
+        self.time_stamp += 1;
+
+        // Check if the position is in one of the cached lines
+        for cache_entry in self.line_cache.iter_mut() {
+            if pos >= cache_entry.line_start && pos < cache_entry.line_end {
+                cache_entry.time_stamp = self.time_stamp;
+                return Some((cache_entry.file.clone(),
+                             cache_entry.line_number,
+                             pos - cache_entry.line_start));
+            }
+        }
+
+        // No cache hit ...
+        let mut oldest = 0;
+        for index in 1 .. self.line_cache.len() {
+            if self.line_cache[index].time_stamp < self.line_cache[oldest].time_stamp {
+                oldest = index;
+            }
+        }
+
+        let cache_entry = &mut self.line_cache[oldest];
+
+        // If the entry doesn't point to the correct file, fix it up
+        if pos < cache_entry.file.start_pos || pos >= cache_entry.file.end_pos {
+            let file_valid;
+            let files = self.codemap.files.borrow();
+
+            if files.len() > 0 {
+                let file_index = self.codemap.lookup_filemap_idx(pos);
+                let file = files[file_index].clone();
+
+                if pos >= file.start_pos && pos < file.end_pos {
+                    cache_entry.file = file;
+                    file_valid = true;
+                } else {
+                    file_valid = false;
+                }
+            } else {
+                file_valid = false;
+            }
+
+            if !file_valid {
+                return None;
+            }
+        }
+
+        let line_index = cache_entry.file.lookup_line(pos).unwrap();
+        let line_bounds = cache_entry.file.line_bounds(line_index);
+
+        cache_entry.line_number = line_index + 1;
+        cache_entry.line_start = line_bounds.0;
+        cache_entry.line_end = line_bounds.1;
+        cache_entry.time_stamp = self.time_stamp;
+
+        return Some((cache_entry.file.clone(),
+                     cache_entry.line_number,
+                     pos - cache_entry.line_start));
+    }
+}

src/librustc_incremental/calculate_svh/mod.rs

@@ -35,32 +35,46 @@ use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
 use rustc::hir::intravisit as visit;
 use rustc::ty::TyCtxt;
 use rustc_data_structures::fnv::FnvHashMap;
+use rustc::util::common::record_time;
+use rustc::session::config::DebugInfoLevel::NoDebugInfo;
 
 use self::def_path_hash::DefPathHashes;
 use self::svh_visitor::StrictVersionHashVisitor;
+use self::caching_codemap_view::CachingCodemapView;
 
 mod def_path_hash;
 mod svh_visitor;
+mod caching_codemap_view;
 
 pub type IncrementalHashesMap = FnvHashMap<DepNode<DefId>, u64>;
 
 pub fn compute_incremental_hashes_map<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
                                                     -> IncrementalHashesMap {
     let _ignore = tcx.dep_graph.in_ignore();
     let krate = tcx.map.krate();
-    let mut visitor = HashItemsVisitor { tcx: tcx,
-                                         hashes: FnvHashMap(),
-                                         def_path_hashes: DefPathHashes::new(tcx) };
-    visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX), |v| visit::walk_crate(v, krate));
-    krate.visit_all_items(&mut visitor);
-    visitor.compute_crate_hash();
+    let hash_spans = tcx.sess.opts.debuginfo != NoDebugInfo;
+    let mut visitor = HashItemsVisitor {
+        tcx: tcx,
+        hashes: FnvHashMap(),
+        def_path_hashes: DefPathHashes::new(tcx),
+        codemap: CachingCodemapView::new(tcx),
+        hash_spans: hash_spans,
+    };
+    record_time(&tcx.sess.perf_stats.incr_comp_hashes_time, || {
+        visitor.calculate_def_id(DefId::local(CRATE_DEF_INDEX),
+                                 |v| visit::walk_crate(v, krate));
+        krate.visit_all_items(&mut visitor);
+    });
+    record_time(&tcx.sess.perf_stats.svh_time, || visitor.compute_crate_hash());
     visitor.hashes
 }
 
 struct HashItemsVisitor<'a, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     def_path_hashes: DefPathHashes<'a, 'tcx>,
+    codemap: CachingCodemapView<'tcx>,
     hashes: IncrementalHashesMap,
+    hash_spans: bool,
 }
 
 impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
@@ -81,7 +95,9 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
         let mut state = SipHasher::new();
         walk_op(&mut StrictVersionHashVisitor::new(&mut state,
                                                    self.tcx,
-                                                   &mut self.def_path_hashes));
+                                                   &mut self.def_path_hashes,
+                                                   &mut self.codemap,
+                                                   self.hash_spans));
         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);
@@ -117,9 +133,13 @@ impl<'a, 'tcx> HashItemsVisitor<'a, 'tcx> {
             item_hashes.hash(&mut crate_state);
         }
 
-        for attr in &krate.attrs {
-            debug!("krate attr {:?}", attr);
-            attr.meta().hash(&mut crate_state);
+        {
+            let mut visitor = StrictVersionHashVisitor::new(&mut crate_state,
+                                                            self.tcx,
+                                                            &mut self.def_path_hashes,
+                                                            &mut self.codemap,
+                                                            self.hash_spans);
+            visitor.hash_attributes(&krate.attrs);
         }
 
         let crate_hash = crate_state.finish();