diff options
author | Aspen Smith <root@gws.fyi> | 2024-07-28T16·50-0400 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-08-10T15·28+0000 |
commit | d378111d774777f4a5f319946d26e3d481a49870 (patch) | |
tree | 50f3f40d24e00ab3f5d9e5a7ea0dc3b543ba34d3 /tvix/eval/src/value | |
parent | a6d6fc418de0cdce921698410265fa78ca6b7f23 (diff) |
feat(tvix/eval): Store hash in key of interner r/8474
Rather than storing the leaked allocation for the string as the key in the interner, store the hash (using NoHashHashBuilder). I thought this would improve performance, but it doesn't: hello outpath time: [736.85 ms 748.42 ms 760.42 ms] change: [-2.0754% +0.4798% +2.7096%] (p = 0.72 > 0.05) No change in performance detected. but it at least doesn't *hurt* performance, and it *does* avoid an `unsafe`, so it's probably net good. Change-Id: Ie413955bdb6f04b1f468f511e5ebce56e329fa37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/12049 Tested-by: BuildkiteCI Reviewed-by: flokli <flokli@flokli.de> Autosubmit: aspen <root@gws.fyi>
Diffstat (limited to 'tvix/eval/src/value')
-rw-r--r-- | tvix/eval/src/value/string.rs | 27 |
1 files changed, 20 insertions, 7 deletions
diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index 0365f5a9be77..247d23a7c895 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -4,18 +4,20 @@ //! level, allowing us to shave off some memory overhead and only //! paying the cost when creating new strings. use bstr::{BStr, BString, ByteSlice, Chars}; +use nohash_hasher::BuildNoHashHasher; use rnix::ast; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashSet; +use rustc_hash::FxHasher; use std::alloc::dealloc; use std::alloc::{alloc, handle_alloc_error, Layout}; use std::borrow::{Borrow, Cow}; use std::cell::RefCell; use std::ffi::c_void; use std::fmt::{self, Debug, Display}; -use std::hash::Hash; +use std::hash::{Hash, Hasher}; use std::ops::Deref; use std::ptr::{self, NonNull}; -use std::{mem, slice}; +use std::slice; use serde::de::{Deserializer, Visitor}; use serde::{Deserialize, Serialize}; @@ -399,22 +401,33 @@ impl NixStringInner { #[derive(Default)] struct InternerInner { - map: FxHashMap<&'static [u8], NonNull<c_void>>, + #[allow(clippy::disallowed_types)] // Not using the default hasher + map: std::collections::HashMap<u64, NonNull<c_void>, BuildNoHashHasher<u64>>, #[cfg(feature = "no_leak")] + #[allow(clippy::disallowed_types)] // Not using the default hasher interned_strings: FxHashSet<NonNull<c_void>>, } unsafe impl Send for InternerInner {} +fn hash<T>(s: T) -> u64 +where + T: Hash, +{ + let mut hasher = FxHasher::default(); + s.hash(&mut hasher); + hasher.finish() +} + impl InternerInner { pub fn intern(&mut self, s: &[u8]) -> NixString { - if let Some(s) = self.map.get(s) { + let hash = hash(s); + if let Some(s) = self.map.get(&hash) { return NixString(*s); } let string = NixString::new_inner(s, None); - self.map - .insert(unsafe { mem::transmute(string.as_bytes()) }, string.0); + self.map.insert(hash, string.0); #[cfg(feature = "no_leak")] self.interned_strings.insert(string.0); string |