From 05c09cb62d05df0ccb65503992a7dae3b8b12b08 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 15 Sep 2021 18:33:41 +0200 Subject: [PATCH 1/3] 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. --- compiler/rustc_span/src/lib.rs | 4 ++-- compiler/rustc_span/src/symbol.rs | 31 ++++++++++++++----------- compiler/rustc_span/src/symbol/tests.rs | 2 +- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index eb496140553ad..9c5469f635f71 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -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, span_interner: Lock, hygiene_data: Lock, source_map: Lock>>, @@ -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), diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 1df5b8cd2cc8d..d0163c36aa225 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -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}; @@ -1696,6 +1697,9 @@ impl ToStableHashKey for Symbol { } } +#[derive(Default)] +pub(crate) struct Interner(Lock); + // The `&'static str`s in this type actually point into the arena. // // The `FxHashMap`+`Vec` pair could be replaced by `FxIndexSet`, but #75278 @@ -1705,7 +1709,7 @@ impl ToStableHashKey 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>, @@ -1713,37 +1717,38 @@ pub(crate) struct Interner { 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()] } } @@ -1875,8 +1880,8 @@ impl Ident { } #[inline] -fn with_interner T>(f: F) -> T { - with_session_globals(|session_globals| f(&mut *session_globals.symbol_interner.lock())) +fn with_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 diff --git a/compiler/rustc_span/src/symbol/tests.rs b/compiler/rustc_span/src/symbol/tests.rs index 11dea265b4e66..0958fce5fee30 100644 --- a/compiler/rustc_span/src/symbol/tests.rs +++ b/compiler/rustc_span/src/symbol/tests.rs @@ -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: From 0ad89819457e7067884597e9a816dab483377ed5 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 15 Sep 2021 18:44:17 +0200 Subject: [PATCH 2/3] Inline with_interner --- compiler/rustc_span/src/symbol.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index d0163c36aa225..95ab19b55d256 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1624,14 +1624,15 @@ impl Symbol { /// Maps a string to its interned representation. pub fn intern(string: &str) -> Self { - with_interner(|interner| interner.intern(string)) + with_session_globals(|session_globals| session_globals.symbol_interner.intern(string)) } /// Convert to a `SymbolStr`. This is a slowish operation because it /// requires locking the symbol interner. pub fn as_str(self) -> SymbolStr { - with_interner(|interner| unsafe { - SymbolStr { string: std::mem::transmute::<&str, &str>(interner.get(self)) } + with_session_globals(|session_globals| { + let symbol_str = session_globals.symbol_interner.get(self); + unsafe { SymbolStr { string: std::mem::transmute::<&str, &str>(symbol_str) } } }) } @@ -1640,7 +1641,7 @@ impl Symbol { } pub fn len(self) -> usize { - with_interner(|interner| interner.get(self).len()) + with_session_globals(|session_globals| session_globals.symbol_interner.get(self).len()) } pub fn is_empty(self) -> bool { @@ -1879,11 +1880,6 @@ impl Ident { } } -#[inline] -fn with_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 /// be accessed. It deliberately has limited functionality and should only be /// used for temporary values. From ccba8cb4bb6ba5c2c136f14952ff6119020cc7f5 Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Wed, 15 Sep 2021 18:44:31 +0200 Subject: [PATCH 3/3] Make two functions private --- compiler/rustc_span/src/symbol.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 95ab19b55d256..4cdf2167b9034 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1726,7 +1726,7 @@ impl Interner { } #[inline] - pub(crate) fn intern(&self, string: &str) -> Symbol { + fn intern(&self, string: &str) -> Symbol { let mut inner = self.0.lock(); if let Some(&name) = inner.names.get(string) { return name; @@ -1748,7 +1748,7 @@ impl Interner { // Get the symbol as a string. `Symbol::as_str()` should be used in // preference to this function. - pub(crate) fn get(&self, symbol: Symbol) -> &str { + fn get(&self, symbol: Symbol) -> &str { self.0.lock().strings[symbol.0.as_usize()] } }