Merge pull request #4614 from stacks-network/feat/tuple-perf-improvements

2.5: Tuple performance improvements

Author: kantai

clarity/src/vm/analysis/contract_interface_builder/mod.rs

@@ -171,14 +171,16 @@ impl ContractInterfaceAtomType {
     pub fn vec_from_tuple_type(
         tuple_type: &TupleTypeSignature,
     ) -> Vec<ContractInterfaceTupleEntryType> {
-        tuple_type
+        let mut out: Vec<_> = tuple_type
             .get_type_map()
             .iter()
             .map(|(name, sig)| ContractInterfaceTupleEntryType {
                 name: name.to_string(),
                 type_f: Self::from_type_signature(sig),
             })
-            .collect()
+            .collect();
+        out.sort_unstable_by(|ty1, ty2| ty1.name.cmp(&ty2.name));
+        out
     }
 
     pub fn from_type_signature(sig: &TypeSignature) -> ContractInterfaceAtomType {

clarity/src/vm/analysis/mod.rs

@@ -71,14 +71,15 @@ pub fn mem_type_check(
         cost_tracker,
         epoch,
         version,
+        true,
     ) {
         Ok(x) => {
             // return the first type result of the type checker
             let first_type = x
                 .type_map
                 .as_ref()
                 .ok_or_else(|| CheckErrors::Expects("Should be non-empty".into()))?
-                .get_type(
+                .get_type_expected(
                     x.expressions
                         .last()
                         .ok_or_else(|| CheckErrors::Expects("Should be non-empty".into()))?,
@@ -111,6 +112,7 @@ pub fn type_check(
         LimitedCostTracker::new_free(),
         *epoch,
         *version,
+        true,
     )
     .map_err(|(e, _cost_tracker)| e)
 }
@@ -123,6 +125,7 @@ pub fn run_analysis(
     cost_tracker: LimitedCostTracker,
     epoch: StacksEpochId,
     version: ClarityVersion,
+    build_type_map: bool,
 ) -> Result<ContractAnalysis, (CheckError, LimitedCostTracker)> {
     let mut contract_analysis = ContractAnalysis::new(
         contract_identifier.clone(),
@@ -135,15 +138,15 @@ pub fn run_analysis(
         ReadOnlyChecker::run_pass(&epoch, &mut contract_analysis, db)?;
         match epoch {
             StacksEpochId::Epoch20 | StacksEpochId::Epoch2_05 => {
-                TypeChecker2_05::run_pass(&epoch, &mut contract_analysis, db)
+                TypeChecker2_05::run_pass(&epoch, &mut contract_analysis, db, build_type_map)
             }
             StacksEpochId::Epoch21
             | StacksEpochId::Epoch22
             | StacksEpochId::Epoch23
             | StacksEpochId::Epoch24
             | StacksEpochId::Epoch25
             | StacksEpochId::Epoch30 => {
-                TypeChecker2_1::run_pass(&epoch, &mut contract_analysis, db)
+                TypeChecker2_1::run_pass(&epoch, &mut contract_analysis, db, build_type_map)
             }
             StacksEpochId::Epoch10 => {
                 return Err(CheckErrors::Expects(

clarity/src/vm/analysis/type_checker/contexts.rs

@@ -14,6 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use std::collections::HashSet;
+
 use hashbrown::HashMap;
 use stacks_common::types::StacksEpochId;
 
@@ -22,9 +24,22 @@ use crate::vm::types::signatures::CallableSubtype;
 use crate::vm::types::{TraitIdentifier, TypeSignature};
 use crate::vm::{ClarityName, ClarityVersion, SymbolicExpression, MAX_CONTEXT_DEPTH};
 
-#[derive(Debug, PartialEq, Eq, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct TypeMap {
-    map: HashMap<u64, TypeSignature>,
+    map: TypeMapDataType,
+}
+
+#[derive(Debug, Clone, PartialEq)]
+/// This enum allows the type checker to operate
+/// with two different kinds of type maps. The Set
+/// version is more efficient, and only triggers an error
+/// if an AST node is visited more than once. The Map
+/// version is used when the actual type of each AST node
+/// is needed by a subsequent reader. This is only used by
+/// tests and docs generation.
+enum TypeMapDataType {
+    Map(HashMap<u64, TypeSignature>),
+    Set(HashSet<u64>),
 }
 
 pub struct TypingContext<'a> {
@@ -36,33 +51,44 @@ pub struct TypingContext<'a> {
     pub depth: u16,
 }
 
-impl Default for TypeMap {
-    fn default() -> Self {
-        Self::new()
-    }
-}
-
 impl TypeMap {
-    pub fn new() -> TypeMap {
-        TypeMap {
-            map: HashMap::new(),
-        }
+    pub fn new(build_map: bool) -> TypeMap {
+        let map = if build_map {
+            TypeMapDataType::Map(HashMap::new())
+        } else {
+            TypeMapDataType::Set(HashSet::new())
+        };
+        TypeMap { map }
     }
 
     pub fn set_type(
         &mut self,
         expr: &SymbolicExpression,
         type_sig: TypeSignature,
     ) -> CheckResult<()> {
-        if self.map.insert(expr.id, type_sig).is_some() {
-            Err(CheckError::new(CheckErrors::TypeAlreadyAnnotatedFailure))
-        } else {
-            Ok(())
+        match self.map {
+            TypeMapDataType::Map(ref mut map) => {
+                if map.insert(expr.id, type_sig).is_some() {
+                    Err(CheckError::new(CheckErrors::TypeAlreadyAnnotatedFailure))
+                } else {
+                    Ok(())
+                }
+            }
+            TypeMapDataType::Set(ref mut map) => {
+                if !map.insert(expr.id) {
+                    Err(CheckError::new(CheckErrors::TypeAlreadyAnnotatedFailure))
+                } else {
+                    Ok(())
+                }
+            }
         }
     }
 
-    pub fn get_type(&self, expr: &SymbolicExpression) -> Option<&TypeSignature> {
-        self.map.get(&expr.id)
+    pub fn get_type_expected(&self, expr: &SymbolicExpression) -> Option<&TypeSignature> {
+        match self.map {
+            TypeMapDataType::Map(ref map) => map.get(&expr.id),
+            TypeMapDataType::Set(_) => None,
+        }
     }
 }
 

clarity/src/vm/analysis/type_checker/v2_05/mod.rs

@@ -111,14 +111,15 @@ impl CostTracker for TypeChecker<'_, '_> {
     }
 }
 
-impl AnalysisPass for TypeChecker<'_, '_> {
-    fn run_pass(
+impl TypeChecker<'_, '_> {
+    pub fn run_pass(
         _epoch: &StacksEpochId,
         contract_analysis: &mut ContractAnalysis,
         analysis_db: &mut AnalysisDatabase,
+        build_type_map: bool,
     ) -> CheckResult<()> {
         let cost_track = contract_analysis.take_contract_cost_tracker();
-        let mut command = TypeChecker::new(analysis_db, cost_track);
+        let mut command = TypeChecker::new(analysis_db, cost_track, build_type_map);
         // run the analysis, and replace the cost tracker whether or not the
         //   analysis succeeded.
         match command.run(contract_analysis) {
@@ -343,13 +344,14 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
     fn new(
         db: &'a mut AnalysisDatabase<'b>,
         cost_track: LimitedCostTracker,
+        build_type_map: bool,
     ) -> TypeChecker<'a, 'b> {
         Self {
             db,
             cost_track,
             contract_context: ContractContext::new(),
             function_return_tracker: None,
-            type_map: TypeMap::new(),
+            type_map: TypeMap::new(build_type_map),
         }
     }