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.

Sign up for GitHub

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

Switch CrateMetadata's source_map_import_info from RwLock to Once #65979

Merged
merged 3 commits into from
Oct 31, 2019
Merged

Switch CrateMetadata's source_map_import_info from RwLock to Once #65979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
308c4a6
Correctly indent get_predicates function
spastorino Oct 30, 2019
a6ac22e
Change CrateMetadata's source_map_import_info from RwLock to Once
spastorino Oct 30, 2019
12273cb
Make init_locking return a reference to the initialized data
spastorino Oct 30, 2019
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
14 changes: 8 additions & 6 deletions src/librustc_data_structures/sync.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,13 +497,15 @@ impl<T> Once<T> {
/// If the value was already initialized the closure is not called and `false` is returned,
/// otherwise if the value from the closure initializes the inner value, `true` is returned
#[inline]
spastorino marked this conversation as resolved.
Show resolved Hide resolved
pub fn init_locking<F: FnOnce() -> T>(&self, f: F) -> bool {
let mut lock = self.0.lock();
if lock.is_some() {
return false;
pub fn init_locking<F: FnOnce() -> T>(&self, f: F) -> &T {
{
let mut lock = self.0.lock();
if lock.is_none() {
*lock = Some(f());
}
}
*lock = Some(f());
true

self.borrow()
spastorino marked this conversation as resolved.
Show resolved Hide resolved
}

/// Tries to initialize the inner value by calling the closure without ensuring that no-one
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_metadata/creader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::cstore::{self, CStore, MetadataBlob};
use crate::locator::{self, CratePaths};
use crate::schema::{CrateRoot, CrateDep};
use rustc_data_structures::sync::{RwLock, Lock, AtomicCell};
use rustc_data_structures::sync::{Lock, Once, AtomicCell};

use rustc::hir::def_id::CrateNum;
use rustc_data_structures::svh::Svh;
Expand Down Expand Up @@ -249,7 +249,7 @@ impl<'a> CrateLoader<'a> {
cnum_map,
cnum,
dependencies: Lock::new(dependencies),
source_map_import_info: RwLock::new(vec![]),
source_map_import_info: Once::new(),
alloc_decoding_state: AllocDecodingState::new(interpret_alloc_index),
dep_kind: Lock::new(dep_kind),
source,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_metadata/cstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc::middle::cstore::{CrateSource, DepKind, ExternCrate};
use rustc::mir::interpret::AllocDecodingState;
use rustc_index::vec::IndexVec;
use rustc::util::nodemap::FxHashMap;
use rustc_data_structures::sync::{Lrc, RwLock, Lock, MetadataRef, AtomicCell};
use rustc_data_structures::sync::{Lrc, Lock, MetadataRef, Once, AtomicCell};
use syntax::ast;
use syntax::edition::Edition;
use syntax_expand::base::SyntaxExtension;
Expand Down Expand Up @@ -62,7 +62,7 @@ crate struct CrateMetadata {
/// Proc macro descriptions for this crate, if it's a proc macro crate.
crate raw_proc_macros: Option<&'static [ProcMacro]>,
/// Source maps for code from the crate.
crate source_map_import_info: RwLock<Vec<ImportedSourceFile>>,
crate source_map_import_info: Once<Vec<ImportedSourceFile>>,
/// Used for decoding interpret::AllocIds in a cached & thread-safe manner.
crate alloc_decoding_state: AllocDecodingState,
/// The `DepNodeIndex` of the `DepNode` representing this upstream crate.
Expand Down
145 changes: 63 additions & 82 deletions src/librustc_metadata/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::schema::*;
use crate::table::{FixedSizeEncoding, PerDefTable};

use rustc_index::vec::IndexVec;
use rustc_data_structures::sync::{Lrc, ReadGuard};
use rustc_data_structures::sync::Lrc;
use rustc::hir::map::{DefKey, DefPath, DefPathData, DefPathHash};
use rustc::hir;
use rustc::middle::cstore::{LinkagePreference, NativeLibrary, ForeignModule};
Expand Down Expand Up @@ -664,7 +664,7 @@ impl<'a, 'tcx> CrateMetadata {
tcx: TyCtxt<'tcx>,
) -> ty::GenericPredicates<'tcx> {
self.root.per_def.predicates.get(self, item_id).unwrap().decode((self, tcx))
}
}

crate fn get_predicates_defined_on(
&self,
Expand Down Expand Up @@ -1290,87 +1290,68 @@ impl<'a, 'tcx> CrateMetadata {
fn imported_source_files(
&'a self,
local_source_map: &source_map::SourceMap,
) -> ReadGuard<'a, Vec<cstore::ImportedSourceFile>> {
{
let source_files = self.source_map_import_info.borrow();
if !source_files.is_empty() {
return source_files;
}
}

// Lock the source_map_import_info to ensure this only happens once
let mut source_map_import_info = self.source_map_import_info.borrow_mut();

if !source_map_import_info.is_empty() {
drop(source_map_import_info);
return self.source_map_import_info.borrow();
}

let external_source_map = self.root.source_map.decode(self);

let imported_source_files = external_source_map.map(|source_file_to_import| {
// We can't reuse an existing SourceFile, so allocate a new one
// containing the information we need.
let syntax_pos::SourceFile { name,
name_was_remapped,
src_hash,
start_pos,
end_pos,
mut lines,
mut multibyte_chars,
mut non_narrow_chars,
mut normalized_pos,
name_hash,
.. } = source_file_to_import;

let source_length = (end_pos - start_pos).to_usize();

// Translate line-start positions and multibyte character
// position into frame of reference local to file.
// `SourceMap::new_imported_source_file()` will then translate those
// coordinates to their new global frame of reference when the
// offset of the SourceFile is known.
for pos in &mut lines {
*pos = *pos - start_pos;
}
for mbc in &mut multibyte_chars {
mbc.pos = mbc.pos - start_pos;
}
for swc in &mut non_narrow_chars {
*swc = *swc - start_pos;
}
for np in &mut normalized_pos {
np.pos = np.pos - start_pos;
}

let local_version = local_source_map.new_imported_source_file(name,
name_was_remapped,
self.cnum.as_u32(),
src_hash,
name_hash,
source_length,
lines,
multibyte_chars,
non_narrow_chars,
normalized_pos);
debug!("CrateMetaData::imported_source_files alloc \
source_file {:?} original (start_pos {:?} end_pos {:?}) \
translated (start_pos {:?} end_pos {:?})",
local_version.name, start_pos, end_pos,
local_version.start_pos, local_version.end_pos);

cstore::ImportedSourceFile {
original_start_pos: start_pos,
original_end_pos: end_pos,
translated_source_file: local_version,
}
}).collect();

*source_map_import_info = imported_source_files;
drop(source_map_import_info);
) -> &[cstore::ImportedSourceFile] {
self.source_map_import_info.init_locking(|| {
let external_source_map = self.root.source_map.decode(self);

external_source_map.map(|source_file_to_import| {
// We can't reuse an existing SourceFile, so allocate a new one
// containing the information we need.
let syntax_pos::SourceFile { name,
name_was_remapped,
src_hash,
start_pos,
end_pos,
mut lines,
mut multibyte_chars,
mut non_narrow_chars,
mut normalized_pos,
name_hash,
.. } = source_file_to_import;

let source_length = (end_pos - start_pos).to_usize();

// Translate line-start positions and multibyte character
// position into frame of reference local to file.
// `SourceMap::new_imported_source_file()` will then translate those
// coordinates to their new global frame of reference when the
// offset of the SourceFile is known.
for pos in &mut lines {
*pos = *pos - start_pos;
}
for mbc in &mut multibyte_chars {
mbc.pos = mbc.pos - start_pos;
}
for swc in &mut non_narrow_chars {
*swc = *swc - start_pos;
}
for np in &mut normalized_pos {
np.pos = np.pos - start_pos;
}

// This shouldn't borrow twice, but there is no way to downgrade RefMut to Ref.
self.source_map_import_info.borrow()
let local_version = local_source_map.new_imported_source_file(name,
name_was_remapped,
self.cnum.as_u32(),
src_hash,
name_hash,
source_length,
lines,
multibyte_chars,
non_narrow_chars,
normalized_pos);
debug!("CrateMetaData::imported_source_files alloc \
source_file {:?} original (start_pos {:?} end_pos {:?}) \
translated (start_pos {:?} end_pos {:?})",
local_version.name, start_pos, end_pos,
local_version.start_pos, local_version.end_pos);

cstore::ImportedSourceFile {
original_start_pos: start_pos,
original_end_pos: end_pos,
translated_source_file: local_version,
}
}).collect()
})
}

/// Get the `DepNodeIndex` corresponding this crate. The result of this
Expand Down