Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaces HashSet and HashMap with FxHashSet and FxHashMap #165

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 11 additions & 15 deletions crates/steel-core/src/compiler/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,9 @@ use crate::{
parser::parser::Sources,
};

use std::path::PathBuf;
use std::{borrow::Cow, iter::Iterator};
use std::{
collections::{HashMap, HashSet},
path::PathBuf,
};

// TODO: Replace the usages of hashmap with this directly
use fxhash::{FxBuildHasher, FxHashMap, FxHashSet};
use serde::{Deserialize, Serialize};

Expand Down Expand Up @@ -61,8 +57,8 @@ use std::time::Instant;

#[derive(Default)]
pub struct DebruijnIndicesInterner {
flat_defines: HashSet<InternedString>,
second_pass_defines: HashSet<InternedString>,
flat_defines: FxHashSet<InternedString>,
second_pass_defines: FxHashSet<InternedString>,
}

impl DebruijnIndicesInterner {
Expand Down Expand Up @@ -273,7 +269,7 @@ pub enum OptLevel {
#[derive(Clone)]
pub(crate) struct KernelDefMacroSpec {
pub(crate) _env: String,
pub(crate) _exported: Option<HashSet<InternedString>>,
pub(crate) _exported: Option<FxHashSet<InternedString>>,
pub(crate) name_mangler: NameMangler,
}

Expand All @@ -288,11 +284,11 @@ pub struct Compiler {
memoization_table: MemoizationTable,
mangled_identifiers: FxHashSet<InternedString>,
// Try this out?
lifted_kernel_environments: HashMap<String, KernelDefMacroSpec>,
lifted_kernel_environments: FxHashMap<String, KernelDefMacroSpec>,
// Macros that... we need to compile against directly at the top level
// This is really just a hack, but it solves cases for interactively
// running at the top level using private macros.
lifted_macro_environments: HashSet<PathBuf>,
lifted_macro_environments: FxHashSet<PathBuf>,

analysis: Analysis,
shadowed_variable_renamer: RenameShadowedVariables,
Expand Down Expand Up @@ -362,8 +358,8 @@ impl Compiler {
kernel: None,
memoization_table: MemoizationTable::new(),
mangled_identifiers: FxHashSet::default(),
lifted_kernel_environments: HashMap::new(),
lifted_macro_environments: HashSet::new(),
lifted_kernel_environments: FxHashMap::default(),
lifted_macro_environments: FxHashSet::default(),
analysis: Analysis::pre_allocated(),
shadowed_variable_renamer: RenameShadowedVariables::default(),
search_dirs: Vec::new(),
Expand All @@ -386,8 +382,8 @@ impl Compiler {
kernel: Some(kernel),
memoization_table: MemoizationTable::new(),
mangled_identifiers: FxHashSet::default(),
lifted_kernel_environments: HashMap::new(),
lifted_macro_environments: HashSet::new(),
lifted_kernel_environments: FxHashMap::default(),
lifted_macro_environments: FxHashSet::default(),
analysis: Analysis::pre_allocated(),
shadowed_variable_renamer: RenameShadowedVariables::default(),
search_dirs: Vec::new(),
Expand Down Expand Up @@ -1013,7 +1009,7 @@ impl Compiler {

fn _run_const_evaluation_with_memoization(
&mut self,
_constants: ImmutableHashMap<InternedString, SteelVal>,
_constants: ImmutableHashMap<InternedString, SteelVal, FxBuildHasher>,
mut _expanded_statements: Vec<ExprKind>,
) -> Result<Vec<ExprKind>> {
todo!("Implement kernel level const evaluation here!")
Expand Down
10 changes: 5 additions & 5 deletions crates/steel-core/src/compiler/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use crate::parser::{
parser::{ParseError, Parser},
};

use std::collections::HashMap;
use std::{cell::RefCell, rc::Rc};

use fxhash::{FxHashMap, FxHashSet};
// TODO add the serializing and deserializing for constants
use serde::{Deserialize, Serialize};
use steel_parser::parser::lower_entire_ast;
Expand All @@ -17,7 +17,7 @@ use steel_parser::parser::lower_entire_ast;
// underlying representation.
#[derive(Debug, PartialEq)]
pub struct ConstantMap {
map: Rc<RefCell<HashMap<SteelVal, usize>>>,
map: Rc<RefCell<FxHashMap<SteelVal, usize>>>,
values: Rc<RefCell<Vec<SteelVal>>>,
}

Expand Down Expand Up @@ -47,7 +47,7 @@ impl ConstantMap {
pub fn new() -> ConstantMap {
ConstantMap {
values: Rc::new(RefCell::new(Vec::new())),
map: Rc::new(RefCell::new(HashMap::new())),
map: Rc::new(RefCell::new(FxHashMap::default())),
}
}

Expand All @@ -57,8 +57,8 @@ impl ConstantMap {

pub fn to_serializable_vec(
&self,
serializer: &mut std::collections::HashMap<usize, SerializableSteelVal>,
visited: &mut std::collections::HashSet<usize>,
serializer: &mut FxHashMap<usize, SerializableSteelVal>,
visited: &mut FxHashSet<usize>,
) -> Vec<SerializableSteelVal> {
self.values
.borrow()
Expand Down
6 changes: 3 additions & 3 deletions crates/steel-core/src/compiler/map.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::throw;
use crate::{parser::interner::InternedString, rvals::Result};
use fxhash::FxHashMap;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
pub struct SymbolMap {
values: Vec<InternedString>,
map: HashMap<InternedString, usize>,
map: FxHashMap<InternedString, usize>,
}

impl Default for SymbolMap {
Expand All @@ -19,7 +19,7 @@ impl SymbolMap {
pub fn new() -> Self {
SymbolMap {
values: Vec::new(),
map: HashMap::new(),
map: FxHashMap::default(),
}
}

Expand Down
29 changes: 12 additions & 17 deletions crates/steel-core/src/compiler/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,7 @@ use once_cell::sync::Lazy;
// use smallvec::SmallVec;
use steel_parser::{ast::PROTO_HASH_GET, expr_list};

use std::{
borrow::Cow,
collections::{HashMap, HashSet},
io::Read,
path::PathBuf,
};
use std::{borrow::Cow, io::Read, path::PathBuf};

use crate::parser::expander::SteelMacro;
use crate::stop;
Expand Down Expand Up @@ -131,7 +126,7 @@ pub(crate) struct ModuleManager {
compiled_modules: FxHashMap<PathBuf, CompiledModule>,
file_metadata: FxHashMap<PathBuf, SystemTime>,
visited: FxHashSet<PathBuf>,
custom_builtins: HashMap<String, String>,
custom_builtins: FxHashMap<String, String>,
}

impl ModuleManager {
Expand All @@ -143,7 +138,7 @@ impl ModuleManager {
compiled_modules,
file_metadata,
visited: FxHashSet::default(),
custom_builtins: HashMap::new(),
custom_builtins: FxHashMap::default(),
}
}

Expand Down Expand Up @@ -211,8 +206,8 @@ impl ModuleManager {
mut exprs: Vec<ExprKind>,
path: Option<PathBuf>,
builtin_modules: ModuleContainer,
lifted_kernel_environments: &mut HashMap<String, KernelDefMacroSpec>,
lifted_macro_environments: &mut HashSet<PathBuf>,
lifted_kernel_environments: &mut FxHashMap<String, KernelDefMacroSpec>,
lifted_macro_environments: &mut FxHashSet<PathBuf>,
search_dirs: &[PathBuf],
) -> Result<Vec<ExprKind>> {
// Wipe the visited set on entry
Expand Down Expand Up @@ -267,7 +262,7 @@ impl ModuleManager {
// })
// .collect::<Vec<_>>();

let mut explicit_requires = HashMap::new();
let mut explicit_requires = FxHashMap::default();

for require_object in &module_builder.require_objects {
let path = require_object.path.get_path();
Expand Down Expand Up @@ -1036,7 +1031,7 @@ impl CompiledModule {
// ;; Refresh the module definition in this namespace
// (define a-module.rkt-b (hash-get 'b b-module.rkt-b))

let mut explicit_requires = HashMap::new();
let mut explicit_requires = FxHashMap::default();

// TODO: This is the same as the top level, they should be merged
for require_object in &self.require_objects {
Expand Down Expand Up @@ -1652,7 +1647,7 @@ struct ModuleBuilder<'a> {
kernel: &'a mut Option<Kernel>,
builtin_modules: ModuleContainer,
global_macro_map: &'a FxHashMap<InternedString, SteelMacro>,
custom_builtins: &'a HashMap<String, String>,
custom_builtins: &'a FxHashMap<String, String>,
search_dirs: &'a [PathBuf],
}

Expand All @@ -1669,7 +1664,7 @@ impl<'a> ModuleBuilder<'a> {
kernel: &'a mut Option<Kernel>,
builtin_modules: ModuleContainer,
global_macro_map: &'a FxHashMap<InternedString, SteelMacro>,
custom_builtins: &'a HashMap<String, String>,
custom_builtins: &'a FxHashMap<String, String>,
search_dirs: &'a [PathBuf],
) -> Result<Self> {
// TODO don't immediately canonicalize the path unless we _know_ its coming from a path
Expand Down Expand Up @@ -2781,7 +2776,7 @@ impl<'a> ModuleBuilder<'a> {
kernel: &'a mut Option<Kernel>,
builtin_modules: ModuleContainer,
global_macro_map: &'a FxHashMap<InternedString, SteelMacro>,
custom_builtins: &'a HashMap<String, String>,
custom_builtins: &'a FxHashMap<String, String>,
) -> Result<Self> {
ModuleBuilder::raw(
name,
Expand All @@ -2807,7 +2802,7 @@ impl<'a> ModuleBuilder<'a> {
kernel: &'a mut Option<Kernel>,
builtin_modules: ModuleContainer,
global_macro_map: &'a FxHashMap<InternedString, SteelMacro>,
custom_builtins: &'a HashMap<String, String>,
custom_builtins: &'a FxHashMap<String, String>,
search_dirs: &'a [PathBuf],
) -> Result<Self> {
ModuleBuilder::raw(
Expand All @@ -2834,7 +2829,7 @@ impl<'a> ModuleBuilder<'a> {
kernel: &'a mut Option<Kernel>,
builtin_modules: ModuleContainer,
global_macro_map: &'a FxHashMap<InternedString, SteelMacro>,
custom_builtins: &'a HashMap<String, String>,
custom_builtins: &'a FxHashMap<String, String>,
search_dirs: &'a [PathBuf],
) -> Self {
ModuleBuilder {
Expand Down
35 changes: 16 additions & 19 deletions crates/steel-core/src/compiler/passes/analysis.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
collections::{hash_map, HashMap, HashSet},
hash::BuildHasherDefault,
};
use std::{collections::hash_map, hash::BuildHasherDefault};

use im_rc::HashMap as ImmutableHashMap;
use quickscope::ScopeMap;
Expand Down Expand Up @@ -301,10 +298,10 @@ pub struct Analysis {
impl Analysis {
pub fn pre_allocated() -> Self {
Analysis {
info: HashMap::with_capacity_and_hasher(3584, FxBuildHasher::default()),
function_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
call_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
let_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
info: FxHashMap::with_capacity_and_hasher(3584, FxBuildHasher::default()),
function_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
call_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
let_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
scope: ScopeMap::default(),
}
}
Expand Down Expand Up @@ -338,10 +335,10 @@ impl Analysis {
// let mut analysis = Analysis::default();

let mut analysis = Analysis {
info: HashMap::with_capacity_and_hasher(3584, FxBuildHasher::default()),
function_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
call_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
let_info: HashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
info: FxHashMap::with_capacity_and_hasher(3584, FxBuildHasher::default()),
function_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
call_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
let_info: FxHashMap::with_capacity_and_hasher(1792, FxBuildHasher::default()),
scope: ScopeMap::default(),
};

Expand Down Expand Up @@ -760,7 +757,7 @@ impl<'a> AnalysisPass<'a> {
fn _get_possible_captures(
&self,
let_level_bindings: &[&InternedString],
) -> HashSet<InternedString> {
) -> FxHashSet<InternedString> {
self.info
.scope
.iter()
Expand Down Expand Up @@ -2594,7 +2591,7 @@ impl<'a> VisitorMutUnitRef<'a> for FreeIdentifierVisitor<'a> {

// TODO: Don't need the analysis at all
struct IdentifierFinder<'a> {
ids: &'a mut HashMap<SyntaxObjectId, Option<InternedString>>,
ids: &'a mut FxHashMap<SyntaxObjectId, Option<InternedString>>,
}

impl<'a> VisitorMutUnitRef<'a> for IdentifierFinder<'a> {
Expand Down Expand Up @@ -3737,7 +3734,7 @@ impl<'a> SemanticAnalysis<'a> {
// Syntax object must be the id associated with a given require define statement
pub fn resolve_required_identifiers(
&self,
identifiers: HashSet<SyntaxObjectId>,
identifiers: FxHashSet<SyntaxObjectId>,
) -> Vec<(SyntaxObjectId, RequiredIdentifierInformation<'_>)> {
let mut results = Vec::new();

Expand Down Expand Up @@ -4546,8 +4543,8 @@ impl<'a> SemanticAnalysis<'a> {
// happen if the analysis gets invalidated by refreshing the vars.
pub fn syntax_object_ids_to_identifiers<'b>(
&self,
ids: &'a mut HashMap<SyntaxObjectId, Option<InternedString>>,
) -> &mut HashMap<SyntaxObjectId, Option<InternedString>> {
ids: &'a mut FxHashMap<SyntaxObjectId, Option<InternedString>>,
) -> &mut FxHashMap<SyntaxObjectId, Option<InternedString>> {
let mut identifier_finder = IdentifierFinder { ids };

for expr in self.exprs.iter() {
Expand Down Expand Up @@ -4591,7 +4588,7 @@ impl<'a> SemanticAnalysis<'a> {

pub fn check_if_values_are_redefined(&mut self) -> Result<&mut Self, SteelErr> {
// TODO: Maybe reuse this memory somehow?
let mut non_builtin_definitions = HashSet::new();
let mut non_builtin_definitions = FxHashSet::default();

for expr in self.exprs.iter() {
match expr {
Expand Down Expand Up @@ -5792,7 +5789,7 @@ mod analysis_pass_tests {
{
let mut analysis = SemanticAnalysis::new(&mut exprs);

let mut constants = im_rc::HashMap::default();
let mut constants = ImmutableHashMap::<_, _, FxBuildHasher>::default();
constants.insert("+".into(), SteelVal::Void);
constants.insert("<=".into(), SteelVal::Void);
constants.insert("-".into(), SteelVal::Void);
Expand Down
Loading