From a67ded06a36b7d758a63d15f0a4c59b9f678f14c Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 10 Apr 2018 09:58:46 +0200 Subject: [PATCH 1/6] Don't recurse into allocations, use a global table instead --- src/librustc/ich/impls_ty.rs | 24 +++-- src/librustc/mir/interpret/mod.rs | 48 ++++----- src/librustc/ty/maps/on_disk_cache.rs | 135 ++++++++++++++----------- src/librustc_metadata/decoder.rs | 49 ++++----- src/librustc_metadata/encoder.rs | 68 +++++++++---- src/librustc_metadata/schema.rs | 1 + src/test/incremental/static_cycle/b.rs | 19 ++++ 7 files changed, 192 insertions(+), 152 deletions(-) create mode 100644 src/test/incremental/static_cycle/b.rs diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 41cfac2674be6..2425fef7e7f53 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -398,12 +398,12 @@ impl_stable_hash_for!(struct mir::interpret::MemoryPointer { enum AllocDiscriminant { Alloc, - ExternStatic, + Static, Function, } impl_stable_hash_for!(enum self::AllocDiscriminant { Alloc, - ExternStatic, + Static, Function }); @@ -414,24 +414,26 @@ impl<'a> HashStable> for mir::interpret::AllocId { hasher: &mut StableHasher, ) { ty::tls::with_opt(|tcx| { + trace!("hashing {:?}", *self); let tcx = tcx.expect("can't hash AllocIds during hir lowering"); - if let Some(alloc) = tcx.interpret_interner.get_alloc(*self) { + if let Some(def_id) = tcx.interpret_interner + .get_corresponding_static_def_id(*self) { + AllocDiscriminant::Static.hash_stable(hcx, hasher); + trace!("hashing {:?} as static {:?}", *self, def_id); + def_id.hash_stable(hcx, hasher); + } else if let Some(alloc) = tcx.interpret_interner.get_alloc(*self) { AllocDiscriminant::Alloc.hash_stable(hcx, hasher); if hcx.alloc_id_recursion_tracker.insert(*self) { - tcx - .interpret_interner - .get_corresponding_static_def_id(*self) - .hash_stable(hcx, hasher); + trace!("hashing {:?} as alloc {:#?}", *self, alloc); alloc.hash_stable(hcx, hasher); assert!(hcx.alloc_id_recursion_tracker.remove(self)); + } else { + trace!("skipping hashing of {:?} due to recursion", *self); } } else if let Some(inst) = tcx.interpret_interner.get_fn(*self) { + trace!("hashing {:?} as fn {:#?}", *self, inst); AllocDiscriminant::Function.hash_stable(hcx, hasher); inst.hash_stable(hcx, hasher); - } else if let Some(def_id) = tcx.interpret_interner - .get_corresponding_static_def_id(*self) { - AllocDiscriminant::ExternStatic.hash_stable(hcx, hasher); - def_id.hash_stable(hcx, hasher); } else { bug!("no allocation for {}", self); } diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index e242ec4985ab4..9003cca815ee3 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -154,10 +154,12 @@ pub struct AllocId(pub u64); impl ::rustc_serialize::UseSpecializedEncodable for AllocId {} impl ::rustc_serialize::UseSpecializedDecodable for AllocId {} -pub const ALLOC_DISCRIMINANT: usize = 0; -pub const FN_DISCRIMINANT: usize = 1; -pub const EXTERN_STATIC_DISCRIMINANT: usize = 2; -pub const SHORTHAND_START: usize = 3; +#[derive(RustcDecodable, RustcEncodable)] +enum AllocKind { + Alloc, + Fn, + ExternStatic, +} pub fn specialized_encode_alloc_id< 'a, 'tcx, @@ -166,14 +168,10 @@ pub fn specialized_encode_alloc_id< encoder: &mut E, tcx: TyCtxt<'a, 'tcx, 'tcx>, alloc_id: AllocId, - shorthand: Option, ) -> Result<(), E::Error> { - if let Some(shorthand) = shorthand { - return shorthand.encode(encoder); - } if let Some(alloc) = tcx.interpret_interner.get_alloc(alloc_id) { trace!("encoding {:?} with {:#?}", alloc_id, alloc); - ALLOC_DISCRIMINANT.encode(encoder)?; + AllocKind::Alloc.encode(encoder)?; alloc.encode(encoder)?; // encode whether this allocation is the root allocation of a static tcx.interpret_interner @@ -181,11 +179,11 @@ pub fn specialized_encode_alloc_id< .encode(encoder)?; } else if let Some(fn_instance) = tcx.interpret_interner.get_fn(alloc_id) { trace!("encoding {:?} with {:#?}", alloc_id, fn_instance); - FN_DISCRIMINANT.encode(encoder)?; + AllocKind::Fn.encode(encoder)?; fn_instance.encode(encoder)?; } else if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) { // extern "C" statics don't have allocations, just encode its def_id - EXTERN_STATIC_DISCRIMINANT.encode(encoder)?; + AllocKind::ExternStatic.encode(encoder)?; did.encode(encoder)?; } else { bug!("alloc id without corresponding allocation: {}", alloc_id); @@ -196,21 +194,18 @@ pub fn specialized_encode_alloc_id< pub fn specialized_decode_alloc_id< 'a, 'tcx, D: Decoder, - CACHE: FnOnce(&mut D, usize, AllocId), - SHORT: FnOnce(&mut D, usize) -> Result + CACHE: FnOnce(&mut D, AllocId), >( decoder: &mut D, tcx: TyCtxt<'a, 'tcx, 'tcx>, - pos: usize, cache: CACHE, - short: SHORT, ) -> Result { - match usize::decode(decoder)? { - ALLOC_DISCRIMINANT => { + match AllocKind::decode(decoder)? { + AllocKind::Alloc => { let alloc_id = tcx.interpret_interner.reserve(); - trace!("creating alloc id {:?} at {}", alloc_id, pos); + trace!("creating alloc id {:?}", alloc_id); // insert early to allow recursive allocs - cache(decoder, pos, alloc_id); + cache(decoder, alloc_id); let allocation = Allocation::decode(decoder)?; trace!("decoded alloc {:?} {:#?}", alloc_id, allocation); @@ -223,26 +218,23 @@ pub fn specialized_decode_alloc_id< Ok(alloc_id) }, - FN_DISCRIMINANT => { - trace!("creating fn alloc id at {}", pos); + AllocKind::Fn => { + trace!("creating fn alloc id"); let instance = ty::Instance::decode(decoder)?; trace!("decoded fn alloc instance: {:?}", instance); let id = tcx.interpret_interner.create_fn_alloc(instance); trace!("created fn alloc id: {:?}", id); - cache(decoder, pos, id); + cache(decoder, id); Ok(id) }, - EXTERN_STATIC_DISCRIMINANT => { - trace!("creating extern static alloc id at {}", pos); + AllocKind::ExternStatic => { + trace!("creating extern static alloc id at"); let did = DefId::decode(decoder)?; let alloc_id = tcx.interpret_interner.reserve(); + cache(decoder, alloc_id); tcx.interpret_interner.cache(did, alloc_id); Ok(alloc_id) }, - shorthand => { - trace!("loading shorthand {}", shorthand); - short(decoder, shorthand) - }, } } diff --git a/src/librustc/ty/maps/on_disk_cache.rs b/src/librustc/ty/maps/on_disk_cache.rs index 9ea4b21c55221..62f2cd88935d3 100644 --- a/src/librustc/ty/maps/on_disk_cache.rs +++ b/src/librustc/ty/maps/on_disk_cache.rs @@ -77,12 +77,11 @@ pub struct OnDiskCache<'sess> { // `serialized_data`. prev_diagnostics_index: FxHashMap, - // A cache to ensure we don't read allocations twice - interpret_alloc_cache: RefCell>, + // Alloc indices to memory location map + prev_interpret_alloc_index: Vec, - // A map from positions to size of the serialized allocation - // so we can skip over already processed allocations - interpret_alloc_size: RefCell>, + /// Deserialization: A cache to ensure we don't read allocations twice + interpret_alloc_cache: RefCell>, } // This type is used only for (de-)serialization. @@ -92,6 +91,8 @@ struct Footer { prev_cnums: Vec<(u32, String, CrateDisambiguator)>, query_result_index: EncodedQueryResultIndex, diagnostics_index: EncodedQueryResultIndex, + // the location of all allocations + interpret_alloc_index: Vec, } type EncodedQueryResultIndex = Vec<(SerializedDepNodeIndex, AbsoluteBytePos)>; @@ -148,8 +149,8 @@ impl<'sess> OnDiskCache<'sess> { query_result_index: footer.query_result_index.into_iter().collect(), prev_diagnostics_index: footer.diagnostics_index.into_iter().collect(), synthetic_expansion_infos: Lock::new(FxHashMap()), + prev_interpret_alloc_index: footer.interpret_alloc_index, interpret_alloc_cache: RefCell::new(FxHashMap::default()), - interpret_alloc_size: RefCell::new(FxHashMap::default()), } } @@ -165,8 +166,8 @@ impl<'sess> OnDiskCache<'sess> { query_result_index: FxHashMap(), prev_diagnostics_index: FxHashMap(), synthetic_expansion_infos: Lock::new(FxHashMap()), + prev_interpret_alloc_index: Vec::new(), interpret_alloc_cache: RefCell::new(FxHashMap::default()), - interpret_alloc_size: RefCell::new(FxHashMap::default()), } } @@ -199,7 +200,9 @@ impl<'sess> OnDiskCache<'sess> { type_shorthands: FxHashMap(), predicate_shorthands: FxHashMap(), expn_info_shorthands: FxHashMap(), - interpret_alloc_shorthands: FxHashMap(), + interpret_allocs: FxHashMap(), + interpret_alloc_ids: FxHashSet(), + interpret_allocs_inverse: Vec::new(), codemap: CachingCodemapView::new(tcx.sess.codemap()), file_to_file_index, }; @@ -277,6 +280,31 @@ impl<'sess> OnDiskCache<'sess> { diagnostics_index }; + let interpret_alloc_index = { + let mut interpret_alloc_index = Vec::new(); + let mut n = 0; + loop { + let new_n = encoder.interpret_alloc_ids.len(); + for idx in n..new_n { + let id = encoder.interpret_allocs_inverse[idx]; + let pos = AbsoluteBytePos::new(encoder.position()); + interpret_alloc_index.push(pos); + interpret::specialized_encode_alloc_id( + &mut encoder, + tcx, + id, + )?; + } + // if we have found new ids, serialize those, too + if n == new_n { + // otherwise, abort + break; + } + n = new_n; + } + interpret_alloc_index + }; + let sorted_cnums = sorted_cnums_including_local_crate(tcx); let prev_cnums: Vec<_> = sorted_cnums.iter().map(|&cnum| { let crate_name = tcx.original_crate_name(cnum).as_str().to_string(); @@ -291,6 +319,7 @@ impl<'sess> OnDiskCache<'sess> { prev_cnums, query_result_index, diagnostics_index, + interpret_alloc_index, })?; // Encode the position of the footer as the last 8 bytes of the @@ -396,8 +425,8 @@ impl<'sess> OnDiskCache<'sess> { file_index_to_file: &self.file_index_to_file, file_index_to_stable_id: &self.file_index_to_stable_id, synthetic_expansion_infos: &self.synthetic_expansion_infos, + prev_interpret_alloc_index: &self.prev_interpret_alloc_index, interpret_alloc_cache: &self.interpret_alloc_cache, - interpret_alloc_size: &self.interpret_alloc_size, }; match decode_tagged(&mut decoder, dep_node_index) { @@ -460,7 +489,8 @@ struct CacheDecoder<'a, 'tcx: 'a, 'x> { file_index_to_file: &'x Lock>>, file_index_to_stable_id: &'x FxHashMap, interpret_alloc_cache: &'x RefCell>, - interpret_alloc_size: &'x RefCell>, + /// maps from index in the cache file to location in the cache file + prev_interpret_alloc_index: &'x [AbsoluteBytePos], } impl<'a, 'tcx, 'x> CacheDecoder<'a, 'tcx, 'x> { @@ -584,36 +614,29 @@ implement_ty_decoder!( CacheDecoder<'a, 'tcx, 'x> ); impl<'a, 'tcx, 'x> SpecializedDecoder for CacheDecoder<'a, 'tcx, 'x> { fn specialized_decode(&mut self) -> Result { let tcx = self.tcx; - let pos = TyDecoder::position(self); - trace!("specialized_decode_alloc_id: {:?}", pos); - if let Some(cached) = self.interpret_alloc_cache.borrow().get(&pos).cloned() { - // if there's no end position we are currently deserializing a recursive - // allocation - if let Some(end) = self.interpret_alloc_size.borrow().get(&pos).cloned() { - trace!("{} already cached as {:?}", pos, cached); - // skip ahead - self.opaque.set_position(end); - return Ok(cached) - } + let idx = usize::decode(self)?; + trace!("loading index {}", idx); + + if let Some(cached) = self.interpret_alloc_cache.borrow().get(&idx).cloned() { + trace!("loading alloc id {:?} from alloc_cache", cached); + return Ok(cached); } - let id = interpret::specialized_decode_alloc_id( - self, - tcx, - pos, - |this, pos, alloc_id| { - assert!(this.interpret_alloc_cache.borrow_mut().insert(pos, alloc_id).is_none()); - }, - |this, shorthand| { - // need to load allocation - this.with_position(shorthand, |this| interpret::AllocId::decode(this)) - } - )?; - assert!(self - .interpret_alloc_size - .borrow_mut() - .insert(pos, TyDecoder::position(self)) - .is_none()); - Ok(id) + let pos = self.prev_interpret_alloc_index[idx].to_usize(); + trace!("loading position {}", pos); + self.with_position(pos, |this| { + interpret::specialized_decode_alloc_id( + this, + tcx, + |this, alloc_id| { + trace!("caching idx {} for alloc id {} at position {}", idx, alloc_id, pos); + assert!(this + .interpret_alloc_cache + .borrow_mut() + .insert(idx, alloc_id) + .is_none()); + }, + ) + }) } } impl<'a, 'tcx, 'x> SpecializedDecoder for CacheDecoder<'a, 'tcx, 'x> { @@ -777,7 +800,9 @@ struct CacheEncoder<'enc, 'a, 'tcx, E> type_shorthands: FxHashMap, usize>, predicate_shorthands: FxHashMap, usize>, expn_info_shorthands: FxHashMap, - interpret_alloc_shorthands: FxHashMap, + interpret_allocs: FxHashMap, + interpret_allocs_inverse: Vec, + interpret_alloc_ids: FxHashSet, codemap: CachingCodemapView<'tcx>, file_to_file_index: FxHashMap<*const FileMap, FileMapIndex>, } @@ -814,27 +839,17 @@ impl<'enc, 'a, 'tcx, E> SpecializedEncoder for CacheEncoder< where E: 'enc + ty_codec::TyEncoder { fn specialized_encode(&mut self, alloc_id: &interpret::AllocId) -> Result<(), Self::Error> { - use std::collections::hash_map::Entry; - let tcx = self.tcx; - let pos = self.position(); - let shorthand = match self.interpret_alloc_shorthands.entry(*alloc_id) { - Entry::Occupied(entry) => Some(entry.get().clone()), - Entry::Vacant(entry) => { - // ensure that we don't place any AllocIds at the very beginning - // of the metadata file, because that would end up making our indices - // not special. It is essentially impossible for that to happen, - // but let's make sure - assert!(pos >= interpret::SHORTHAND_START); - entry.insert(pos); - None - }, + let index = if self.interpret_alloc_ids.insert(*alloc_id) { + let idx = self.interpret_alloc_ids.len() - 1; + assert_eq!(idx, self.interpret_allocs_inverse.len()); + self.interpret_allocs_inverse.push(*alloc_id); + assert!(self.interpret_allocs.insert(*alloc_id, idx).is_none()); + idx + } else { + self.interpret_allocs[alloc_id] }; - interpret::specialized_encode_alloc_id( - self, - tcx, - *alloc_id, - shorthand, - ) + + index.encode(self) } } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 3ea4ddc25226f..9173d12827446 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -59,9 +59,6 @@ pub struct DecodeContext<'a, 'tcx: 'a> { // interpreter allocation cache interpret_alloc_cache: FxHashMap, - // a cache for sizes of interpreter allocations - // needed to skip already deserialized allocations - interpret_alloc_size: FxHashMap, } /// Abstract over the various ways one can create metadata decoders. @@ -81,7 +78,6 @@ pub trait Metadata<'a, 'tcx>: Copy { last_filemap_index: 0, lazy_state: LazyState::NoNode, interpret_alloc_cache: FxHashMap::default(), - interpret_alloc_size: FxHashMap::default(), } } } @@ -290,34 +286,25 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { fn specialized_decode(&mut self) -> Result { - let tcx = self.tcx.expect("need tcx for AllocId decoding"); - let pos = self.position(); - if let Some(cached) = self.interpret_alloc_cache.get(&pos).cloned() { - // if there's no end position we are currently deserializing a recursive - // allocation - if let Some(end) = self.interpret_alloc_size.get(&pos).cloned() { - trace!("{} already cached as {:?}", pos, cached); - // skip ahead - self.opaque.set_position(end); - return Ok(cached) - } + let tcx = self.tcx.unwrap(); + let idx = usize::decode(self)?; + + if let Some(cached) = self.interpret_alloc_cache.get(&idx).cloned() { + return Ok(cached); } - let id = interpret::specialized_decode_alloc_id( - self, - tcx, - pos, - |this, pos, alloc_id| { this.interpret_alloc_cache.insert(pos, alloc_id); }, - |this, shorthand| { - // need to load allocation - this.with_position(shorthand, |this| interpret::AllocId::decode(this)) - } - )?; - let end_pos = self.position(); - assert!(self - .interpret_alloc_size - .insert(pos, end_pos) - .is_none()); - Ok(id) + let pos = self + .cdata() + .root + .interpret_alloc_index[idx]; + self.with_position(pos as usize, |this| { + interpret::specialized_decode_alloc_id( + this, + tcx, + |this, alloc_id| { + assert!(this.interpret_alloc_cache.insert(idx, alloc_id).is_none()); + }, + ) + }) } } diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 22b440eea60ef..cc2d0eab2331e 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -29,7 +29,7 @@ use rustc::ty::{self, Ty, TyCtxt, ReprOptions, SymbolName}; use rustc::ty::codec::{self as ty_codec, TyEncoder}; use rustc::session::config::{self, CrateTypeProcMacro}; -use rustc::util::nodemap::FxHashMap; +use rustc::util::nodemap::{FxHashMap, FxHashSet}; use rustc_data_structures::stable_hasher::StableHasher; use rustc_serialize::{Encodable, Encoder, SpecializedEncoder, opaque}; @@ -59,7 +59,10 @@ pub struct EncodeContext<'a, 'tcx: 'a> { lazy_state: LazyState, type_shorthands: FxHashMap, usize>, predicate_shorthands: FxHashMap, usize>, - interpret_alloc_shorthands: FxHashMap, + + interpret_allocs: FxHashMap, + interpret_allocs_inverse: Vec, + interpret_alloc_ids: FxHashSet, // This is used to speed up Span encoding. filemap_cache: Lrc, @@ -196,26 +199,17 @@ impl<'a, 'tcx> SpecializedEncoder> for EncodeContext<'a, 'tcx> { impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { fn specialized_encode(&mut self, alloc_id: &interpret::AllocId) -> Result<(), Self::Error> { - use std::collections::hash_map::Entry; - let tcx = self.tcx; - let pos = self.position(); - let shorthand = match self.interpret_alloc_shorthands.entry(*alloc_id) { - Entry::Occupied(entry) => Some(entry.get().clone()), - Entry::Vacant(entry) => { - // ensure that we don't place any AllocIds at the very beginning - // of the metadata file, because that would end up making our indices - // not special. This is essentially impossible, but let's make sure - assert!(pos >= interpret::SHORTHAND_START); - entry.insert(pos); - None - }, + let index = if self.interpret_alloc_ids.insert(*alloc_id) { + let idx = self.interpret_alloc_ids.len() - 1; + assert_eq!(idx, self.interpret_allocs_inverse.len()); + self.interpret_allocs_inverse.push(*alloc_id); + assert!(self.interpret_allocs.insert(*alloc_id, idx).is_none()); + idx + } else { + self.interpret_allocs[alloc_id] }; - interpret::specialized_encode_alloc_id( - self, - tcx, - *alloc_id, - shorthand, - ) + + index.encode(self) } } @@ -460,6 +454,33 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let has_default_lib_allocator = attr::contains_name(tcx.hir.krate_attrs(), "default_lib_allocator"); let has_global_allocator = *tcx.sess.has_global_allocator.get(); + + // Encode the allocation index + let interpret_alloc_index = { + let mut interpret_alloc_index = Vec::new(); + let mut n = 0; + loop { + let new_n = self.interpret_alloc_ids.len(); + for idx in n..new_n { + let id = self.interpret_allocs_inverse[idx]; + let pos = self.position() as u32; + interpret_alloc_index.push(pos); + interpret::specialized_encode_alloc_id( + self, + tcx, + id, + ).unwrap(); + } + // if we have found new ids, serialize those, too + if n == new_n { + // otherwise, abort + break; + } + n = new_n; + } + interpret_alloc_index + }; + let root = self.lazy(&CrateRoot { name: tcx.crate_name(LOCAL_CRATE), extra_filename: tcx.sess.opts.cg.extra_filename.clone(), @@ -492,6 +513,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { exported_symbols, wasm_custom_sections, index, + interpret_alloc_index, }); let total_bytes = self.position(); @@ -1760,7 +1782,9 @@ pub fn encode_metadata<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, type_shorthands: Default::default(), predicate_shorthands: Default::default(), filemap_cache: tcx.sess.codemap().files()[0].clone(), - interpret_alloc_shorthands: Default::default(), + interpret_allocs: Default::default(), + interpret_allocs_inverse: Default::default(), + interpret_alloc_ids: Default::default(), }; // Encode the rustc version string in a predictable location. diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index e3986bb7d91f9..4eaf08742ecae 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -207,6 +207,7 @@ pub struct CrateRoot { pub impls: LazySeq, pub exported_symbols: EncodedExportedSymbols, pub wasm_custom_sections: LazySeq, + pub interpret_alloc_index: Vec, pub index: LazySeq, } diff --git a/src/test/incremental/static_cycle/b.rs b/src/test/incremental/static_cycle/b.rs new file mode 100644 index 0000000000000..b659703bef004 --- /dev/null +++ b/src/test/incremental/static_cycle/b.rs @@ -0,0 +1,19 @@ +// Copyright 2014 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. + +// revisions:rpass1 rpass2 + +#![cfg_attr(rpass2, warn(dead_code))] + +pub static mut BAA: *const i8 = unsafe { &BOO as *const _ as *const i8 }; + +pub static mut BOO: *const i8 = unsafe { &BAA as *const _ as *const i8 }; + +fn main() {} From 62c0501be8a0fa5e511a45ed9ffbcef42094e9ad Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 10 Apr 2018 16:25:10 +0200 Subject: [PATCH 2/6] Stop referring to statics' AllocIds directly --- src/librustc/ich/impls_ty.rs | 3 +- src/librustc/mir/interpret/mod.rs | 21 +--- src/librustc/ty/context.rs | 39 +++--- src/librustc_mir/interpret/const_eval.rs | 136 +++++---------------- src/librustc_mir/interpret/eval_context.rs | 14 +-- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_trans/mir/constant.rs | 15 ++- 7 files changed, 68 insertions(+), 162 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 2425fef7e7f53..99ac5869b2969 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -416,8 +416,7 @@ impl<'a> HashStable> for mir::interpret::AllocId { ty::tls::with_opt(|tcx| { trace!("hashing {:?}", *self); let tcx = tcx.expect("can't hash AllocIds during hir lowering"); - if let Some(def_id) = tcx.interpret_interner - .get_corresponding_static_def_id(*self) { + if let Some(def_id) = tcx.interpret_interner.get_static(*self) { AllocDiscriminant::Static.hash_stable(hcx, hasher); trace!("hashing {:?} as static {:?}", *self, def_id); def_id.hash_stable(hcx, hasher); diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 9003cca815ee3..07768ac3a3b97 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -158,7 +158,7 @@ impl ::rustc_serialize::UseSpecializedDecodable for AllocId {} enum AllocKind { Alloc, Fn, - ExternStatic, + Static, } pub fn specialized_encode_alloc_id< @@ -173,17 +173,13 @@ pub fn specialized_encode_alloc_id< trace!("encoding {:?} with {:#?}", alloc_id, alloc); AllocKind::Alloc.encode(encoder)?; alloc.encode(encoder)?; - // encode whether this allocation is the root allocation of a static - tcx.interpret_interner - .get_corresponding_static_def_id(alloc_id) - .encode(encoder)?; } else if let Some(fn_instance) = tcx.interpret_interner.get_fn(alloc_id) { trace!("encoding {:?} with {:#?}", alloc_id, fn_instance); AllocKind::Fn.encode(encoder)?; fn_instance.encode(encoder)?; - } else if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) { - // extern "C" statics don't have allocations, just encode its def_id - AllocKind::ExternStatic.encode(encoder)?; + } else if let Some(did) = tcx.interpret_interner.get_static(alloc_id) { + // referring to statics doesn't need to know about their allocations, just hash the DefId + AllocKind::Static.encode(encoder)?; did.encode(encoder)?; } else { bug!("alloc id without corresponding allocation: {}", alloc_id); @@ -212,10 +208,6 @@ pub fn specialized_decode_alloc_id< let allocation = tcx.intern_const_alloc(allocation); tcx.interpret_interner.intern_at_reserved(alloc_id, allocation); - if let Some(glob) = Option::::decode(decoder)? { - tcx.interpret_interner.cache(glob, alloc_id); - } - Ok(alloc_id) }, AllocKind::Fn => { @@ -227,12 +219,11 @@ pub fn specialized_decode_alloc_id< cache(decoder, id); Ok(id) }, - AllocKind::ExternStatic => { + AllocKind::Static => { trace!("creating extern static alloc id at"); let did = DefId::decode(decoder)?; - let alloc_id = tcx.interpret_interner.reserve(); + let alloc_id = tcx.interpret_interner.cache_static(did); cache(decoder, alloc_id); - tcx.interpret_interner.cache(did, alloc_id); Ok(alloc_id) }, } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index a508f33db3f77..ed3332f32d02f 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -956,18 +956,16 @@ struct InterpretInternerInner<'tcx> { /// Allows obtaining const allocs via a unique identifier alloc_by_id: FxHashMap, - /// Reverse map of `alloc_cache` - global_cache: FxHashMap, + /// Allows obtaining static def ids via a unique id + statics: FxHashMap, /// The AllocId to assign to the next new regular allocation. /// Always incremented, never gets smaller. next_id: interpret::AllocId, - /// Allows checking whether a static already has an allocation - /// - /// This is only important for detecting statics referring to themselves - // FIXME(oli-obk) move it to the EvalContext? - alloc_cache: FxHashMap, + /// Inverse map of `statics` + /// Used so we don't allocate a new pointer every time we need one + static_cache: FxHashMap, /// A cache for basic byte allocations keyed by their contents. This is used to deduplicate /// allocations for string and bytestring literals. @@ -1001,30 +999,25 @@ impl<'tcx> InterpretInterner<'tcx> { self.inner.borrow().alloc_by_id.get(&id).cloned() } - pub fn get_cached( - &self, - static_id: DefId, - ) -> Option { - self.inner.borrow().alloc_cache.get(&static_id).cloned() - } - - pub fn cache( + pub fn cache_static( &self, static_id: DefId, - alloc_id: interpret::AllocId, - ) { - let mut inner = self.inner.borrow_mut(); - inner.global_cache.insert(alloc_id, static_id); - if let Some(old) = inner.alloc_cache.insert(static_id, alloc_id) { - bug!("tried to cache {:?}, but was already existing as {:#?}", static_id, old); + ) -> interpret::AllocId { + if let Some(alloc_id) = self.inner.borrow().static_cache.get(&static_id).cloned() { + return alloc_id; } + let alloc_id = self.reserve(); + let mut inner = self.inner.borrow_mut(); + inner.static_cache.insert(static_id, alloc_id); + inner.statics.insert(alloc_id, static_id); + alloc_id } - pub fn get_corresponding_static_def_id( + pub fn get_static( &self, ptr: interpret::AllocId, ) -> Option { - self.inner.borrow().global_cache.get(&ptr).cloned() + self.inner.borrow().statics.get(&ptr).cloned() } pub fn intern_at_reserved( diff --git a/src/librustc_mir/interpret/const_eval.rs b/src/librustc_mir/interpret/const_eval.rs index 57977b6201a61..954a3dbe5b9ab 100644 --- a/src/librustc_mir/interpret/const_eval.rs +++ b/src/librustc_mir/interpret/const_eval.rs @@ -5,7 +5,6 @@ use rustc::mir; use rustc::ty::{self, TyCtxt, Ty, Instance}; use rustc::ty::layout::{self, LayoutOf}; use rustc::ty::subst::Subst; -use rustc::util::nodemap::FxHashSet; use syntax::ast::Mutability; use syntax::codemap::Span; @@ -110,53 +109,38 @@ fn eval_body_and_ecx<'a, 'mir, 'tcx>( } span = mir.span; let layout = ecx.layout_of(mir.return_ty().subst(tcx, cid.instance.substs))?; - let alloc = tcx.interpret_interner.get_cached(cid.instance.def_id()); - let is_static = tcx.is_static(cid.instance.def_id()).is_some(); - let alloc = match alloc { - Some(alloc) => { - assert!(cid.promoted.is_none()); - assert!(param_env.caller_bounds.is_empty()); - alloc - }, - None => { - assert!(!layout.is_unsized()); - let ptr = ecx.memory.allocate( - layout.size.bytes(), - layout.align, - None, - )?; - if is_static { - tcx.interpret_interner.cache(cid.instance.def_id(), ptr.alloc_id); - } - let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span); - let mutability = tcx.is_static(cid.instance.def_id()); - let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable { - Mutability::Mutable - } else { - Mutability::Immutable - }; - let cleanup = StackPopCleanup::MarkStatic(mutability); - let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id())); - let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p)); - trace!("const_eval: pushing stack frame for global: {}{}", name, prom); - assert!(mir.arg_count == 0); - ecx.push_stack_frame( - cid.instance, - mir.span, - mir, - Place::from_ptr(ptr, layout.align), - cleanup, - )?; - - while ecx.step()? {} - ptr.alloc_id - } + assert!(!layout.is_unsized()); + let ptr = ecx.memory.allocate( + layout.size.bytes(), + layout.align, + None, + )?; + let internally_mutable = !layout.ty.is_freeze(tcx, param_env, mir.span); + let mutability = tcx.is_static(cid.instance.def_id()); + let mutability = if mutability == Some(hir::Mutability::MutMutable) || internally_mutable { + Mutability::Mutable + } else { + Mutability::Immutable }; - let ptr = MemoryPointer::new(alloc, 0).into(); + let cleanup = StackPopCleanup::MarkStatic(mutability); + let name = ty::tls::with(|tcx| tcx.item_path_str(cid.instance.def_id())); + let prom = cid.promoted.map_or(String::new(), |p| format!("::promoted[{:?}]", p)); + trace!("const_eval: pushing stack frame for global: {}{}", name, prom); + assert!(mir.arg_count == 0); + ecx.push_stack_frame( + cid.instance, + mir.span, + mir, + Place::from_ptr(ptr, layout.align), + cleanup, + )?; + + while ecx.step()? {} + let ptr = ptr.into(); // always try to read the value and report errors let value = match ecx.try_read_value(ptr, layout.align, layout.ty)? { // if it's a constant (so it needs no address, directly compute its value) - Some(val) if !is_static => val, + Some(val) if tcx.is_static(cid.instance.def_id()).is_none() => val, // point at the allocation _ => Value::ByRef(ptr, layout.align), }; @@ -340,21 +324,10 @@ impl<'mir, 'tcx> super::Machine<'mir, 'tcx> for CompileTimeEvaluator { ecx: &mut EvalContext<'a, 'mir, 'tcx, Self>, cid: GlobalId<'tcx>, ) -> EvalResult<'tcx, AllocId> { - let alloc = ecx - .tcx - .interpret_interner - .get_cached(cid.instance.def_id()); - // Don't evaluate when already cached to prevent cycles - if let Some(alloc) = alloc { - return Ok(alloc) - } - // ensure the static is computed - ecx.const_eval(cid)?; Ok(ecx .tcx .interpret_interner - .get_cached(cid.instance.def_id()) - .expect("uncached static")) + .cache_static(cid.instance.def_id())) } fn box_alloc<'a>( @@ -460,16 +433,7 @@ pub fn const_eval_provider<'a, 'tcx>( let def_id = cid.instance.def.def_id(); if tcx.is_foreign_item(def_id) { - let id = tcx.interpret_interner.get_cached(def_id); - let id = match id { - // FIXME: due to caches this shouldn't happen, add some assertions - Some(id) => id, - None => { - let id = tcx.interpret_interner.reserve(); - tcx.interpret_interner.cache(def_id, id); - id - }, - }; + let id = tcx.interpret_interner.cache_static(def_id); let ty = tcx.type_of(def_id); let layout = tcx.layout_of(key.param_env.and(ty)).unwrap(); let ptr = MemoryPointer::new(id, 0); @@ -505,13 +469,7 @@ pub fn const_eval_provider<'a, 'tcx>( }; let (res, ecx) = eval_body_and_ecx(tcx, cid, None, key.param_env); - res.map(|(miri_value, ptr, miri_ty)| { - if tcx.is_static(def_id).is_some() { - if let Ok(ptr) = ptr.primval.to_ptr() { - let mut seen = FxHashSet::default(); - create_depgraph_edges(tcx, ptr.alloc_id, &mut seen); - } - } + res.map(|(miri_value, _, miri_ty)| { tcx.mk_const(ty::Const { val: ConstVal::Value(miri_value), ty: miri_ty, @@ -528,35 +486,3 @@ pub fn const_eval_provider<'a, 'tcx>( } }) } - -// This function creates dep graph edges from statics to all referred to statics. -// This is necessary, because the `const_eval` query cannot directly call itself -// for other statics, because we cannot prevent recursion in queries. -// -// see test/incremental/static_refering_to_other_static2/issue.rs for an example -// where not creating those edges would cause static A, which refers to static B -// to point to the old allocation of static B, even though B has changed. -// -// In the future we will want to remove this funcion in favour of a system that -// makes sure that statics don't need to have edges to other statics as long as -// they are only referring by reference and not inspecting the other static's body. -fn create_depgraph_edges<'a, 'tcx>( - tcx: TyCtxt<'a, 'tcx, 'tcx>, - alloc_id: AllocId, - seen: &mut FxHashSet, -) { - trace!("create_depgraph_edges: {:?}, {:?}", alloc_id, seen); - if seen.insert(alloc_id) { - trace!("seen: {:?}, {:?}", alloc_id, seen); - if let Some(alloc) = tcx.interpret_interner.get_alloc(alloc_id) { - trace!("get_alloc: {:?}, {:?}, {:?}", alloc_id, seen, alloc); - for (_, &reloc) in &alloc.relocations { - if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(reloc) { - trace!("get_corresponding: {:?}, {:?}, {:?}, {:?}, {:?}", alloc_id, seen, alloc, did, reloc); - let _ = tcx.maybe_optimized_mir(did); - } - create_depgraph_edges(tcx, reloc, seen); - } - } - } -} diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 58ea8d48e9711..158d674580b53 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -938,16 +938,14 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M } pub fn read_global_as_value(&self, gid: GlobalId<'tcx>, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> { - if gid.promoted.is_none() { - let cached = self + if self.tcx.is_static(gid.instance.def_id()).is_some() { + let alloc_id = self .tcx .interpret_interner - .get_cached(gid.instance.def_id()); - if let Some(alloc_id) = cached { - let layout = self.layout_of(ty)?; - let ptr = MemoryPointer::new(alloc_id, 0); - return Ok(Value::ByRef(ptr.into(), layout.align)) - } + .cache_static(gid.instance.def_id()); + let layout = self.layout_of(ty)?; + let ptr = MemoryPointer::new(alloc_id, 0); + return Ok(Value::ByRef(ptr.into(), layout.align)) } let cv = self.const_eval(gid)?; self.const_to_value(&cv.val, ty) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 83ef28e4f156c..008165f33b2bb 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1142,7 +1142,7 @@ fn collect_miri<'a, 'tcx>( alloc_id: AllocId, output: &mut Vec>, ) { - if let Some(did) = tcx.interpret_interner.get_corresponding_static_def_id(alloc_id) { + if let Some(did) = tcx.interpret_interner.get_static(alloc_id) { let instance = Instance::mono(tcx, did); if should_monomorphize_locally(tcx, &instance) { trace!("collecting static {:?}", did); diff --git a/src/librustc_trans/mir/constant.rs b/src/librustc_trans/mir/constant.rs index 977c7c983d6f2..6e07b8e73ef22 100644 --- a/src/librustc_trans/mir/constant.rs +++ b/src/librustc_trans/mir/constant.rs @@ -50,7 +50,7 @@ pub fn primval_to_llvm(cx: &CodegenCx, let static_ = cx .tcx .interpret_interner - .get_corresponding_static_def_id(ptr.alloc_id); + .get_static(ptr.alloc_id); let base_addr = if let Some(def_id) = static_ { assert!(cx.tcx.is_static(def_id).is_some()); consts::get_static(cx, def_id) @@ -126,18 +126,17 @@ pub fn trans_static_initializer<'a, 'tcx>( promoted: None }; let param_env = ty::ParamEnv::reveal_all(); - cx.tcx.const_eval(param_env.and(cid))?; + let static_ = cx.tcx.const_eval(param_env.and(cid))?; - let alloc_id = cx - .tcx - .interpret_interner - .get_cached(def_id) - .expect("global not cached"); + let ptr = match static_.val { + ConstVal::Value(MiriValue::ByRef(ptr, _)) => ptr, + _ => bug!("static const eval returned {:#?}", static_), + }; let alloc = cx .tcx .interpret_interner - .get_alloc(alloc_id) + .get_alloc(ptr.primval.to_ptr().expect("static has integer pointer").alloc_id) .expect("miri allocation never successfully created"); Ok(global_initializer(cx, alloc)) } From 6f251c2a0318cc4b61c7f9e96113d2e31175d8fc Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 11 Apr 2018 10:47:52 +0200 Subject: [PATCH 3/6] Use `LazySeq` instead of `Vec` --- src/librustc_metadata/decoder.rs | 22 ++++++++++++++---- src/librustc_metadata/encoder.rs | 39 ++++++++++++++++++-------------- src/librustc_metadata/schema.rs | 2 +- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 9173d12827446..c69ee180dc9c9 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -59,6 +59,9 @@ pub struct DecodeContext<'a, 'tcx: 'a> { // interpreter allocation cache interpret_alloc_cache: FxHashMap, + + // Read from the LazySeq CrateRoot::inpterpret_alloc_index on demand + interpret_alloc_index: Option>, } /// Abstract over the various ways one can create metadata decoders. @@ -78,6 +81,7 @@ pub trait Metadata<'a, 'tcx>: Copy { last_filemap_index: 0, lazy_state: LazyState::NoNode, interpret_alloc_cache: FxHashMap::default(), + interpret_alloc_index: None, } } } @@ -176,6 +180,17 @@ impl<'a, 'tcx> DecodeContext<'a, 'tcx> { self.lazy_state = LazyState::Previous(position + min_size); Ok(position) } + + fn interpret_alloc(&mut self, idx: usize) -> usize { + if let Some(index) = self.interpret_alloc_index.as_mut() { + return index[idx] as usize; + } + let index = self.cdata().root.interpret_alloc_index; + let index: Vec = index.decode(self.cdata()).collect(); + let pos = index[idx]; + self.interpret_alloc_index = Some(index); + pos as usize + } } impl<'a, 'tcx: 'a> TyDecoder<'a, 'tcx> for DecodeContext<'a, 'tcx> { @@ -292,11 +307,8 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx if let Some(cached) = self.interpret_alloc_cache.get(&idx).cloned() { return Ok(cached); } - let pos = self - .cdata() - .root - .interpret_alloc_index[idx]; - self.with_position(pos as usize, |this| { + let pos = self.interpret_alloc(idx); + self.with_position(pos, |this| { interpret::specialized_decode_alloc_id( this, tcx, diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index cc2d0eab2331e..d6c673f550d72 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -265,7 +265,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { start - min_end } LazyState::Previous(last_min_end) => { - assert!(last_min_end <= position); + assert!( + last_min_end <= position, + "make sure that the calls to `lazy*` \ + are in the same order as the metadata fields", + ); position - last_min_end } }; @@ -439,21 +443,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { IsolatedEncoder::encode_wasm_custom_sections, &wasm_custom_sections); - // Encode and index the items. - i = self.position(); - let items = self.encode_info_for_items(); - let item_bytes = self.position() - i; - - i = self.position(); - let index = items.write_index(&mut self.opaque.cursor); - let index_bytes = self.position() - i; - let tcx = self.tcx; - let link_meta = self.link_meta; - let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro); - let has_default_lib_allocator = - attr::contains_name(tcx.hir.krate_attrs(), "default_lib_allocator"); - let has_global_allocator = *tcx.sess.has_global_allocator.get(); // Encode the allocation index let interpret_alloc_index = { @@ -478,9 +468,24 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { } n = new_n; } - interpret_alloc_index + self.lazy_seq(interpret_alloc_index) }; + // Encode and index the items. + i = self.position(); + let items = self.encode_info_for_items(); + let item_bytes = self.position() - i; + + i = self.position(); + let index = items.write_index(&mut self.opaque.cursor); + let index_bytes = self.position() - i; + + let link_meta = self.link_meta; + let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro); + let has_default_lib_allocator = + attr::contains_name(tcx.hir.krate_attrs(), "default_lib_allocator"); + let has_global_allocator = tcx.sess.has_global_allocator.get(); + let root = self.lazy(&CrateRoot { name: tcx.crate_name(LOCAL_CRATE), extra_filename: tcx.sess.opts.cg.extra_filename.clone(), @@ -512,8 +517,8 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { impls, exported_symbols, wasm_custom_sections, - index, interpret_alloc_index, + index, }); let total_bytes = self.position(); diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 4eaf08742ecae..23ea5e4cc5504 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -207,7 +207,7 @@ pub struct CrateRoot { pub impls: LazySeq, pub exported_symbols: EncodedExportedSymbols, pub wasm_custom_sections: LazySeq, - pub interpret_alloc_index: Vec, + pub interpret_alloc_index: LazySeq, pub index: LazySeq, } From 04b3ab67d9d30a292e94d3875e5b7ee46fbdb563 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 11 Apr 2018 13:31:37 +0200 Subject: [PATCH 4/6] Encode items before encoding the list of AllocIds --- src/librustc_metadata/encoder.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index d6c673f550d72..5ecfbd7c6fba3 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -445,12 +445,24 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let tcx = self.tcx; + // Encode the items. + i = self.position(); + let items = self.encode_info_for_items(); + let item_bytes = self.position() - i; + // Encode the allocation index let interpret_alloc_index = { let mut interpret_alloc_index = Vec::new(); let mut n = 0; + trace!("beginning to encode alloc ids"); loop { let new_n = self.interpret_alloc_ids.len(); + // if we have found new ids, serialize those, too + if n == new_n { + // otherwise, abort + break; + } + trace!("encoding {} further alloc ids", new_n - n); for idx in n..new_n { let id = self.interpret_allocs_inverse[idx]; let pos = self.position() as u32; @@ -461,21 +473,12 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { id, ).unwrap(); } - // if we have found new ids, serialize those, too - if n == new_n { - // otherwise, abort - break; - } n = new_n; } self.lazy_seq(interpret_alloc_index) }; - // Encode and index the items. - i = self.position(); - let items = self.encode_info_for_items(); - let item_bytes = self.position() - i; - + // Index the items i = self.position(); let index = items.write_index(&mut self.opaque.cursor); let index_bytes = self.position() - i; From 748e71e8f4356ac3ce1313e473aa000118c1e109 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Wed, 11 Apr 2018 13:31:51 +0200 Subject: [PATCH 5/6] Reduce the number of calls to `cdata` --- src/librustc_metadata/decoder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index c69ee180dc9c9..936d680380c99 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -185,8 +185,8 @@ impl<'a, 'tcx> DecodeContext<'a, 'tcx> { if let Some(index) = self.interpret_alloc_index.as_mut() { return index[idx] as usize; } - let index = self.cdata().root.interpret_alloc_index; - let index: Vec = index.decode(self.cdata()).collect(); + let cdata = self.cdata(); + let index: Vec = cdata.root.interpret_alloc_index.decode(cdata).collect(); let pos = index[idx]; self.interpret_alloc_index = Some(index); pos as usize From 7f7d4c376af12f66e09a41d10612dbd0010a6abe Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Fri, 13 Apr 2018 18:48:41 +0200 Subject: [PATCH 6/6] Get rid of redundant `HashSet` --- src/librustc/mir/interpret/mod.rs | 2 +- src/librustc/ty/maps/on_disk_cache.rs | 31 +++++++++++++-------------- src/librustc_metadata/encoder.rs | 25 +++++++++++---------- 3 files changed, 28 insertions(+), 30 deletions(-) diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 07768ac3a3b97..c9eed0e4a2885 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -178,7 +178,7 @@ pub fn specialized_encode_alloc_id< AllocKind::Fn.encode(encoder)?; fn_instance.encode(encoder)?; } else if let Some(did) = tcx.interpret_interner.get_static(alloc_id) { - // referring to statics doesn't need to know about their allocations, just hash the DefId + // referring to statics doesn't need to know about their allocations, just about its DefId AllocKind::Static.encode(encoder)?; did.encode(encoder)?; } else { diff --git a/src/librustc/ty/maps/on_disk_cache.rs b/src/librustc/ty/maps/on_disk_cache.rs index 62f2cd88935d3..d60206ffd327c 100644 --- a/src/librustc/ty/maps/on_disk_cache.rs +++ b/src/librustc/ty/maps/on_disk_cache.rs @@ -201,7 +201,6 @@ impl<'sess> OnDiskCache<'sess> { predicate_shorthands: FxHashMap(), expn_info_shorthands: FxHashMap(), interpret_allocs: FxHashMap(), - interpret_alloc_ids: FxHashSet(), interpret_allocs_inverse: Vec::new(), codemap: CachingCodemapView::new(tcx.sess.codemap()), file_to_file_index, @@ -284,7 +283,12 @@ impl<'sess> OnDiskCache<'sess> { let mut interpret_alloc_index = Vec::new(); let mut n = 0; loop { - let new_n = encoder.interpret_alloc_ids.len(); + let new_n = encoder.interpret_allocs_inverse.len(); + // if we have found new ids, serialize those, too + if n == new_n { + // otherwise, abort + break; + } for idx in n..new_n { let id = encoder.interpret_allocs_inverse[idx]; let pos = AbsoluteBytePos::new(encoder.position()); @@ -295,11 +299,6 @@ impl<'sess> OnDiskCache<'sess> { id, )?; } - // if we have found new ids, serialize those, too - if n == new_n { - // otherwise, abort - break; - } n = new_n; } interpret_alloc_index @@ -802,7 +801,6 @@ struct CacheEncoder<'enc, 'a, 'tcx, E> expn_info_shorthands: FxHashMap, interpret_allocs: FxHashMap, interpret_allocs_inverse: Vec, - interpret_alloc_ids: FxHashSet, codemap: CachingCodemapView<'tcx>, file_to_file_index: FxHashMap<*const FileMap, FileMapIndex>, } @@ -839,14 +837,15 @@ impl<'enc, 'a, 'tcx, E> SpecializedEncoder for CacheEncoder< where E: 'enc + ty_codec::TyEncoder { fn specialized_encode(&mut self, alloc_id: &interpret::AllocId) -> Result<(), Self::Error> { - let index = if self.interpret_alloc_ids.insert(*alloc_id) { - let idx = self.interpret_alloc_ids.len() - 1; - assert_eq!(idx, self.interpret_allocs_inverse.len()); - self.interpret_allocs_inverse.push(*alloc_id); - assert!(self.interpret_allocs.insert(*alloc_id, idx).is_none()); - idx - } else { - self.interpret_allocs[alloc_id] + use std::collections::hash_map::Entry; + let index = match self.interpret_allocs.entry(*alloc_id) { + Entry::Occupied(e) => *e.get(), + Entry::Vacant(e) => { + let idx = self.interpret_allocs_inverse.len(); + self.interpret_allocs_inverse.push(*alloc_id); + e.insert(idx); + idx + }, }; index.encode(self) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 5ecfbd7c6fba3..a61428b841fea 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -29,7 +29,7 @@ use rustc::ty::{self, Ty, TyCtxt, ReprOptions, SymbolName}; use rustc::ty::codec::{self as ty_codec, TyEncoder}; use rustc::session::config::{self, CrateTypeProcMacro}; -use rustc::util::nodemap::{FxHashMap, FxHashSet}; +use rustc::util::nodemap::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; use rustc_serialize::{Encodable, Encoder, SpecializedEncoder, opaque}; @@ -62,7 +62,6 @@ pub struct EncodeContext<'a, 'tcx: 'a> { interpret_allocs: FxHashMap, interpret_allocs_inverse: Vec, - interpret_alloc_ids: FxHashSet, // This is used to speed up Span encoding. filemap_cache: Lrc, @@ -199,14 +198,15 @@ impl<'a, 'tcx> SpecializedEncoder> for EncodeContext<'a, 'tcx> { impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { fn specialized_encode(&mut self, alloc_id: &interpret::AllocId) -> Result<(), Self::Error> { - let index = if self.interpret_alloc_ids.insert(*alloc_id) { - let idx = self.interpret_alloc_ids.len() - 1; - assert_eq!(idx, self.interpret_allocs_inverse.len()); - self.interpret_allocs_inverse.push(*alloc_id); - assert!(self.interpret_allocs.insert(*alloc_id, idx).is_none()); - idx - } else { - self.interpret_allocs[alloc_id] + use std::collections::hash_map::Entry; + let index = match self.interpret_allocs.entry(*alloc_id) { + Entry::Occupied(e) => *e.get(), + Entry::Vacant(e) => { + let idx = self.interpret_allocs_inverse.len(); + self.interpret_allocs_inverse.push(*alloc_id); + e.insert(idx); + idx + }, }; index.encode(self) @@ -456,7 +456,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let mut n = 0; trace!("beginning to encode alloc ids"); loop { - let new_n = self.interpret_alloc_ids.len(); + let new_n = self.interpret_allocs_inverse.len(); // if we have found new ids, serialize those, too if n == new_n { // otherwise, abort @@ -487,7 +487,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let is_proc_macro = tcx.sess.crate_types.borrow().contains(&CrateTypeProcMacro); let has_default_lib_allocator = attr::contains_name(tcx.hir.krate_attrs(), "default_lib_allocator"); - let has_global_allocator = tcx.sess.has_global_allocator.get(); + let has_global_allocator = *tcx.sess.has_global_allocator.get(); let root = self.lazy(&CrateRoot { name: tcx.crate_name(LOCAL_CRATE), @@ -1792,7 +1792,6 @@ pub fn encode_metadata<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, filemap_cache: tcx.sess.codemap().files()[0].clone(), interpret_allocs: Default::default(), interpret_allocs_inverse: Default::default(), - interpret_alloc_ids: Default::default(), }; // Encode the rustc version string in a predictable location.