From f70225ca1dcf747e101995b54de558ddee7b2711 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 23 Mar 2021 12:13:53 +0800 Subject: [PATCH] Change HashMap to BTreeMap. --- CHANGELOG.md | 13 +++++++ Cargo.toml | 8 +--- codegen/src/rhai_module.rs | 6 +-- src/ast.rs | 21 ++--------- src/bin/rhai-repl.rs | 1 + src/dynamic.rs | 22 ++++++++++- src/engine.rs | 38 +++++++------------ src/engine_api.rs | 8 ++-- src/lib.rs | 2 +- src/module/mod.rs | 73 ++++++++++++++---------------------- src/module/resolvers/file.rs | 6 +-- src/module/resolvers/stat.rs | 4 +- src/packages/array_basic.rs | 12 +++--- src/packages/fn_basic.rs | 30 +++++++-------- src/packages/mod.rs | 2 +- src/parser.rs | 40 ++++++++++---------- src/serde/ser.rs | 4 +- src/stdlib.rs | 3 +- src/utils.rs | 60 +---------------------------- 19 files changed, 139 insertions(+), 214 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9055d3df..ad8a65cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,21 @@ Rhai Release Notes Version 0.19.15 =============== +This version replaces internal usage of `HashMap` with `BTreeMap` in many cases, which should result +in speed improvements because a `BTreeMap` is faster when the number of items held is small. + +This also translates to the Rhai object map type, `Map`, which used to be an alias to `HashMap` and +is now aliased to `BTreeMap` instead. This change is due to the fact that, in the vast majority of +object map usages, the number of properties held is small. + +`HashMap` and `BTreeMap` have almost identical public API's so this change is unlikely to break a +lot of existing code. + + Breaking changes ---------------- +* `Map` is now an alias to `BTreeMap` instead of `HashMap`. This is because most object maps used have few properties. * The traits `RegisterFn` and `RegisterResultFn` are removed. `Engine::register_fn` and `Engine::register_result_fn` are now implemented directly on `Engine`. * `FnPtr::call_dynamic` now takes `&NativeCallContext` instead of consuming it. * All `Module::set_fn_XXX` methods are removed, in favor of `Module::set_native_fn`. @@ -16,6 +28,7 @@ Breaking changes Enhancements ------------ +* Replaced most `HashMap` usage with `BTreeMap` for better performance when the number of items is small. * `Engine::register_result_fn` no longer requires the successful return type to be `Dynamic`. It can now be any clonable type. * `#[rhai_fn(return_raw)]` can now return `Result>` where `T` is any clonable type instead of `Result>`. diff --git a/Cargo.toml b/Cargo.toml index c95fc996..849e2e1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,7 +40,7 @@ internals = [] # expose internal data structures unicode-xid-ident = ["unicode-xid"] # allow Unicode Standard Annex #31 for identifiers. metadata = ["serde", "serde_json"] # enables exporting functions metadata to JSON -no_std = ["smallvec/union", "num-traits/libm", "hashbrown", "core-error", "libm", "ahash/compile-time-rng"] +no_std = ["smallvec/union", "num-traits/libm", "core-error", "libm", "ahash/compile-time-rng"] # compiling for WASM wasm-bindgen = ["instant/wasm-bindgen"] @@ -63,12 +63,6 @@ default_features = false features = ["alloc"] optional = true -[dependencies.hashbrown] -version = "0.11" -default-features = false -features = ["ahash", "nightly", "inline-more"] -optional = true - [dependencies.serde] version = "1.0" default_features = false diff --git a/codegen/src/rhai_module.rs b/codegen/src/rhai_module.rs index 1b83a407..b1eb1947 100644 --- a/codegen/src/rhai_module.rs +++ b/codegen/src/rhai_module.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::BTreeMap; use quote::{quote, ToTokens}; @@ -238,8 +238,8 @@ pub fn check_rename_collisions(fns: &Vec) -> Result<(), syn::Error> }) } - let mut renames = HashMap::::new(); - let mut fn_defs = HashMap::::new(); + let mut renames = BTreeMap::::new(); + let mut fn_defs = BTreeMap::::new(); for item_fn in fns.iter() { if !item_fn.params().name.is_empty() || item_fn.params().special != FnSpecialAccess::None { diff --git a/src/ast.rs b/src/ast.rs index 5ca155a4..f420ae7a 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -6,6 +6,7 @@ use crate::module::NamespaceRef; use crate::stdlib::{ borrow::Cow, boxed::Box, + collections::BTreeMap, fmt, hash::Hash, num::NonZeroUsize, @@ -15,7 +16,6 @@ use crate::stdlib::{ vec::Vec, }; use crate::token::Token; -use crate::utils::{HashableHashMap, StraightHasherBuilder}; use crate::{ Dynamic, FnNamespace, FnPtr, ImmutableString, Module, Position, Shared, StaticVec, INT, }; @@ -866,14 +866,7 @@ pub enum Stmt { /// `if` expr `{` stmt `}` `else` `{` stmt `}` If(Expr, Box<(StmtBlock, StmtBlock)>, Position), /// `switch` expr `{` literal or _ `=>` stmt `,` ... `}` - Switch( - Expr, - Box<( - HashableHashMap, - StmtBlock, - )>, - Position, - ), + Switch(Expr, Box<(BTreeMap, StmtBlock)>, Position), /// `while` expr `{` stmt `}` While(Expr, Box, Position), /// `do` `{` stmt `}` `while`|`until` expr @@ -1594,20 +1587,14 @@ impl Expr { #[cfg(not(feature = "no_index"))] Self::Array(x, _) if self.is_constant() => { - let mut arr = Array::with_capacity(crate::stdlib::cmp::max( - crate::engine::TYPICAL_ARRAY_SIZE, - x.len(), - )); + let mut arr = Array::with_capacity(x.len()); arr.extend(x.iter().map(|v| v.get_constant_value().unwrap())); Dynamic(Union::Array(Box::new(arr), AccessMode::ReadOnly)) } #[cfg(not(feature = "no_object"))] Self::Map(x, _) if self.is_constant() => { - let mut map = Map::with_capacity(crate::stdlib::cmp::max( - crate::engine::TYPICAL_MAP_SIZE, - x.len(), - )); + let mut map = Map::new(); map.extend( x.iter() .map(|(k, v)| (k.name.clone(), v.get_constant_value().unwrap())), diff --git a/src/bin/rhai-repl.rs b/src/bin/rhai-repl.rs index 8a03d85b..291842e2 100644 --- a/src/bin/rhai-repl.rs +++ b/src/bin/rhai-repl.rs @@ -61,6 +61,7 @@ fn main() { let mut engine = Engine::new(); #[cfg(not(feature = "no_module"))] + #[cfg(not(feature = "no_std"))] { // Set a file module resolver without caching let mut resolver = rhai::module_resolvers::FileModuleResolver::new(); diff --git a/src/dynamic.rs b/src/dynamic.rs index 4b52e89e..014a0ed9 100644 --- a/src/dynamic.rs +++ b/src/dynamic.rs @@ -812,8 +812,8 @@ impl Dynamic { /// A [`Vec`][Vec] does not get automatically converted to an [`Array`], but will be a generic /// restricted trait object instead, because [`Vec`][Vec] is not a supported standard type. /// - /// Similarly, passing in a [`HashMap`][std::collections::HashMap] will not get a [`Map`] - /// but a trait object. + /// Similarly, passing in a [`HashMap`][std::collections::HashMap] or + /// [`BTreeMap`][std::collections::BTreeMap] will not get a [`Map`] but a trait object. /// /// # Examples /// @@ -1696,6 +1696,7 @@ impl crate::stdlib::iter::FromIterator for Dynamic { } } #[cfg(not(feature = "no_object"))] +#[cfg(not(feature = "no_std"))] impl, T: Variant + Clone> From> for Dynamic { @@ -1712,6 +1713,23 @@ impl, T: Variant + Clone> From, T: Variant + Clone> From> + for Dynamic +{ + #[inline(always)] + fn from(value: crate::stdlib::collections::BTreeMap) -> Self { + Self(Union::Map( + Box::new( + value + .into_iter() + .map(|(k, v)| (k.into(), Dynamic::from(v))) + .collect(), + ), + AccessMode::ReadWrite, + )) + } +} impl From for Dynamic { #[inline(always)] fn from(value: FnPtr) -> Self { diff --git a/src/engine.rs b/src/engine.rs index b2a3dcfb..4d1a9541 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -14,7 +14,7 @@ use crate::stdlib::{ any::{type_name, TypeId}, borrow::Cow, boxed::Box, - collections::{HashMap, HashSet}, + collections::{BTreeMap, BTreeSet}, fmt, format, hash::{Hash, Hasher}, num::{NonZeroU8, NonZeroUsize}, @@ -23,7 +23,7 @@ use crate::stdlib::{ vec::Vec, }; use crate::syntax::CustomSyntax; -use crate::utils::{get_hasher, StraightHasherBuilder}; +use crate::utils::get_hasher; use crate::{ Dynamic, EvalAltResult, FnPtr, ImmutableString, Module, Position, RhaiResult, Scope, Shared, StaticVec, @@ -32,15 +32,9 @@ use crate::{ #[cfg(not(feature = "no_index"))] use crate::{calc_fn_hash, stdlib::iter::empty, Array}; -#[cfg(not(feature = "no_index"))] -pub const TYPICAL_ARRAY_SIZE: usize = 8; // Small arrays are typical - #[cfg(not(feature = "no_object"))] use crate::Map; -#[cfg(not(feature = "no_object"))] -pub const TYPICAL_MAP_SIZE: usize = 8; // Small maps are typical - pub type Precedence = NonZeroU8; /// _(INTERNALS)_ A stack of imported [modules][Module]. @@ -501,7 +495,7 @@ pub struct FnResolutionCacheEntry { } /// A function resolution cache. -pub type FnResolutionCache = HashMap, StraightHasherBuilder>; +pub type FnResolutionCache = BTreeMap>; /// _(INTERNALS)_ A type that holds all the current states of the [`Engine`]. /// Exported under the `internals` feature only. @@ -540,9 +534,7 @@ impl State { /// Get a mutable reference to the current function resolution cache. pub fn fn_resolution_cache_mut(&mut self) -> &mut FnResolutionCache { if self.fn_resolution_caches.0.is_empty() { - self.fn_resolution_caches - .0 - .push(HashMap::with_capacity_and_hasher(64, StraightHasherBuilder)); + self.fn_resolution_caches.0.push(BTreeMap::new()); } self.fn_resolution_caches.0.last_mut().unwrap() } @@ -711,21 +703,21 @@ pub struct Engine { /// A collection of all modules loaded into the global namespace of the Engine. pub(crate) global_modules: StaticVec>, /// A collection of all sub-modules directly loaded into the Engine. - pub(crate) global_sub_modules: HashMap>, + pub(crate) global_sub_modules: BTreeMap>, /// A module resolution service. #[cfg(not(feature = "no_module"))] pub(crate) module_resolver: Box, - /// A hashmap mapping type names to pretty-print names. - pub(crate) type_names: HashMap, + /// A map mapping type names to pretty-print names. + pub(crate) type_names: BTreeMap, - /// A hashset containing symbols to disable. - pub(crate) disabled_symbols: HashSet, - /// A hashmap containing custom keywords and precedence to recognize. - pub(crate) custom_keywords: HashMap>, + /// A set of symbols to disable. + pub(crate) disabled_symbols: BTreeSet, + /// A map containing custom keywords and precedence to recognize. + pub(crate) custom_keywords: BTreeMap>, /// Custom syntax. - pub(crate) custom_syntax: HashMap, + pub(crate) custom_syntax: BTreeMap, /// Callback closure for resolving variable access. pub(crate) resolve_var: Option, @@ -1682,8 +1674,7 @@ impl Engine { #[cfg(not(feature = "no_index"))] Expr::Array(x, _) => { - let mut arr = - Array::with_capacity(crate::stdlib::cmp::max(TYPICAL_ARRAY_SIZE, x.len())); + let mut arr = Array::with_capacity(x.len()); for item in x.as_ref() { arr.push( self.eval_expr(scope, mods, state, lib, this_ptr, item, level)? @@ -1695,8 +1686,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] Expr::Map(x, _) => { - let mut map = - Map::with_capacity(crate::stdlib::cmp::max(TYPICAL_MAP_SIZE, x.len())); + let mut map = Map::new(); for (Ident { name: key, .. }, expr) in x.as_ref() { map.insert( key.clone(), diff --git a/src/engine_api.rs b/src/engine_api.rs index 1f80b747..86c523d7 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -880,7 +880,7 @@ impl Engine { #[cfg(not(feature = "no_module"))] pub fn register_static_module(&mut self, name: &str, module: Shared) -> &mut Self { fn register_static_module_raw( - root: &mut crate::stdlib::collections::HashMap>, + root: &mut crate::stdlib::collections::BTreeMap>, name: &str, module: Shared, ) { @@ -1012,14 +1012,14 @@ impl Engine { ast::{ASTNode, Expr, Stmt}, fn_native::shared_take_or_clone, module::resolvers::StaticModuleResolver, - stdlib::collections::HashSet, + stdlib::collections::BTreeSet, ImmutableString, }; fn collect_imports( ast: &AST, resolver: &StaticModuleResolver, - imports: &mut HashSet, + imports: &mut BTreeSet, ) { ast.walk(&mut |path| match path.last().unwrap() { // Collect all `import` statements with a string constant path @@ -1035,7 +1035,7 @@ impl Engine { let mut resolver = StaticModuleResolver::new(); let mut ast = self.compile_scripts_with_scope(scope, &[script])?; - let mut imports = HashSet::::new(); + let mut imports = Default::default(); collect_imports(&ast, &mut resolver, &mut imports); diff --git a/src/lib.rs b/src/lib.rs index 982f3e01..3a880f4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -164,7 +164,7 @@ pub type Array = stdlib::vec::Vec; /// /// Not available under `no_object`. #[cfg(not(feature = "no_object"))] -pub type Map = stdlib::collections::HashMap; +pub type Map = stdlib::collections::BTreeMap; #[cfg(not(feature = "no_module"))] pub use module::ModuleResolver; diff --git a/src/module/mod.rs b/src/module/mod.rs index 123c2261..13b8c405 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -7,7 +7,7 @@ use crate::fn_register::RegisterNativeFunction; use crate::stdlib::{ any::TypeId, boxed::Box, - collections::HashMap, + collections::BTreeMap, fmt, format, iter::empty, num::NonZeroUsize, @@ -16,7 +16,6 @@ use crate::stdlib::{ vec::Vec, }; use crate::token::Token; -use crate::utils::StraightHasherBuilder; use crate::{ calc_fn_hash, calc_fn_params_hash, combine_hashes, Dynamic, EvalAltResult, ImmutableString, NativeCallContext, Position, Shared, StaticVec, @@ -130,20 +129,20 @@ pub struct Module { /// ID identifying the module. id: Option, /// Sub-modules. - modules: HashMap>, + modules: BTreeMap>, /// [`Module`] variables. - variables: HashMap, + variables: BTreeMap, /// Flattened collection of all [`Module`] variables, including those in sub-modules. - all_variables: HashMap, + all_variables: BTreeMap, /// External Rust functions. - functions: HashMap, StraightHasherBuilder>, + functions: BTreeMap>, /// Flattened collection of all external Rust functions, native or scripted. /// including those in sub-modules. - all_functions: HashMap, + all_functions: BTreeMap, /// Iterator functions, keyed by the type producing the iterator. - type_iterators: HashMap, + type_iterators: BTreeMap, /// Flattened collection of iterator functions, including those in sub-modules. - all_type_iterators: HashMap, + all_type_iterators: BTreeMap, /// Is the [`Module`] indexed? indexed: bool, /// Does the [`Module`] contain indexed functions that have been exposed to the global namespace? @@ -158,8 +157,8 @@ impl Default for Module { modules: Default::default(), variables: Default::default(), all_variables: Default::default(), - functions: HashMap::with_capacity_and_hasher(64, StraightHasherBuilder), - all_functions: HashMap::with_capacity_and_hasher(256, StraightHasherBuilder), + functions: Default::default(), + all_functions: Default::default(), type_iterators: Default::default(), all_type_iterators: Default::default(), indexed: false, @@ -270,25 +269,6 @@ impl Module { Default::default() } - /// Create a new [`Module`] with a specified capacity for native Rust functions. - /// - /// # Example - /// - /// ``` - /// use rhai::Module; - /// - /// let mut module = Module::new(); - /// module.set_var("answer", 42_i64); - /// assert_eq!(module.get_var_value::("answer").unwrap(), 42); - /// ``` - #[inline(always)] - pub fn new_with_capacity(capacity: usize) -> Self { - Self { - functions: HashMap::with_capacity_and_hasher(capacity, StraightHasherBuilder), - ..Default::default() - } - } - /// Get the ID of the [`Module`], if any. /// /// # Example @@ -520,7 +500,7 @@ impl Module { .map(|f| f.func.get_fn_def()) } - /// Get a mutable reference to the underlying [`HashMap`] of sub-modules. + /// Get a mutable reference to the underlying [`BTreeMap`] of sub-modules. /// /// # WARNING /// @@ -528,7 +508,7 @@ impl Module { /// Thus the [`Module`] is automatically set to be non-indexed. #[cfg(not(feature = "no_module"))] #[inline(always)] - pub(crate) fn sub_modules_mut(&mut self) -> &mut HashMap> { + pub(crate) fn sub_modules_mut(&mut self) -> &mut BTreeMap> { // We must assume that the user has changed the sub-modules // (otherwise why take a mutable reference?) self.all_functions.clear(); @@ -1235,13 +1215,16 @@ impl Module { &mut self, filter: impl Fn(FnNamespace, FnAccess, &str, usize) -> bool, ) -> &mut Self { - self.functions.retain(|_, f| { - if f.func.is_script() { - filter(f.namespace, f.access, f.name.as_str(), f.params) - } else { - false - } - }); + self.functions = crate::stdlib::mem::take(&mut self.functions) + .into_iter() + .filter(|(_, f)| { + if f.func.is_script() { + filter(f.namespace, f.access, f.name.as_str(), f.params) + } else { + false + } + }) + .collect(); self.all_functions.clear(); self.all_variables.clear(); @@ -1460,9 +1443,9 @@ impl Module { fn index_module<'a>( module: &'a Module, path: &mut Vec<&'a str>, - variables: &mut HashMap, - functions: &mut HashMap, - type_iterators: &mut HashMap, + variables: &mut BTreeMap, + functions: &mut BTreeMap, + type_iterators: &mut BTreeMap, ) -> bool { let mut contains_indexed_global_functions = false; @@ -1518,9 +1501,9 @@ impl Module { if !self.indexed { let mut path = Vec::with_capacity(4); - let mut variables = HashMap::with_capacity_and_hasher(16, StraightHasherBuilder); - let mut functions = HashMap::with_capacity_and_hasher(256, StraightHasherBuilder); - let mut type_iterators = HashMap::with_capacity(16); + let mut variables = Default::default(); + let mut functions = Default::default(); + let mut type_iterators = Default::default(); path.push("root"); diff --git a/src/module/resolvers/file.rs b/src/module/resolvers/file.rs index e3a86634..79ba4cf0 100644 --- a/src/module/resolvers/file.rs +++ b/src/module/resolvers/file.rs @@ -1,6 +1,6 @@ use crate::stdlib::{ boxed::Box, - collections::HashMap, + collections::BTreeMap, io::Error as IoError, path::{Path, PathBuf}, string::String, @@ -44,9 +44,9 @@ pub struct FileModuleResolver { cache_enabled: bool, #[cfg(not(feature = "sync"))] - cache: crate::stdlib::cell::RefCell>>, + cache: crate::stdlib::cell::RefCell>>, #[cfg(feature = "sync")] - cache: crate::stdlib::sync::RwLock>>, + cache: crate::stdlib::sync::RwLock>>, } impl Default for FileModuleResolver { diff --git a/src/module/resolvers/stat.rs b/src/module/resolvers/stat.rs index 4b0c0f36..89cc458f 100644 --- a/src/module/resolvers/stat.rs +++ b/src/module/resolvers/stat.rs @@ -1,4 +1,4 @@ -use crate::stdlib::{boxed::Box, collections::HashMap, ops::AddAssign, string::String}; +use crate::stdlib::{boxed::Box, collections::BTreeMap, ops::AddAssign, string::String}; use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; /// A static [module][Module] resolution service that serves [modules][Module] added into it. @@ -19,7 +19,7 @@ use crate::{Engine, EvalAltResult, Module, ModuleResolver, Position, Shared}; /// engine.set_module_resolver(resolver); /// ``` #[derive(Debug, Clone, Default)] -pub struct StaticModuleResolver(HashMap>); +pub struct StaticModuleResolver(BTreeMap>); impl StaticModuleResolver { /// Create a new [`StaticModuleResolver`]. diff --git a/src/packages/array_basic.rs b/src/packages/array_basic.rs index 0c4701a7..2708305a 100644 --- a/src/packages/array_basic.rs +++ b/src/packages/array_basic.rs @@ -1,9 +1,9 @@ #![cfg(not(feature = "no_index"))] #![allow(non_snake_case)] -use crate::engine::{OP_EQUALS, TYPICAL_ARRAY_SIZE}; +use crate::engine::OP_EQUALS; use crate::plugin::*; -use crate::stdlib::{any::TypeId, boxed::Box, cmp::max, cmp::Ordering, mem, string::ToString}; +use crate::stdlib::{any::TypeId, boxed::Box, cmp::Ordering, mem, string::ToString}; use crate::{def_package, Array, Dynamic, EvalAltResult, FnPtr, NativeCallContext, Position, INT}; def_package!(crate:BasicArrayPackage:"Basic array utilities.", lib, { @@ -170,7 +170,7 @@ mod array_functions { array: &mut Array, mapper: FnPtr, ) -> Result> { - let mut ar = Array::with_capacity(max(TYPICAL_ARRAY_SIZE, array.len())); + let mut ar = Array::with_capacity(array.len()); for (i, item) in array.iter().enumerate() { ar.push( @@ -203,7 +203,7 @@ mod array_functions { array: &mut Array, filter: FnPtr, ) -> Result> { - let mut ar = Array::with_capacity(max(TYPICAL_ARRAY_SIZE, array.len())); + let mut ar = Array::new(); for (i, item) in array.iter().enumerate() { if filter @@ -565,7 +565,7 @@ mod array_functions { array: &mut Array, filter: FnPtr, ) -> Result> { - let mut drained = Array::with_capacity(max(TYPICAL_ARRAY_SIZE, array.len())); + let mut drained = Array::with_capacity(array.len()); let mut i = array.len(); @@ -625,7 +625,7 @@ mod array_functions { array: &mut Array, filter: FnPtr, ) -> Result> { - let mut drained = Array::with_capacity(max(TYPICAL_ARRAY_SIZE, array.len())); + let mut drained = Array::new(); let mut i = array.len(); diff --git a/src/packages/fn_basic.rs b/src/packages/fn_basic.rs index c6eab910..65ae4469 100644 --- a/src/packages/fn_basic.rs +++ b/src/packages/fn_basic.rs @@ -34,34 +34,34 @@ mod fn_ptr_functions { #[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] fn collect_fn_metadata(ctx: NativeCallContext) -> crate::Array { - use crate::{ast::ScriptFnDef, stdlib::collections::HashMap, Array, Map}; + use crate::{ast::ScriptFnDef, stdlib::collections::BTreeSet, Array, Map}; // Create a metadata record for a function. fn make_metadata( - dict: &HashMap<&str, ImmutableString>, + dict: &BTreeSet, namespace: Option, f: &ScriptFnDef, ) -> Map { - let mut map = Map::with_capacity(6); + let mut map = Map::new(); if let Some(ns) = namespace { - map.insert(dict["namespace"].clone(), ns.into()); + map.insert(dict.get("namespace").unwrap().clone(), ns.into()); } - map.insert(dict["name"].clone(), f.name.clone().into()); + map.insert(dict.get("name").unwrap().clone(), f.name.clone().into()); map.insert( - dict["access"].clone(), + dict.get("access").unwrap().clone(), match f.access { - FnAccess::Public => dict["public"].clone(), - FnAccess::Private => dict["private"].clone(), + FnAccess::Public => dict.get("public").unwrap().clone(), + FnAccess::Private => dict.get("private").unwrap().clone(), } .into(), ); map.insert( - dict["is_anonymous"].clone(), + dict.get("is_anonymous").unwrap().clone(), f.name.starts_with(crate::engine::FN_ANONYMOUS).into(), ); map.insert( - dict["params"].clone(), + dict.get("params").unwrap().clone(), f.params .iter() .cloned() @@ -74,8 +74,7 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> crate::Array { } // Intern strings - let mut dict = HashMap::<&str, ImmutableString>::with_capacity(8); - [ + let dict: BTreeSet = [ "namespace", "name", "access", @@ -85,9 +84,8 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> crate::Array { "params", ] .iter() - .for_each(|&s| { - dict.insert(s, s.into()); - }); + .map(|&s| s.into()) + .collect(); let mut list: Array = Default::default(); @@ -100,7 +98,7 @@ fn collect_fn_metadata(ctx: NativeCallContext) -> crate::Array { // Recursively scan modules for script-defined functions. fn scan_module( list: &mut Array, - dict: &HashMap<&str, ImmutableString>, + dict: &BTreeSet, namespace: ImmutableString, module: &Module, ) { diff --git a/src/packages/mod.rs b/src/packages/mod.rs index e54763da..7524ec3d 100644 --- a/src/packages/mod.rs +++ b/src/packages/mod.rs @@ -89,7 +89,7 @@ macro_rules! def_package { impl $package { pub fn new() -> Self { - let mut module = $root::Module::new_with_capacity(1024); + let mut module = $root::Module::new(); ::init(&mut module); module.build_index(); Self(module.into()) diff --git a/src/parser.rs b/src/parser.rs index 21ae4114..de7a507e 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -12,7 +12,7 @@ use crate::optimize::OptimizationLevel; use crate::stdlib::{ borrow::Cow, boxed::Box, - collections::HashMap, + collections::BTreeMap, format, hash::{Hash, Hasher}, iter::empty, @@ -23,7 +23,7 @@ use crate::stdlib::{ }; use crate::syntax::{CustomSyntax, MARKER_BLOCK, MARKER_EXPR, MARKER_IDENT}; use crate::token::{is_keyword_function, is_valid_identifier, Token, TokenStream}; -use crate::utils::{get_hasher, StraightHasherBuilder}; +use crate::utils::get_hasher; use crate::{ calc_fn_hash, Dynamic, Engine, ImmutableString, LexError, ParseError, ParseErrorType, Position, Scope, Shared, StaticVec, AST, @@ -37,7 +37,7 @@ use crate::FnAccess; type PERR = ParseErrorType; -type FunctionsLib = HashMap, StraightHasherBuilder>; +type FunctionsLib = BTreeMap>; /// A type that encapsulates the current state of the parser. #[derive(Debug)] @@ -45,14 +45,14 @@ struct ParseState<'e> { /// Reference to the scripting [`Engine`]. engine: &'e Engine, /// Interned strings. - strings: HashMap, + interned_strings: BTreeMap, /// Encapsulates a local stack with variable names to simulate an actual runtime scope. stack: Vec<(ImmutableString, AccessMode)>, /// Size of the local variables stack upon entry of the current block scope. entry_stack_len: usize, /// Tracks a list of external variables (variables that are not explicitly declared in the scope). #[cfg(not(feature = "no_closure"))] - externals: HashMap, + external_vars: BTreeMap, /// An indicator that disables variable capturing into externals one single time /// up until the nearest consumed Identifier token. /// If set to false the next call to `access_var` will not capture the variable. @@ -89,10 +89,10 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_function"))] max_function_expr_depth, #[cfg(not(feature = "no_closure"))] - externals: Default::default(), + external_vars: Default::default(), #[cfg(not(feature = "no_closure"))] allow_capture: true, - strings: HashMap::with_capacity(64), + interned_strings: Default::default(), stack: Vec::with_capacity(16), entry_stack_len: 0, #[cfg(not(feature = "no_module"))] @@ -130,8 +130,8 @@ impl<'e> ParseState<'e> { #[cfg(not(feature = "no_closure"))] if self.allow_capture { - if index.is_none() && !self.externals.contains_key(name) { - self.externals.insert(name.into(), _pos); + if index.is_none() && !self.external_vars.contains_key(name) { + self.external_vars.insert(name.into(), _pos); } } else { self.allow_capture = true @@ -171,12 +171,13 @@ impl<'e> ParseState<'e> { text: impl AsRef + Into, ) -> ImmutableString { #[allow(clippy::map_entry)] - if !self.strings.contains_key(text.as_ref()) { + if !self.interned_strings.contains_key(text.as_ref()) { let value = text.into(); - self.strings.insert(value.clone().into(), value.clone()); + self.interned_strings + .insert(value.clone().into(), value.clone()); value } else { - self.strings.get(text.as_ref()).unwrap().clone() + self.interned_strings.get(text.as_ref()).unwrap().clone() } } } @@ -813,7 +814,7 @@ fn parse_switch( } } - let mut table = HashMap::::new(); + let mut table = BTreeMap::::new(); let mut def_stmt = None; loop { @@ -902,13 +903,10 @@ fn parse_switch( } } - let mut final_table = HashMap::with_capacity_and_hasher(table.len(), StraightHasherBuilder); - final_table.extend(table.into_iter()); - Ok(Stmt::Switch( item, Box::new(( - final_table.into(), + table, def_stmt.unwrap_or_else(|| Stmt::Noop(Position::NONE).into()), )), settings.pos, @@ -1003,7 +1001,7 @@ fn parse_primary( let (expr, func) = parse_anon_fn(input, &mut new_state, lib, settings)?; #[cfg(not(feature = "no_closure"))] - new_state.externals.iter().for_each(|(closure, pos)| { + new_state.external_vars.iter().for_each(|(closure, pos)| { state.access_var(closure, *pos); }); @@ -2739,7 +2737,7 @@ fn parse_fn( #[cfg(not(feature = "no_closure"))] let externals = state - .externals + .external_vars .iter() .map(|(name, _)| name) .filter(|name| !params.contains(name)) @@ -2860,7 +2858,7 @@ fn parse_anon_fn( #[cfg(not(feature = "no_closure"))] { state - .externals + .external_vars .iter() .map(|(name, &pos)| Ident { name: name.clone(), @@ -2966,7 +2964,7 @@ impl Engine { input: &mut TokenStream, ) -> Result<(Vec, Vec>), ParseError> { let mut statements = Vec::with_capacity(16); - let mut functions = HashMap::with_capacity_and_hasher(16, StraightHasherBuilder); + let mut functions = BTreeMap::new(); let mut state = ParseState::new( self, #[cfg(not(feature = "unchecked"))] diff --git a/src/serde/ser.rs b/src/serde/ser.rs index 5f169431..6b0d221e 100644 --- a/src/serde/ser.rs +++ b/src/serde/ser.rs @@ -390,7 +390,7 @@ impl Serializer for &mut DynamicSerializer { #[cfg(not(feature = "no_object"))] return Ok(StructVariantSerializer { variant: _variant, - map: Map::with_capacity(_len), + map: Default::default(), }); #[cfg(feature = "no_object")] return EvalAltResult::ErrorMismatchDataType( @@ -691,7 +691,7 @@ impl serde::ser::SerializeStructVariant for StructVariantSerializer { #[cfg(not(feature = "no_object"))] fn make_variant(variant: &'static str, value: Dynamic) -> RhaiResult { - let mut map = Map::with_capacity(1); + let mut map = Map::new(); map.insert(variant.into(), value); Ok(map.into()) } diff --git a/src/stdlib.rs b/src/stdlib.rs index d1aa2f43..e0a32470 100644 --- a/src/stdlib.rs +++ b/src/stdlib.rs @@ -19,7 +19,8 @@ mod inner { pub use core_error as error; pub mod collections { - pub use hashbrown::{hash_map, hash_set, HashMap, HashSet}; + pub use alloc::collections::btree_map::BTreeMap; + pub use alloc::collections::btree_set::BTreeSet; } } diff --git a/src/utils.rs b/src/utils.rs index 3bdc9250..6be824bf 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,15 +6,13 @@ use crate::stdlib::{ borrow::Borrow, boxed::Box, cmp::Ordering, - collections::HashMap, fmt, fmt::{Debug, Display}, hash::{BuildHasher, Hash, Hasher}, iter::FromIterator, - ops::{Add, AddAssign, Deref, DerefMut, Sub, SubAssign}, + ops::{Add, AddAssign, Deref, Sub, SubAssign}, str::FromStr, string::{String, ToString}, - vec::Vec, }; use crate::Shared; @@ -106,62 +104,6 @@ pub(crate) fn combine_hashes(a: u64, b: u64) -> u64 { a ^ b } -/// _(INTERNALS)_ A type that wraps a [`HashMap`] and implements [`Hash`]. -/// Exported under the `internals` feature only. -#[derive(Clone, Default)] -pub struct HashableHashMap(HashMap); - -impl From> for HashableHashMap { - #[inline(always)] - fn from(value: HashMap) -> Self { - Self(value) - } -} -impl AsRef> for HashableHashMap { - #[inline(always)] - fn as_ref(&self) -> &HashMap { - &self.0 - } -} -impl AsMut> for HashableHashMap { - #[inline(always)] - fn as_mut(&mut self) -> &mut HashMap { - &mut self.0 - } -} -impl Deref for HashableHashMap { - type Target = HashMap; - - #[inline(always)] - fn deref(&self) -> &Self::Target { - &self.0 - } -} -impl DerefMut for HashableHashMap { - #[inline(always)] - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } -} -impl Debug for HashableHashMap { - #[inline(always)] - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} -impl Hash for HashableHashMap { - #[inline(always)] - fn hash(&self, state: &mut B) { - let mut keys: Vec<_> = self.0.keys().collect(); - keys.sort(); - - keys.into_iter().for_each(|key| { - key.hash(state); - self.0.get(&key).unwrap().hash(state); - }); - } -} - /// The system immutable string type. /// /// An [`ImmutableString`] wraps an [`Rc`][std::rc::Rc]`<`[`String`]`>`