Use HashMaps instead of BTreeMaps in isle where possible (#4147)

The HashMap implementation is significantly simpler than the BTreeMap
implementation. Because of this switching reduces compilation time of
cranelift-isle by about 10%.

# Before
$ hyperfine --prepare "cargo clean" "cargo build"
Benchmark 1: cargo build
  Time (mean ± σ):      5.221 s ±  0.094 s    [User: 10.659 s, System: 0.734 s]
  Range (min … max):    5.151 s …  5.420 s    10 runs

# After
$ hyperfine --prepare "cargo clean" "cargo build"
Benchmark 1: cargo build
  Time (mean ± σ):      4.746 s ±  0.150 s    [User: 9.109 s, System: 0.721 s]
  Range (min … max):    4.630 s …  5.144 s    10 runs

cranelift/isle/isle/src/codegen.rs

@@ -5,7 +5,8 @@ use crate::log;
 use crate::sema::ExternalSig;
 use crate::sema::{TermEnv, TermId, Type, TypeEnv, TypeId, Variant};
 use crate::trie::{TrieEdge, TrieNode, TrieSymbol};
-use std::collections::{BTreeMap, BTreeSet};
+use crate::{StableMap, StableSet};
+use std::collections::BTreeMap;
 use std::fmt::Write;
 
 /// Options for code generation.
@@ -36,7 +37,7 @@ struct Codegen<'a> {
 #[derive(Clone, Debug, Default)]
 struct BodyContext {
     /// For each value: (is_ref, ty).
-    values: BTreeMap<Value, (bool, TypeId)>,
+    values: StableMap<Value, (bool, TypeId)>,
 }
 
 impl<'a> Codegen<'a> {
@@ -720,7 +721,7 @@ impl<'a> Codegen<'a> {
                     // Gather adjacent match variants so that we can turn these
                     // into a `match` rather than a sequence of `if let`s.
                     let mut last = i;
-                    let mut adjacent_variants = BTreeSet::new();
+                    let mut adjacent_variants = StableSet::new();
                     let mut adjacent_variant_input = None;
                     log!(
                         "edge: prio = {:?}, symbol = {:?}",

cranelift/isle/isle/src/ir.rs

@@ -3,7 +3,7 @@
 use crate::lexer::Pos;
 use crate::log;
 use crate::sema::*;
-use std::collections::BTreeMap;
+use crate::StableMap;
 
 declare_id!(
     /// The id of an instruction in a `PatternSequence`.
@@ -341,7 +341,7 @@ impl PatternSequence {
         typeenv: &TypeEnv,
         termenv: &TermEnv,
         pat: &Pattern,
-        vars: &mut BTreeMap<VarId, Value>,
+        vars: &mut StableMap<VarId, Value>,
     ) {
         match pat {
             &Pattern::BindPattern(_ty, var, ref subpat) => {
@@ -548,7 +548,7 @@ impl ExprSequence {
         typeenv: &TypeEnv,
         termenv: &TermEnv,
         expr: &Expr,
-        vars: &BTreeMap<VarId, Value>,
+        vars: &StableMap<VarId, Value>,
     ) -> Value {
         log!("gen_expr: expr {:?}", expr);
         match expr {
@@ -622,7 +622,7 @@ pub fn lower_rule(
     expr_seq.pos = termenv.rules[rule.index()].pos;
 
     let ruledata = &termenv.rules[rule.index()];
-    let mut vars = BTreeMap::new();
+    let mut vars = StableMap::new();
     let root_term = ruledata
         .lhs
         .root_term()

cranelift/isle/isle/src/lib.rs

@@ -1,6 +1,11 @@
 #![doc = include_str!("../README.md")]
 #![deny(missing_docs)]
 
+use std::collections::hash_map::Entry;
+use std::collections::{HashMap, HashSet};
+use std::hash::Hash;
+use std::ops::Index;
+
 macro_rules! declare_id {
     (
         $(#[$attr:meta])*
@@ -18,6 +23,74 @@ macro_rules! declare_id {
     };
 }
 
+/// A wrapper around a [HashSet] which prevents accidentally observing the non-deterministic
+/// iteration order.
+#[derive(Clone, Debug)]
+struct StableSet<T>(HashSet<T>);
+
+impl<T> StableSet<T> {
+    fn new() -> Self {
+        StableSet(HashSet::new())
+    }
+}
+
+impl<T: Hash + Eq> StableSet<T> {
+    fn insert(&mut self, val: T) -> bool {
+        self.0.insert(val)
+    }
+
+    fn contains(&self, val: &T) -> bool {
+        self.0.contains(val)
+    }
+}
+
+/// A wrapper around a [HashMap] which prevents accidentally observing the non-deterministic
+/// iteration order.
+#[derive(Clone, Debug)]
+pub struct StableMap<K, V>(HashMap<K, V>);
+
+impl<K, V> StableMap<K, V> {
+    fn new() -> Self {
+        StableMap(HashMap::new())
+    }
+
+    fn len(&self) -> usize {
+        self.0.len()
+    }
+}
+
+impl<K, V> Default for StableMap<K, V> {
+    fn default() -> Self {
+        StableMap(HashMap::new())
+    }
+}
+
+impl<K: Hash + Eq, V> StableMap<K, V> {
+    fn insert(&mut self, k: K, v: V) -> Option<V> {
+        self.0.insert(k, v)
+    }
+
+    fn contains_key(&self, k: &K) -> bool {
+        self.0.contains_key(k)
+    }
+
+    fn get(&self, k: &K) -> Option<&V> {
+        self.0.get(k)
+    }
+
+    fn entry(&mut self, k: K) -> Entry<K, V> {
+        self.0.entry(k)
+    }
+}
+
+impl<K: Hash + Eq, V> Index<&K> for StableMap<K, V> {
+    type Output = V;
+
+    fn index(&self, index: &K) -> &Self::Output {
+        self.0.index(index)
+    }
+}
+
 pub mod ast;
 pub mod codegen;
 pub mod compile;

cranelift/isle/isle/src/sema.rs

@@ -18,7 +18,8 @@ use crate::ast::Ident;
 use crate::error::*;
 use crate::lexer::Pos;
 use crate::log;
-use std::collections::btree_map::Entry;
+use crate::{StableMap, StableSet};
+use std::collections::hash_map::Entry;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::sync::Arc;
@@ -73,7 +74,7 @@ pub struct TypeEnv {
     pub syms: Vec<String>,
 
     /// Map of already-interned symbol names to their `Sym` ids.
-    pub sym_map: BTreeMap<String, Sym>,
+    pub sym_map: StableMap<String, Sym>,
 
     /// Arena of type definitions.
     ///
@@ -81,10 +82,10 @@ pub struct TypeEnv {
     pub types: Vec<Type>,
 
     /// A map from a type name symbol to its `TypeId`.
-    pub type_map: BTreeMap<Sym, TypeId>,
+    pub type_map: StableMap<Sym, TypeId>,
 
     /// The types of constant symbols.
-    pub const_types: BTreeMap<Sym, TypeId>,
+    pub const_types: StableMap<Sym, TypeId>,
 
     /// Type errors that we've found so far during type checking.
     pub errors: Vec<Error>,
@@ -188,7 +189,7 @@ pub struct TermEnv {
     pub terms: Vec<Term>,
 
     /// A map from am interned `Term`'s name to its `TermId`.
-    pub term_map: BTreeMap<Sym, TermId>,
+    pub term_map: StableMap<Sym, TermId>,
 
     /// Arena of interned rules defined in this ISLE program.
     ///
@@ -198,7 +199,7 @@ pub struct TermEnv {
     /// Map from (inner_ty, outer_ty) pairs to term IDs, giving the
     /// defined implicit type-converter terms we can try to use to fit
     /// types together.
-    pub converters: BTreeMap<(TypeId, TypeId), TermId>,
+    pub converters: StableMap<(TypeId, TypeId), TermId>,
 }
 
 /// A term.
@@ -548,10 +549,10 @@ impl TypeEnv {
             filenames: defs.filenames.clone(),
             file_texts: defs.file_texts.clone(),
             syms: vec![],
-            sym_map: BTreeMap::new(),
+            sym_map: StableMap::new(),
             types: vec![],
-            type_map: BTreeMap::new(),
-            const_types: BTreeMap::new(),
+            type_map: StableMap::new(),
+            const_types: StableMap::new(),
             errors: vec![],
         };
 
@@ -771,9 +772,9 @@ impl TermEnv {
     pub fn from_ast(tyenv: &mut TypeEnv, defs: &ast::Defs) -> Result<TermEnv> {
         let mut env = TermEnv {
             terms: vec![],
-            term_map: BTreeMap::new(),
+            term_map: StableMap::new(),
             rules: vec![],
-            converters: BTreeMap::new(),
+            converters: StableMap::new(),
         };
 
         env.collect_term_sigs(tyenv, defs);
@@ -1041,7 +1042,7 @@ impl TermEnv {
         let mut stack = vec![];
         'outer: for root in extractor_call_graph.keys().copied() {
             stack.clear();
-            stack.push((root, vec![root], BTreeSet::new()));
+            stack.push((root, vec![root], StableSet::new()));
 
             while let Some((caller, path, mut seen)) = stack.pop() {
                 let is_new = seen.insert(caller);