perf!: move identifiers to SharedProgramData

Part of the same effort to reduce `CairoRunner` creation time. Breaks due to that field being public. To reduce future breakage, we make all remaining fields `pub(crate)`.
lambdaclass · Apr 20, 2023 · 0633b8f · 0633b8f
1 parent f470b0e
commit 0633b8f
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 120 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 #### Upcoming Changes
 
+* BREAKING CHANGE: move `Program::identifiers` to `SharedProgramData::identifiers` [#1023](https://github.com/lambdaclass/cairo-rs/pull/1023)
+    * Optimizes `CairoRunner::new`, needed for sequencers and other workflows reusing the same `Program` instance across `CairoRunner`s
+    * Breaking change: make all fields in `Program` and `SharedProgramData` `pub(crate)`, since we break by moving the field let's make it the last break for this struct
+
 * Add missing hint on uint256_improvements lib [#1016](https://github.com/lambdaclass/cairo-rs/pull/1016):
 
     `BuiltinHintProcessor` now supports the following hint:

diff --git a/src/serde/deserialize_program.rs b/src/serde/deserialize_program.rs
@@ -375,6 +375,17 @@ pub fn parse_program_json(
         None => None,
     };
 
+    let mut constants = HashMap::new();
+    for (key, value) in program_json.identifiers.iter() {
+        if value.type_.as_deref() == Some("const") {
+            let value = value
+                .value
+                .clone()
+                .ok_or_else(|| ProgramError::ConstWithoutValue(key.clone()))?;
+            constants.insert(key.clone(), value);
+        }
+    }
+
     let shared_program_data = SharedProgramData {
         builtins: program_json.builtins,
         data: program_json.data,
@@ -390,25 +401,12 @@ pub fn parse_program_json(
         instruction_locations: program_json
             .debug_info
             .map(|debug_info| debug_info.instruction_locations),
+        identifiers: program_json.identifiers,
     };
     Ok(Program {
         shared_program_data: Arc::new(shared_program_data),
-        constants: {
-            let mut constants = HashMap::new();
-            for (key, value) in program_json.identifiers.iter() {
-                if value.type_.as_deref() == Some("const") {
-                    let value = value
-                        .value
-                        .clone()
-                        .ok_or_else(|| ProgramError::ConstWithoutValue(key.clone()))?;
-                    constants.insert(key.clone(), value);
-                }
-            }
-
-            constants
-        },
+        constants,
         reference_manager: program_json.reference_manager,
-        identifiers: program_json.identifiers,
     })
 }
 

diff --git a/src/types/program.rs b/src/types/program.rs
@@ -35,23 +35,23 @@ use std::path::Path;
 // Fields in `Program` (other than `SharedProgramData` itself) are used by the main logic.
 #[derive(Clone, Default, Debug, PartialEq, Eq)]
 pub(crate) struct SharedProgramData {
-    pub builtins: Vec<BuiltinName>,
-    pub data: Vec<MaybeRelocatable>,
-    pub hints: HashMap<usize, Vec<HintParams>>,
-    pub main: Option<usize>,
+    pub(crate) builtins: Vec<BuiltinName>,
+    pub(crate) data: Vec<MaybeRelocatable>,
+    pub(crate) hints: HashMap<usize, Vec<HintParams>>,
+    pub(crate) main: Option<usize>,
     //start and end labels will only be used in proof-mode
-    pub start: Option<usize>,
-    pub end: Option<usize>,
-    pub error_message_attributes: Vec<Attribute>,
-    pub instruction_locations: Option<HashMap<usize, InstructionLocation>>,
+    pub(crate) start: Option<usize>,
+    pub(crate) end: Option<usize>,
+    pub(crate) error_message_attributes: Vec<Attribute>,
+    pub(crate) instruction_locations: Option<HashMap<usize, InstructionLocation>>,
+    pub(crate) identifiers: HashMap<String, Identifier>,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Program {
     pub(crate) shared_program_data: Arc<SharedProgramData>,
-    pub constants: HashMap<String, Felt252>,
-    pub reference_manager: ReferenceManager,
-    pub identifiers: HashMap<String, Identifier>,
+    pub(crate) constants: HashMap<String, Felt252>,
+    pub(crate) reference_manager: ReferenceManager,
 }
 
 impl Program {
@@ -66,6 +66,16 @@ impl Program {
         error_message_attributes: Vec<Attribute>,
         instruction_locations: Option<HashMap<usize, InstructionLocation>>,
     ) -> Result<Program, ProgramError> {
+        let mut constants = HashMap::new();
+        for (key, value) in identifiers.iter() {
+            if value.type_.as_deref() == Some("const") {
+                let value = value
+                    .value
+                    .clone()
+                    .ok_or_else(|| ProgramError::ConstWithoutValue(key.clone()))?;
+                constants.insert(key.clone(), value);
+            }
+        }
         let shared_program_data = SharedProgramData {
             builtins,
             data,
@@ -75,25 +85,12 @@ impl Program {
             end: None,
             error_message_attributes,
             instruction_locations,
+            identifiers,
         };
         Ok(Self {
             shared_program_data: Arc::new(shared_program_data),
-            constants: {
-                let mut constants = HashMap::new();
-                for (key, value) in identifiers.iter() {
-                    if value.type_.as_deref() == Some("const") {
-                        let value = value
-                            .value
-                            .clone()
-                            .ok_or_else(|| ProgramError::ConstWithoutValue(key.clone()))?;
-                        constants.insert(key.clone(), value);
-                    }
-                }
-
-                constants
-            },
+            constants,
             reference_manager,
-            identifiers,
         })
     }
 
@@ -133,7 +130,6 @@ impl Default for Program {
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
-            identifiers: HashMap::new(),
         }
     }
 }
@@ -181,7 +177,7 @@ mod tests {
         assert_eq!(program.shared_program_data.builtins, builtins);
         assert_eq!(program.shared_program_data.data, data);
         assert_eq!(program.shared_program_data.main, None);
-        assert_eq!(program.identifiers, HashMap::new());
+        assert_eq!(program.shared_program_data.identifiers, HashMap::new());
     }
 
     #[test]
@@ -243,7 +239,7 @@ mod tests {
         assert_eq!(program.shared_program_data.builtins, builtins);
         assert_eq!(program.shared_program_data.data, data);
         assert_eq!(program.shared_program_data.main, None);
-        assert_eq!(program.identifiers, identifiers);
+        assert_eq!(program.shared_program_data.identifiers, identifiers);
         assert_eq!(
             program.constants,
             [("__main__.main.SIZEOF_LOCALS", Felt252::zero())]
@@ -497,7 +493,7 @@ mod tests {
         assert_eq!(program.shared_program_data.builtins, builtins);
         assert_eq!(program.shared_program_data.data, data);
         assert_eq!(program.shared_program_data.main, Some(0));
-        assert_eq!(program.identifiers, identifiers);
+        assert_eq!(program.shared_program_data.identifiers, identifiers);
     }
 
     /// Deserialize a program without an entrypoint.
@@ -596,7 +592,7 @@ mod tests {
         assert_eq!(program.shared_program_data.builtins, builtins);
         assert_eq!(program.shared_program_data.data, data);
         assert_eq!(program.shared_program_data.main, None);
-        assert_eq!(program.identifiers, identifiers);
+        assert_eq!(program.shared_program_data.identifiers, identifiers);
         assert_eq!(
             program.shared_program_data.error_message_attributes,
             error_message_attributes
@@ -654,14 +650,14 @@ mod tests {
             end: None,
             error_message_attributes: Vec::new(),
             instruction_locations: None,
+            identifiers: HashMap::new(),
         };
         let program = Program {
             shared_program_data: Arc::new(shared_program_data),
             constants: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
-            identifiers: HashMap::new(),
         };
 
         assert_eq!(program, Program::default());

diff --git a/src/utils.rs b/src/utils.rs
@@ -255,14 +255,14 @@ pub mod test_utils {
                 end: None,
                 error_message_attributes: crate::stdlib::vec::Vec::new(),
                 instruction_locations: None,
+                identifiers: crate::stdlib::collections::HashMap::new(),
             };
             Program {
                 shared_program_data: Arc::new(shared_program_data),
                 constants: crate::stdlib::collections::HashMap::new(),
                 reference_manager: ReferenceManager {
                     references: crate::stdlib::vec::Vec::new(),
                 },
-                identifiers: crate::stdlib::collections::HashMap::new(),
             }
         }};
         // Custom program definition
@@ -282,14 +282,6 @@ pub mod test_utils {
                 ..Default::default()
             }
         };
-        ($(identifiers = $value:expr),* $(,)?) => {
-            Program {
-                $(
-                    identifiers: $value,
-                )*
-                ..Default::default()
-            }
-        };
         ($($field:ident = $value:expr),* $(,)?) => {{
             let shared_data = SharedProgramData {
                 $(
@@ -883,14 +875,14 @@ mod test {
             end: None,
             error_message_attributes: Vec::new(),
             instruction_locations: None,
+            identifiers: HashMap::new(),
         };
         let program = Program {
             shared_program_data: Arc::new(shared_data),
             constants: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
-            identifiers: HashMap::new(),
         };
         assert_eq!(program, program!())
     }
@@ -907,14 +899,14 @@ mod test {
             end: None,
             error_message_attributes: Vec::new(),
             instruction_locations: None,
+            identifiers: HashMap::new(),
         };
         let program = Program {
             shared_program_data: Arc::new(shared_data),
             constants: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
-            identifiers: HashMap::new(),
         };
 
         assert_eq!(program, program![BuiltinName::range_check])
@@ -932,14 +924,14 @@ mod test {
             end: None,
             error_message_attributes: Vec::new(),
             instruction_locations: None,
+            identifiers: HashMap::new(),
         };
         let program = Program {
             shared_program_data: Arc::new(shared_data),
             constants: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
-            identifiers: HashMap::new(),
         };
 
         assert_eq!(