Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Move the Lock into symbol::Interner
This makes it easier to make the symbol interner (near) lock free in
case of concurrent accesses in the future.
  • Loading branch information
bjorn3 committed Sep 15, 2021
1 parent 2c7bc5e commit 05c09cb
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 16 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_span/src/lib.rs
Expand Up @@ -78,7 +78,7 @@ mod tests;
// threads within the compilation session, but is not accessible outside the
// session.
pub struct SessionGlobals {
symbol_interner: Lock<symbol::Interner>,
symbol_interner: symbol::Interner,
span_interner: Lock<span_encoding::SpanInterner>,
hygiene_data: Lock<hygiene::HygieneData>,
source_map: Lock<Option<Lrc<SourceMap>>>,
Expand All @@ -87,7 +87,7 @@ pub struct SessionGlobals {
impl SessionGlobals {
pub fn new(edition: Edition) -> SessionGlobals {
SessionGlobals {
symbol_interner: Lock::new(symbol::Interner::fresh()),
symbol_interner: symbol::Interner::fresh(),
span_interner: Lock::new(span_encoding::SpanInterner::default()),
hygiene_data: Lock::new(hygiene::HygieneData::new(edition)),
source_map: Lock::new(None),
Expand Down
31 changes: 18 additions & 13 deletions compiler/rustc_span/src/symbol.rs
Expand Up @@ -5,6 +5,7 @@
use rustc_arena::DroplessArena;
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, ToStableHashKey};
use rustc_data_structures::sync::Lock;
use rustc_macros::HashStable_Generic;
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};

Expand Down Expand Up @@ -1696,6 +1697,9 @@ impl<CTX> ToStableHashKey<CTX> for Symbol {
}
}

#[derive(Default)]
pub(crate) struct Interner(Lock<InternerInner>);

// The `&'static str`s in this type actually point into the arena.
//
// The `FxHashMap`+`Vec` pair could be replaced by `FxIndexSet`, but #75278
Expand All @@ -1705,45 +1709,46 @@ impl<CTX> ToStableHashKey<CTX> for Symbol {
// This type is private to prevent accidentally constructing more than one `Interner` on the same
// thread, which makes it easy to mixup `Symbol`s between `Interner`s.
#[derive(Default)]
pub(crate) struct Interner {
struct InternerInner {
arena: DroplessArena,
names: FxHashMap<&'static str, Symbol>,
strings: Vec<&'static str>,
}

impl Interner {
fn prefill(init: &[&'static str]) -> Self {
Interner {
Interner(Lock::new(InternerInner {
strings: init.into(),
names: init.iter().copied().zip((0..).map(Symbol::new)).collect(),
..Default::default()
}
}))
}

#[inline]
pub fn intern(&mut self, string: &str) -> Symbol {
if let Some(&name) = self.names.get(string) {
pub(crate) fn intern(&self, string: &str) -> Symbol {
let mut inner = self.0.lock();
if let Some(&name) = inner.names.get(string) {
return name;
}

let name = Symbol::new(self.strings.len() as u32);
let name = Symbol::new(inner.strings.len() as u32);

// `from_utf8_unchecked` is safe since we just allocated a `&str` which is known to be
// UTF-8.
let string: &str =
unsafe { str::from_utf8_unchecked(self.arena.alloc_slice(string.as_bytes())) };
unsafe { str::from_utf8_unchecked(inner.arena.alloc_slice(string.as_bytes())) };
// It is safe to extend the arena allocation to `'static` because we only access
// these while the arena is still alive.
let string: &'static str = unsafe { &*(string as *const str) };
self.strings.push(string);
self.names.insert(string, name);
inner.strings.push(string);
inner.names.insert(string, name);
name
}

// Get the symbol as a string. `Symbol::as_str()` should be used in
// preference to this function.
pub fn get(&self, symbol: Symbol) -> &str {
self.strings[symbol.0.as_usize()]
pub(crate) fn get(&self, symbol: Symbol) -> &str {
self.0.lock().strings[symbol.0.as_usize()]
}
}

Expand Down Expand Up @@ -1875,8 +1880,8 @@ impl Ident {
}

#[inline]
fn with_interner<T, F: FnOnce(&mut Interner) -> T>(f: F) -> T {
with_session_globals(|session_globals| f(&mut *session_globals.symbol_interner.lock()))
fn with_interner<T, F: FnOnce(&Interner) -> T>(f: F) -> T {
with_session_globals(|session_globals| f(&session_globals.symbol_interner))
}

/// An alternative to [`Symbol`], useful when the chars within the symbol need to
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/symbol/tests.rs
Expand Up @@ -4,7 +4,7 @@ use crate::create_default_session_globals_then;

#[test]
fn interner_tests() {
let mut i: Interner = Interner::default();
let i = Interner::default();
// first one is zero:
assert_eq!(i.intern("dog"), Symbol::new(0));
// re-use gets the same entry:
Expand Down

0 comments on commit 05c09cb

Please sign in to comment.