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.

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

Use 128 bit instead of Symbol for crate disambiguator #45476

Merged
merged 2 commits into from
Oct 25, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 2 additions & 1 deletion src/librustc/hir/map/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use super::*;

use dep_graph::{DepGraph, DepKind, DepNodeIndex};
use ich::Fingerprint;
use hir::intravisit::{Visitor, NestedVisitorMap};
use std::iter::repeat;
use syntax::ast::{NodeId, CRATE_NODE_ID};
Expand Down Expand Up @@ -118,7 +119,7 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
}

pub(super) fn finalize_and_compute_crate_hash(self,
crate_disambiguator: &str)
crate_disambiguator: &Fingerprint)
Copy link
Member

Choose a reason for hiding this comment

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

Usually we pass around Fingerprint by value. It implements Copy so that should be easy.

-> Vec<MapEntry<'hir>> {
let mut node_hashes: Vec<_> = self
.hir_body_nodes
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,7 @@ pub fn map_crate<'hir>(sess: &::session::Session,
hcx);
intravisit::walk_crate(&mut collector, &forest.krate);

let crate_disambiguator = sess.local_crate_disambiguator().as_str();
let crate_disambiguator = sess.local_crate_disambiguator();
collector.finalize_and_compute_crate_hash(&crate_disambiguator)
};

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ pub trait CrateStore {
fn export_macros_untracked(&self, cnum: CrateNum);
fn dep_kind_untracked(&self, cnum: CrateNum) -> DepKind;
fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol;
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> Symbol;
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> ich::Fingerprint;
fn crate_hash_untracked(&self, cnum: CrateNum) -> Svh;
fn struct_field_names_untracked(&self, def: DefId) -> Vec<ast::Name>;
fn item_children_untracked(&self, did: DefId, sess: &Session) -> Vec<def::Export>;
Expand Down Expand Up @@ -338,7 +338,7 @@ impl CrateStore for DummyCrateStore {
fn dep_kind_untracked(&self, cnum: CrateNum) -> DepKind { bug!("is_explicitly_linked") }
fn export_macros_untracked(&self, cnum: CrateNum) { bug!("export_macros") }
fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol { bug!("crate_name") }
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> Symbol {
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> ich::Fingerprint {
bug!("crate_disambiguator")
}
fn crate_hash_untracked(&self, cnum: CrateNum) -> Svh { bug!("crate_hash") }
Expand Down
17 changes: 10 additions & 7 deletions src/librustc/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub use self::code_stats::{CodeStats, DataTypeKind, FieldInfo};
pub use self::code_stats::{SizeKind, TypeSizeInfo, VariantInfo};

use hir::def_id::{CrateNum, DefIndex};
use ich::Fingerprint;

use lint;
use middle::allocator::AllocatorKind;
Expand All @@ -29,7 +30,6 @@ use syntax::json::JsonEmitter;
use syntax::feature_gate;
use syntax::parse;
use syntax::parse::ParseSess;
use syntax::symbol::Symbol;
use syntax::{ast, codemap};
use syntax::feature_gate::AttributeType;
use syntax_pos::{Span, MultiSpan};
Expand Down Expand Up @@ -88,7 +88,7 @@ pub struct Session {
/// forms a unique global identifier for the crate. It is used to allow
/// multiple crates with the same name to coexist. See the
/// trans::back::symbol_names module for more information.
pub crate_disambiguator: RefCell<Option<Symbol>>,
pub crate_disambiguator: RefCell<Option<Fingerprint>>,
pub features: RefCell<feature_gate::Features>,

/// The maximum recursion limit for potentially infinitely recursive
Expand Down Expand Up @@ -165,7 +165,7 @@ enum DiagnosticBuilderMethod {
}

impl Session {
pub fn local_crate_disambiguator(&self) -> Symbol {
pub fn local_crate_disambiguator(&self) -> Fingerprint {
match *self.crate_disambiguator.borrow() {
Some(sym) => sym,
None => bug!("accessing disambiguator before initialization"),
Expand Down Expand Up @@ -471,14 +471,17 @@ impl Session {

/// Returns the symbol name for the registrar function,
/// given the crate Svh and the function DefIndex.
pub fn generate_plugin_registrar_symbol(&self, disambiguator: Symbol, index: DefIndex)
pub fn generate_plugin_registrar_symbol(&self, disambiguator: Fingerprint,
index: DefIndex)
-> String {
format!("__rustc_plugin_registrar__{}_{}", disambiguator, index.as_usize())
format!("__rustc_plugin_registrar__{}_{}", disambiguator.to_hex(),
index.as_usize())
}

pub fn generate_derive_registrar_symbol(&self, disambiguator: Symbol, index: DefIndex)
pub fn generate_derive_registrar_symbol(&self, disambiguator: Fingerprint, index: DefIndex)
-> String {
format!("__rustc_derive_registrar__{}_{}", disambiguator, index.as_usize())
format!("__rustc_derive_registrar__{}_{}", disambiguator.to_hex(),
index.as_usize())
}

pub fn sysroot<'a>(&'a self) -> &'a Path {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
crate_name,
// Don't print the whole crate disambiguator. That's just
// annoying in debug output.
&(crate_disambiguator.as_str())[..4],
&(crate_disambiguator.to_hex())[..4],
self.def_path(def_id).to_string_no_crate())
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ty/maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use dep_graph::{DepConstructor, DepNode};
use errors::DiagnosticBuilder;
use ich::Fingerprint;
use hir::def_id::{CrateNum, DefId, DefIndex};
use hir::def::{Def, Export};
use hir::{self, TraitCandidate, ItemLocalId};
Expand Down Expand Up @@ -283,7 +284,7 @@ define_maps! { <'tcx>
[] fn native_libraries: NativeLibraries(CrateNum) -> Rc<Vec<NativeLibrary>>,
[] fn plugin_registrar_fn: PluginRegistrarFn(CrateNum) -> Option<DefId>,
[] fn derive_registrar_fn: DeriveRegistrarFn(CrateNum) -> Option<DefId>,
[] fn crate_disambiguator: CrateDisambiguator(CrateNum) -> Symbol,
[] fn crate_disambiguator: CrateDisambiguator(CrateNum) -> Fingerprint,
[] fn crate_hash: CrateHash(CrateNum) -> Svh,
[] fn original_crate_name: OriginalCrateName(CrateNum) -> Symbol,

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use hir::{map as hir_map, FreevarMap, TraitMap};
use hir::def::{Def, CtorKind, ExportMap};
use hir::def_id::{CrateNum, DefId, DefIndex, CRATE_DEF_INDEX, LOCAL_CRATE};
use hir::map::DefPathData;
use ich::StableHashingContext;
use ich::{Fingerprint, StableHashingContext};
use middle::const_val::ConstVal;
use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem, FnOnceTraitLangItem};
use middle::privacy::AccessLevels;
Expand Down Expand Up @@ -2562,7 +2562,7 @@ fn param_env<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}

fn crate_disambiguator<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
crate_num: CrateNum) -> Symbol {
crate_num: CrateNum) -> Fingerprint {
assert_eq!(crate_num, LOCAL_CRATE);
tcx.sess.local_crate_disambiguator()
}
Expand Down
18 changes: 8 additions & 10 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ use syntax::{ast, diagnostics, visit};
use syntax::attr;
use syntax::ext::base::ExtCtxt;
use syntax::parse::{self, PResult};
use syntax::symbol::Symbol;
use syntax::util::node_count::NodeCounter;
use syntax;
use syntax_ext;
Expand Down Expand Up @@ -633,12 +632,12 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,

*sess.crate_types.borrow_mut() = collect_crate_types(sess, &krate.attrs);

let disambiguator = Symbol::intern(&compute_crate_disambiguator(sess));
let disambiguator = compute_crate_disambiguator(sess);
*sess.crate_disambiguator.borrow_mut() = Some(disambiguator);
rustc_incremental::prepare_session_directory(
sess,
&crate_name,
&disambiguator.as_str(),
&disambiguator,
);

let dep_graph = if sess.opts.build_dep_graph() {
Expand Down Expand Up @@ -1312,16 +1311,13 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec<c
.collect()
}

pub fn compute_crate_disambiguator(session: &Session) -> String {
pub fn compute_crate_disambiguator(session: &Session) -> Fingerprint {
use std::hash::Hasher;

// The crate_disambiguator is a 128 bit hash. The disambiguator is fed
// into various other hashes quite a bit (symbol hashes, incr. comp. hashes,
// debuginfo type IDs, etc), so we don't want it to be too wide. 128 bits
// should still be safe enough to avoid collisions in practice.
// FIXME(mw): It seems that the crate_disambiguator is used everywhere as
// a hex-string instead of raw bytes. We should really use the
// smaller representation.
let mut hasher = StableHasher::<Fingerprint>::new();

let mut metadata = session.opts.cg.metadata.clone();
Expand All @@ -1340,11 +1336,13 @@ pub fn compute_crate_disambiguator(session: &Session) -> String {
hasher.write(s.as_bytes());
}

// If this is an executable, add a special suffix, so that we don't get
// symbol conflicts when linking against a library of the same name.
// Also incorporate crate type, so that we don't get symbol conflicts when
// linking against a library of the same name, if this is an executable.
let is_exe = session.crate_types.borrow().contains(&config::CrateTypeExecutable);
hasher.write(if is_exe { b"exe" } else { b"lib" });

hasher.finish()

format!("{}{}", hasher.finish().to_hex(), if is_exe { "-exe" } else {""})
}

pub fn build_output_filenames(input: &Input,
Expand Down
15 changes: 5 additions & 10 deletions src/librustc_incremental/persist/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
//! unsupported file system and emit a warning in that case. This is not yet
//! implemented.

use rustc::ich::Fingerprint;
use rustc::hir::svh::Svh;
use rustc::session::Session;
use rustc::util::fs as fs_util;
Expand Down Expand Up @@ -188,7 +189,7 @@ pub fn in_incr_comp_dir(incr_comp_session_dir: &Path, file_name: &str) -> PathBu
/// The garbage collection will take care of it.
pub fn prepare_session_directory(sess: &Session,
crate_name: &str,
crate_disambiguator: &str) {
crate_disambiguator: &Fingerprint) {
if sess.opts.incremental.is_none() {
return
}
Expand Down Expand Up @@ -614,21 +615,15 @@ fn string_to_timestamp(s: &str) -> Result<SystemTime, ()> {

fn crate_path(sess: &Session,
crate_name: &str,
crate_disambiguator: &str)
crate_disambiguator: &Fingerprint)
-> PathBuf {
use std::hash::{Hasher, Hash};
use std::collections::hash_map::DefaultHasher;

let incr_dir = sess.opts.incremental.as_ref().unwrap().clone();

// The full crate disambiguator is really long. A hash of it should be
// sufficient.
let mut hasher = DefaultHasher::new();
crate_disambiguator.hash(&mut hasher);

let crate_disambiguator = crate_disambiguator.to_smaller_hash();
let crate_name = format!("{}-{}",
crate_name,
base_n::encode(hasher.finish(), INT_ENCODE_BASE));
base_n::encode(crate_disambiguator, INT_ENCODE_BASE));
Copy link
Member Author

Choose a reason for hiding this comment

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

This truncates the hash to 64 bits, should I implement a 128 but encoding in base_n?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is pre-existing?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now sure what you mean exactly, but to_smaller_hash() already exists and truncates to 64 bits.
I meant to ask if base_n::encode should be extended to work with Fingerprint/u128, in addition to u64. To be honest, I saw that it accepts u64, so thought that truncating 128->64 might be acceptable in this case

Copy link
Member

Choose a reason for hiding this comment

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

The code you wrote should have roughly the same effect as before since DefaultHasher also produces 64 bits. We don't want these hashes to be too long because they end up in directory names and long paths/names sometimes cause trouble for various reasons (file system support, operating system API restrictions, etc).

Maybe you could add a comment here, similar to the one that is replaced:

// The full crate disambiguator is really long. 64 bits of it should be
// sufficient.

incr_dir.join(crate_name)
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use locator::{self, CratePaths};
use native_libs::relevant_lib;
use schema::CrateRoot;

use rustc::ich::Fingerprint;
use rustc::hir::def_id::{CrateNum, DefIndex, CRATE_DEF_INDEX};
use rustc::hir::svh::Svh;
use rustc::middle::allocator::AllocatorKind;
Expand Down Expand Up @@ -626,7 +627,7 @@ impl<'a> CrateLoader<'a> {
pub fn find_plugin_registrar(&mut self,
span: Span,
name: &str)
-> Option<(PathBuf, Symbol, DefIndex)> {
-> Option<(PathBuf, Fingerprint, DefIndex)> {
let ekrate = self.read_extension_crate(span, &ExternCrateInfo {
name: Symbol::intern(name),
ident: Symbol::intern(name),
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_metadata/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use schema;

use rustc::ich::Fingerprint;
use rustc::hir::def_id::{CRATE_DEF_INDEX, CrateNum, DefIndex};
use rustc::hir::map::definitions::DefPathTable;
use rustc::hir::svh::Svh;
Expand Down Expand Up @@ -171,7 +172,7 @@ impl CrateMetadata {
pub fn hash(&self) -> Svh {
self.root.hash
}
pub fn disambiguator(&self) -> Symbol {
pub fn disambiguator(&self) -> Fingerprint {
self.root.disambiguator
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use rustc::middle::cstore::{CrateStore, DepKind,
LoadedMacro, EncodedMetadata,
EncodedMetadataHashes, NativeLibraryKind};
use rustc::middle::stability::DeprecationEntry;
use rustc::ich::Fingerprint;
use rustc::hir::def;
use rustc::session::Session;
use rustc::ty::{self, TyCtxt};
Expand Down Expand Up @@ -384,7 +385,7 @@ impl CrateStore for cstore::CStore {
self.get_crate_data(cnum).name
}

fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> Symbol
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> Fingerprint
{
self.get_crate_data(cnum).disambiguator()
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_metadata/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use index;
use rustc::hir;
use rustc::hir::def::{self, CtorKind};
use rustc::hir::def_id::{DefIndex, DefId, CrateNum};
use rustc::ich::StableHashingContext;
use rustc::ich::{Fingerprint, StableHashingContext};
use rustc::middle::cstore::{DepKind, LinkagePreference, NativeLibrary};
use rustc::middle::lang_items;
use rustc::mir;
Expand Down Expand Up @@ -191,7 +191,7 @@ pub struct CrateRoot {
pub name: Symbol,
pub triple: String,
pub hash: hir::svh::Svh,
pub disambiguator: Symbol,
pub disambiguator: Fingerprint,
pub panic_strategy: PanicStrategy,
pub has_global_allocator: bool,
pub has_default_lib_allocator: bool,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1417,7 +1417,7 @@ impl<'a> Resolver<'a> {

let mut definitions = Definitions::new();
DefCollector::new(&mut definitions, Mark::root())
.collect_root(crate_name, &session.local_crate_disambiguator().as_str());
.collect_root(crate_name, &session.local_crate_disambiguator().to_hex());

let mut invocations = FxHashMap();
invocations.insert(Mark::root(),
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub fn threshold(tcx: TyCtxt) -> SymbolExportLevel {
pub fn metadata_symbol_name(tcx: TyCtxt) -> String {
format!("rust_metadata_{}_{}",
tcx.crate_name(LOCAL_CRATE),
tcx.crate_disambiguator(LOCAL_CRATE))
tcx.crate_disambiguator(LOCAL_CRATE).to_hex())
}

fn crate_export_threshold(crate_type: config::CrateType) -> SymbolExportLevel {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/back/symbol_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ fn get_symbol_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

if avoid_cross_crate_conflicts {
hasher.hash(tcx.crate_name.as_str());
hasher.hash(tcx.sess.local_crate_disambiguator().as_str());
hasher.hash(tcx.sess.local_crate_disambiguator());
}
});

Expand Down