From d085601c31404d2bbc076ead1e37d17dcd8b561f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B0=95=EB=8F=99=EC=9C=A4=20=28Donny=29?= Date: Fri, 14 Feb 2025 12:59:07 +0900 Subject: [PATCH 1/2] Remove WeakItem --- crates/hstr/src/dynamic.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/hstr/src/dynamic.rs b/crates/hstr/src/dynamic.rs index 54de15d80782..84d8051fefae 100644 --- a/crates/hstr/src/dynamic.rs +++ b/crates/hstr/src/dynamic.rs @@ -30,9 +30,6 @@ impl Deref for Item { } } -/// TODO: Use real weak pointer -type WeakItem = Item; - impl Hash for Item { fn hash(&self, state: &mut H) { state.write_u64(self.0.header.header.header.hash); @@ -56,7 +53,7 @@ pub(crate) unsafe fn restore_arc(v: TaggedValue) -> Item { /// /// # Merging [AtomStore] pub struct AtomStore { - pub(crate) data: hashbrown::HashMap, + pub(crate) data: hashbrown::HashMap, } impl Default for AtomStore { From e8286715330ba81ed021ef7966d9695fec6f53c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B0=95=EB=8F=99=EC=9C=A4=20=28Donny=29?= Date: Fri, 14 Feb 2025 13:13:51 +0900 Subject: [PATCH 2/2] Do not intern from global atom store --- crates/hstr/src/dynamic.rs | 47 ++++++++++++++++++++++----------- crates/hstr/src/global_store.rs | 20 +++----------- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/crates/hstr/src/dynamic.rs b/crates/hstr/src/dynamic.rs index 84d8051fefae..6a6c4dbc0f63 100644 --- a/crates/hstr/src/dynamic.rs +++ b/crates/hstr/src/dynamic.rs @@ -67,13 +67,13 @@ impl Default for AtomStore { impl AtomStore { #[inline(always)] pub fn atom<'a>(&mut self, text: impl Into>) -> Atom { - new_atom(self, text.into()) + atom_in_store(self, text.into()) } } /// This can create any kind of [Atom], although this lives in the `dynamic` /// module. -pub(crate) fn new_atom(storage: S, text: Cow) -> Atom +fn atom_in_store(storage: S, text: Cow) -> Atom where S: Storage, { @@ -110,12 +110,18 @@ pub(crate) trait Storage { impl Storage for &'_ mut AtomStore { #[inline(never)] fn insert_entry(self, text: Cow, hash: u64) -> Item { + macro_rules! make { + () => {{ + Item(ThinArc::from_header_and_slice( + HeaderWithLength::new(Metadata { hash }, text.len()), + text.as_bytes(), + )) + }}; + } + // If the text is too long, interning is not worth it. if text.len() > 512 { - return Item(ThinArc::from_header_and_slice( - HeaderWithLength::new(Metadata { hash }, text.len()), - text.as_bytes(), - )); + return make!(); } let (entry, _) = self @@ -124,15 +130,7 @@ impl Storage for &'_ mut AtomStore { .from_hash(hash, |key| { key.header.header.header.hash == hash && key.slice == *text.as_bytes() }) - .or_insert_with(move || { - ( - Item(ThinArc::from_header_and_slice( - HeaderWithLength::new(Metadata { hash }, text.len()), - text.as_bytes(), - )), - (), - ) - }); + .or_insert_with(move || (make!(), ())); entry.clone() } } @@ -144,7 +142,24 @@ fn calc_hash(text: &str) -> u64 { hasher.finish() } -type BuildEntryHasher = BuildHasherDefault; +/// Do not intern. +struct GlobalAtomStore; + +impl Storage for GlobalAtomStore { + #[inline(never)] + fn insert_entry(self, text: Cow, hash: u64) -> Item { + Item(ThinArc::from_header_and_slice( + HeaderWithLength::new(Metadata { hash }, text.len()), + text.as_bytes(), + )) + } +} + +pub(crate) fn global_atom(text: Cow) -> Atom { + atom_in_store(GlobalAtomStore, text) +} + +pub(crate) type BuildEntryHasher = BuildHasherDefault; /// A "no-op" hasher for [Entry] that returns [Entry::hash]. The design is /// inspired by the `nohash-hasher` crate. diff --git a/crates/hstr/src/global_store.rs b/crates/hstr/src/global_store.rs index af08ad1e0b44..901d7f6c773e 100644 --- a/crates/hstr/src/global_store.rs +++ b/crates/hstr/src/global_store.rs @@ -1,24 +1,12 @@ -use std::{borrow::Cow, cell::RefCell}; +use std::borrow::Cow; -use crate::{Atom, AtomStore}; - -fn atom(text: Cow) -> Atom { - thread_local! { - static GLOBAL_DATA: RefCell = Default::default(); - } - - GLOBAL_DATA.with(|global| { - let mut store = global.borrow_mut(); - - store.atom(text) - }) -} +use crate::{dynamic::global_atom, Atom}; macro_rules! direct_from_impl { ($T:ty) => { impl From<$T> for Atom { fn from(s: $T) -> Self { - atom(s.into()) + global_atom(s.into()) } } }; @@ -30,6 +18,6 @@ direct_from_impl!(String); impl From> for crate::Atom { fn from(s: Box) -> Self { - atom(Cow::Owned(String::from(s))) + global_atom(Cow::Owned(String::from(s))) } }