Merge pull request #651 from schungx/master

Use environment variable for stable hashing.

CHANGELOG.md

@@ -7,7 +7,9 @@ Version 1.11.0
 New features
 ------------
 
-* A new feature flag, `stable_hash`, is added that forces hashing to be consistent using a fixed seed.
+### Stable hashing
+
+* It is now possible to specify a fixed _seed_ for use with the `ahash` hasher in order to force stable (i.e. deterministic) hashes for function signatures.
 
 Enhancements
 ------------

Cargo.toml

@@ -62,7 +62,6 @@ metadata = ["serde", "serde_json", "rhai_codegen/metadata", "smartstring/serde"]
 internals = []                  # expose internal data structures
 debugging = ["internals"]       # enable debugging
 serde = ["dep:serde", "smartstring/serde", "smallvec/serde"] # implement serde for rhai types
-stable_hash = []                # perform all hashing with fixed seed value
 
 # compiling for no-std
 no_std = ["no-std-compat", "num-traits/libm", "core-error", "libm", "ahash/compile-time-rng", "hashbrown/ahash-compile-time-rng"]

build.rs

@@ -0,0 +1,27 @@
+use std::{env, fs::File, io::Write};
+
+const WRITE_ERROR: &str = "cannot write to `config.rs`";
+
+fn main() {
+    // Tell Cargo that if the given environment variable changes, to rerun this build script.
+    println!("cargo:rerun-if-env-changed=RHAI_AHASH_SEED");
+
+    let mut f = File::create("src/config.rs").expect("cannot create `config.rs`");
+
+    f.write_fmt(format_args!(
+        "//! Configuration settings for this Rhai build
+#![allow(dead_code)]
+
+"
+    ))
+    .expect(WRITE_ERROR);
+
+    let seed = env::var("RHAI_AHASH_SEED").map_or_else(|_| "None".into(), |s| format!("Some({s})"));
+
+    f.write_fmt(format_args!(
+        "pub const AHASH_SEED: Option<[u64; 4]> = {seed};\n"
+    ))
+    .expect(WRITE_ERROR);
+
+    f.flush().expect("cannot flush `config.rs`");
+}

src/ast/ast.rs

@@ -42,6 +42,8 @@ impl Default for AST {
 }
 
 impl fmt::Debug for AST {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut fp = f.debug_struct("AST");
 

src/ast/expr.rs

@@ -117,6 +117,8 @@ pub struct FnCallHashes {
 }
 
 impl fmt::Debug for FnCallHashes {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         #[cfg(not(feature = "no_function"))]
         if self.script != 0 {
@@ -199,6 +201,8 @@ pub struct FnCallExpr {
 }
 
 impl fmt::Debug for FnCallExpr {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut ff = f.debug_struct("FnCallExpr");
         #[cfg(not(feature = "no_module"))]
@@ -294,7 +298,8 @@ impl<F: Float> DerefMut for FloatWrapper<F> {
 
 #[cfg(not(feature = "no_float"))]
 impl<F: Float + fmt::Debug> fmt::Debug for FloatWrapper<F> {
-    #[inline(always)]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Debug::fmt(&self.0, f)
     }
@@ -448,6 +453,8 @@ impl Default for Expr {
 }
 
 impl fmt::Debug for Expr {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut display_pos = format!(" @ {:?}", self.start_position());
 

src/ast/ident.rs

@@ -20,6 +20,8 @@ pub struct Ident {
 }
 
 impl fmt::Debug for Ident {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{:?}", self.name)?;
         self.pos.debug_print(f)

src/ast/namespace.rs

@@ -29,6 +29,8 @@ pub struct Namespace {
 }
 
 impl fmt::Debug for Namespace {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.is_empty() {
             return f.write_str("NONE");

src/ast/stmt.rs

@@ -106,6 +106,8 @@ impl OpAssignment {
 }
 
 impl fmt::Debug for OpAssignment {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.is_op_assignment() {
             f.debug_struct("OpAssignment")
@@ -178,7 +180,8 @@ pub enum RangeCase {
 }
 
 impl fmt::Debug for RangeCase {
-    #[inline]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::ExclusiveInt(r, n) => write!(f, "{}..{} => {}", r.start, r.end, n),
@@ -454,6 +457,8 @@ impl AsMut<[Stmt]> for StmtBlock {
 }
 
 impl fmt::Debug for StmtBlock {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str("Block")?;
         fmt::Debug::fmt(&self.block, f)?;

src/config.rs

@@ -0,0 +1,4 @@
+//! Configuration settings for this Rhai build
+#![allow(dead_code)]
+
+pub const AHASH_SEED: Option<[u64; 4]> = None;

src/engine.rs

@@ -150,7 +150,8 @@ pub struct Engine {
 }
 
 impl fmt::Debug for Engine {
-    #[inline]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut f = f.debug_struct("Engine");
 

src/eval/global_state.rs

@@ -354,7 +354,8 @@ impl<K: Into<crate::ImmutableString>, M: Into<crate::Shared<crate::Module>>> Ext
 }
 
 impl fmt::Debug for GlobalRuntimeState<'_> {
-    #[inline]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut f = f.debug_struct("GlobalRuntimeState");
 

src/func/callable_function.rs

@@ -27,6 +27,8 @@ pub enum CallableFunction {
 }
 
 impl fmt::Debug for CallableFunction {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
             Self::Pure(..) => write!(f, "NativePureFunction"),

src/func/hashing.rs

@@ -1,5 +1,6 @@
 //! Module containing utilities to hash functions and function calls.
 
+use crate::config;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{
@@ -74,8 +75,8 @@ impl BuildHasher for StraightHasherBuilder {
 #[inline(always)]
 #[must_use]
 pub fn get_hasher() -> ahash::AHasher {
-    if cfg!(feature = "stable_hash") {
-        ahash::RandomState::with_seeds(42, 999, 123, 0).build_hasher()
+    if let Some([seed1, seed2, seed3, seed4]) = config::AHASH_SEED {
+        ahash::RandomState::with_seeds(seed1, seed2, seed3, seed4).build_hasher()
     } else {
         ahash::AHasher::default()
     }

src/lib.rs

@@ -83,6 +83,7 @@ use std::prelude::v1::*;
 // Internal modules
 mod api;
 mod ast;
+mod config;
 mod engine;
 mod eval;
 mod func;
@@ -224,7 +225,7 @@ pub mod debugger {
 /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most
 /// identifiers are ASCII and short, fewer than 23 characters, so they can be stored inline.
 #[cfg(not(feature = "internals"))]
-pub(crate) type Identifier = SmartString;
+type Identifier = SmartString;
 
 /// An identifier in Rhai. [`SmartString`](https://crates.io/crates/smartstring) is used because most
 /// identifiers are ASCII and short, fewer than 23 characters, so they can be stored inline.
@@ -237,7 +238,7 @@ pub use func::Shared;
 /// Alias to [`RefCell`][std::cell::RefCell] or [`RwLock`][std::sync::RwLock] depending on the `sync` feature flag.
 pub use func::Locked;
 
-pub(crate) use func::{calc_fn_hash, calc_fn_params_hash, calc_var_hash, combine_hashes};
+use func::{calc_fn_hash, calc_fn_params_hash, calc_var_hash, combine_hashes};
 
 pub use rhai_codegen::*;
 
@@ -429,7 +430,7 @@ type FnArgsVec<T> = smallvec::SmallVec<[T; 5]>;
 #[cfg(feature = "no_closure")]
 type FnArgsVec<T> = crate::StaticVec<T>;
 
-pub(crate) type SmartString = smartstring::SmartString<smartstring::LazyCompact>;
+type SmartString = smartstring::SmartString<smartstring::LazyCompact>;
 
 // Compiler guards against mutually-exclusive feature flags
 

src/module/mod.rs

@@ -202,6 +202,8 @@ impl Default for Module {
 }
 
 impl fmt::Debug for Module {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut d = f.debug_struct("Module");
 

src/packages/iter_basic.rs

@@ -47,6 +47,8 @@ pub struct StepRange<T: Debug + Copy + PartialOrd> {
 }
 
 impl<T: Debug + Copy + PartialOrd> Debug for StepRange<T> {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_tuple(&format!("StepRange<{}>", type_name::<T>()))
             .field(&self.from)

src/parser.rs

@@ -47,9 +47,6 @@ const NEVER_ENDS: &str = "`Token`";
 /// Unroll `switch` ranges no larger than this.
 const SMALL_SWITCH_RANGE: INT = 16;
 
-/// Number of string interners used: two additional for property getters/setters if not `no_object`
-const NUM_INTERNERS: usize = if cfg!(feature = "no_object") { 1 } else { 3 };
-
 /// _(internals)_ A type that encapsulates the current state of the parser.
 /// Exported under the `internals` feature only.
 pub struct ParseState<'e> {
@@ -58,7 +55,7 @@ pub struct ParseState<'e> {
     /// Controls whether parsing of an expression should stop given the next token.
     pub expr_filter: fn(&Token) -> bool,
     /// String interners.
-    interned_strings: [StringsInterner<'e>; NUM_INTERNERS],
+    interned_strings: StringsInterner<'e>,
     /// External [scope][Scope] with constants.
     pub scope: &'e Scope<'e>,
     /// Global runtime state.
@@ -88,6 +85,8 @@ pub struct ParseState<'e> {
 }
 
 impl fmt::Debug for ParseState<'_> {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let mut f = f.debug_struct("ParseState");
 
@@ -116,7 +115,7 @@ impl<'e> ParseState<'e> {
     pub fn new(
         engine: &Engine,
         scope: &'e Scope,
-        interned_strings: [StringsInterner<'e>; NUM_INTERNERS],
+        interned_strings: StringsInterner<'e>,
         tokenizer_control: TokenizerControl,
     ) -> Self {
         Self {
@@ -254,7 +253,7 @@ impl<'e> ParseState<'e> {
         &mut self,
         text: impl AsRef<str> + Into<ImmutableString>,
     ) -> ImmutableString {
-        self.interned_strings[0].get(text)
+        self.interned_strings.get(text)
     }
 
     /// Get an interned property getter, creating one if it is not yet interned.
@@ -265,8 +264,11 @@ impl<'e> ParseState<'e> {
         &mut self,
         text: impl AsRef<str> + Into<ImmutableString>,
     ) -> ImmutableString {
-        self.interned_strings[1]
-            .get_with_mapper(|s| crate::engine::make_getter(s.as_ref()).into(), text)
+        self.interned_strings.get_with_mapper(
+            crate::engine::FN_GET,
+            |s| crate::engine::make_getter(s.as_ref()).into(),
+            text,
+        )
     }
 
     /// Get an interned property setter, creating one if it is not yet interned.
@@ -277,8 +279,11 @@ impl<'e> ParseState<'e> {
         &mut self,
         text: impl AsRef<str> + Into<ImmutableString>,
     ) -> ImmutableString {
-        self.interned_strings[2]
-            .get_with_mapper(|s| crate::engine::make_setter(s.as_ref()).into(), text)
+        self.interned_strings.get_with_mapper(
+            crate::engine::FN_SET,
+            |s| crate::engine::make_setter(s.as_ref()).into(),
+            text,
+        )
     }
 }
 

src/tokenizer.rs

@@ -241,6 +241,8 @@ impl fmt::Display for Position {
 }
 
 impl fmt::Debug for Position {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.is_none() {
             f.write_str("none")
@@ -333,7 +335,6 @@ impl Span {
 }
 
 impl fmt::Display for Span {
-    #[inline]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let _f = f;
 
@@ -360,6 +361,8 @@ impl fmt::Display for Span {
 }
 
 impl fmt::Debug for Span {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(self, f)
     }

src/types/custom_types.rs

@@ -17,6 +17,8 @@ pub struct CustomTypeInfo {
 pub struct CustomTypesCollection(BTreeMap<Identifier, CustomTypeInfo>);
 
 impl fmt::Debug for CustomTypesCollection {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.write_str("CustomTypesCollection ")?;
         f.debug_map().entries(self.0.iter()).finish()

src/types/dynamic.rs

@@ -507,6 +507,8 @@ impl fmt::Display for Dynamic {
 }
 
 impl fmt::Debug for Dynamic {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self.0 {
             Union::Unit(ref v, ..) => fmt::Debug::fmt(v, f),

src/types/fn_ptr.rs

@@ -23,6 +23,8 @@ pub struct FnPtr {
 }
 
 impl fmt::Debug for FnPtr {
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if self.is_curried() {
             self.curry

src/types/immutable_string.rs

@@ -197,7 +197,8 @@ impl fmt::Display for ImmutableString {
 }
 
 impl fmt::Debug for ImmutableString {
-    #[inline(always)]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Debug::fmt(self.as_str(), f)
     }

src/types/interner.rs

@@ -1,3 +1,4 @@
+use super::BloomFilterU64;
 use crate::func::{hashing::get_hasher, StraightHashMap};
 use crate::ImmutableString;
 #[cfg(feature = "no_std")]
@@ -14,7 +15,7 @@ use std::{
 };
 
 /// Maximum number of strings interned.
-pub const MAX_INTERNED_STRINGS: usize = 256;
+pub const MAX_INTERNED_STRINGS: usize = 1024;
 
 /// Maximum length of strings interned.
 pub const MAX_STRING_LEN: usize = 24;
@@ -28,8 +29,10 @@ pub struct StringsInterner<'a> {
     pub capacity: usize,
     /// Maximum string length.
     pub max_string_len: usize,
-    /// Normal strings.
-    strings: StraightHashMap<ImmutableString>,
+    /// Cached strings.
+    cache: StraightHashMap<ImmutableString>,
+    /// Bloom filter to avoid caching "one-hit wonders".
+    filter: BloomFilterU64,
     /// Take care of the lifetime parameter.
     dummy: PhantomData<&'a ()>,
 }
@@ -42,9 +45,10 @@ impl Default for StringsInterner<'_> {
 }
 
 impl fmt::Debug for StringsInterner<'_> {
-    #[inline]
+    #[cold]
+    #[inline(never)]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_list().entries(self.strings.values()).finish()
+        f.debug_list().entries(self.cache.values()).finish()
     }
 }
 
@@ -56,7 +60,8 @@ impl StringsInterner<'_> {
         Self {
             capacity: MAX_INTERNED_STRINGS,
             max_string_len: MAX_STRING_LEN,
-            strings: StraightHashMap::default(),
+            cache: StraightHashMap::default(),
+            filter: BloomFilterU64::new(),
             dummy: PhantomData,
         }
     }
@@ -65,7 +70,7 @@ impl StringsInterner<'_> {
     #[inline(always)]
     #[must_use]
     pub fn get<S: AsRef<str> + Into<ImmutableString>>(&mut self, text: S) -> ImmutableString {
-        self.get_with_mapper(Into::into, text)
+        self.get_with_mapper("", Into::into, text)
     }
 
     /// Get an identifier from a text string, adding it to the interner if necessary.
@@ -73,20 +78,23 @@ impl StringsInterner<'_> {
     #[must_use]
     pub fn get_with_mapper<S: AsRef<str>>(
         &mut self,
+        id: &str,
         mapper: impl Fn(S) -> ImmutableString,
         text: S,
     ) -> ImmutableString {
         let key = text.as_ref();
 
-        if key.len() > MAX_STRING_LEN {
+        let hasher = &mut get_hasher();
+        id.hash(hasher);
+        key.hash(hasher);
+        let hash = hasher.finish();
+
+        // Cache long strings only on the second try to avoid caching "one-hit wonders".
+        if key.len() > MAX_STRING_LEN && self.filter.is_absent_and_set(hash) {
             return mapper(text);
         }
 
-        let hasher = &mut get_hasher();
-        key.hash(hasher);
-        let key = hasher.finish();
-
-        let result = match self.strings.entry(key) {
+        let result = match self.cache.entry(hash) {
             Entry::Occupied(e) => return e.get().clone(),
             Entry::Vacant(e) => {
                 let value = mapper(text);
@@ -100,7 +108,7 @@ impl StringsInterner<'_> {
         };
 
         // If the interner is over capacity, remove the longest entry that has the lowest count
-        if self.strings.len() > self.capacity {
+        if self.cache.len() > self.capacity {
             // Leave some buffer to grow when shrinking the cache.
             // We leave at least two entries, one for the empty string, and one for the string
             // that has just been inserted.
@@ -110,21 +118,21 @@ impl StringsInterner<'_> {
                 self.capacity - 3
             };
 
-            while self.strings.len() > max {
-                let (_, _, n) =
-                    self.strings
-                        .iter()
-                        .fold((0, usize::MAX, 0), |(x, c, n), (&k, v)| {
-                            if k != key
-                                && (v.strong_count() < c || (v.strong_count() == c && v.len() > x))
-                            {
-                                (v.len(), v.strong_count(), k)
-                            } else {
-                                (x, c, n)
-                            }
-                        });
+            while self.cache.len() > max {
+                let (_, _, n) = self
+                    .cache
+                    .iter()
+                    .fold((0, usize::MAX, 0), |(x, c, n), (&k, v)| {
+                        if k != hash
+                            && (v.strong_count() < c || (v.strong_count() == c && v.len() > x))
+                        {
+                            (v.len(), v.strong_count(), k)
+                        } else {
+                            (x, c, n)
+                        }
+                    });
 
-                self.strings.remove(&n);
+                self.cache.remove(&n);
             }
         }
 
@@ -136,7 +144,7 @@ impl StringsInterner<'_> {
     #[must_use]
     #[allow(dead_code)]
     pub fn len(&self) -> usize {
-        self.strings.len()
+        self.cache.len()
     }
 
     /// Returns `true` if there are no interned strings.
@@ -144,28 +152,28 @@ impl StringsInterner<'_> {
     #[must_use]
     #[allow(dead_code)]
     pub fn is_empty(&self) -> bool {
-        self.strings.is_empty()
+        self.cache.is_empty()
     }
 
     /// Clear all interned strings.
     #[inline(always)]
     #[allow(dead_code)]
     pub fn clear(&mut self) {
-        self.strings.clear();
+        self.cache.clear();
     }
 }
 
 impl AddAssign<Self> for StringsInterner<'_> {
     #[inline(always)]
     fn add_assign(&mut self, rhs: Self) {
-        self.strings.extend(rhs.strings.into_iter());
+        self.cache.extend(rhs.cache.into_iter());
     }
 }
 
 impl AddAssign<&Self> for StringsInterner<'_> {
     #[inline(always)]
     fn add_assign(&mut self, rhs: &Self) {
-        self.strings
-            .extend(rhs.strings.iter().map(|(&k, v)| (k, v.clone())));
+        self.cache
+            .extend(rhs.cache.iter().map(|(&k, v)| (k, v.clone())));
     }
 }

src/types/scope.rs

@@ -81,7 +81,6 @@ pub struct Scope<'a, const N: usize = SCOPE_ENTRIES_INLINED> {
 }
 
 impl fmt::Display for Scope<'_> {
-    #[inline]
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         for (i, (name, constant, value)) in self.iter_raw().enumerate() {
             #[cfg(not(feature = "no_closure"))]