Merge branch 'main' into newhint13-quad_bit

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 #### Upcoming Changes
 
+* BREAKING CHANGE: refactor `Program` to optimize `Program::clone` [#999](https://github.com/lambdaclass/cairo-rs/pull/999)
+    * Breaking change: many fields that were (unnecessarily) public become hidden by the refactor.
+
 * BREAKING CHANGE: Add _builtin suffix to builtin names e.g.: output -> output_builtin [#1005](https://github.com/lambdaclass/cairo-rs/pull/1005)
 
 * Implement hint on uint384_extension lib [#983](https://github.com/lambdaclass/cairo-rs/pull/983)

src/serde/deserialize_program.rs

@@ -1,9 +1,11 @@
-use crate::stdlib::{collections::HashMap, fmt, prelude::*};
+use crate::stdlib::{collections::HashMap, fmt, prelude::*, sync::Arc};
 
 use crate::{
     serde::deserialize_utils,
     types::{
-        errors::program_errors::ProgramError, instruction::Register, program::Program,
+        errors::program_errors::ProgramError,
+        instruction::Register,
+        program::{Program, SharedProgramData},
         relocatable::MaybeRelocatable,
     },
     vm::runners::builtin_runner::{
@@ -373,10 +375,24 @@ pub fn parse_program_json(
         None => None,
     };
 
-    Ok(Program {
+    let shared_program_data = SharedProgramData {
         builtins: program_json.builtins,
-        prime: PRIME_STR.to_string(),
         data: program_json.data,
+        hints: program_json.hints,
+        main: entrypoint_pc,
+        start,
+        end,
+        error_message_attributes: program_json
+            .attributes
+            .into_iter()
+            .filter(|attr| attr.name == "error_message")
+            .collect(),
+        instruction_locations: program_json
+            .debug_info
+            .map(|debug_info| debug_info.instruction_locations),
+    };
+    Ok(Program {
+        shared_program_data: Arc::new(shared_program_data),
         constants: {
             let mut constants = HashMap::new();
             for (key, value) in program_json.identifiers.iter() {
@@ -391,20 +407,8 @@ pub fn parse_program_json(
 
             constants
         },
-        main: entrypoint_pc,
-        start,
-        end,
-        hints: program_json.hints,
         reference_manager: program_json.reference_manager,
         identifiers: program_json.identifiers,
-        error_message_attributes: program_json
-            .attributes
-            .into_iter()
-            .filter(|attr| attr.name == "error_message")
-            .collect(),
-        instruction_locations: program_json
-            .debug_info
-            .map(|debug_info| debug_info.instruction_locations),
     })
 }
 
@@ -805,14 +809,10 @@ mod tests {
             }],
         );
 
-        assert_eq!(
-            program.prime,
-            "0x800000000000011000000000000000000000000000000000000000000000001".to_string()
-        );
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, Some(0));
-        assert_eq!(program.hints, hints);
+        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.shared_program_data.hints, hints);
     }
 
     /// Deserialize a program without an entrypoint.
@@ -867,14 +867,10 @@ mod tests {
             }],
         );
 
-        assert_eq!(
-            program.prime,
-            "0x800000000000011000000000000000000000000000000000000000000000001".to_string()
-        );
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, None);
-        assert_eq!(program.hints, hints);
+        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.shared_program_data.hints, hints);
     }
 
     #[test]

src/types/program.rs

@@ -1,4 +1,4 @@
-use crate::stdlib::{collections::HashMap, prelude::*};
+use crate::stdlib::{collections::HashMap, prelude::*, sync::Arc};
 
 use crate::{
     serde::deserialize_program::{
@@ -7,34 +7,57 @@ use crate::{
     },
     types::{errors::program_errors::ProgramError, relocatable::MaybeRelocatable},
 };
-use felt::{Felt252, PRIME_STR};
-use serde::{Deserialize, Serialize};
+use felt::Felt252;
 
 #[cfg(feature = "std")]
 use std::path::Path;
 
-#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
-pub struct Program {
+// NOTE: `Program` has been split in two containing some data that will be deep-copied
+// and some that will be allocated on the heap inside an `Arc<_>`.
+// This is because it has been reported that cloning the whole structure when creating
+// a `CairoRunner` becomes a bottleneck, but the following solutions were tried and
+// discarded:
+// - Store only a reference in `CairoRunner` rather than cloning; this doesn't work
+//   because then we need to introduce explicit lifetimes, which broke `cairo-rs-py`
+//   since PyO3 doesn't support Python objects containing structures with lifetimes.
+// - Directly pass an `Arc<Program>` to `CairoRunner::new()` and simply copy that:
+//   there was a prohibitive performance hit of 10-15% when doing so, most likely
+//   either because of branch mispredictions or the extra level of indirection going
+//   through a random location on the heap rather than the likely-to-be-cached spot
+//   on the stack.
+//
+// So, the compromise was to identify which data was less used and avoid copying that,
+// using `Arc<_>`, while the most accessed fields remain on the stack for the main
+// loop to access. The fields in `SharedProgramData` are either preprocessed and
+// copied explicitly (_in addition_ to the clone of `Program`) or are used only in
+// exceptional circumstances, such as when reconstructing a backtrace on execution
+// failures.
+// 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 prime: String,
     pub data: Vec<MaybeRelocatable>,
-    pub constants: HashMap<String, Felt252>,
+    pub hints: HashMap<usize, Vec<HintParams>>,
     pub main: Option<usize>,
     //start and end labels will only be used in proof-mode
     pub start: Option<usize>,
     pub end: Option<usize>,
-    pub hints: HashMap<usize, Vec<HintParams>>,
-    pub reference_manager: ReferenceManager,
-    pub identifiers: HashMap<String, Identifier>,
     pub error_message_attributes: Vec<Attribute>,
     pub instruction_locations: Option<HashMap<usize, InstructionLocation>>,
 }
 
+#[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>,
+}
+
 impl Program {
     #[allow(clippy::too_many_arguments)]
     pub fn new(
         builtins: Vec<BuiltinName>,
-        prime: String,
         data: Vec<MaybeRelocatable>,
         main: Option<usize>,
         hints: HashMap<usize, Vec<HintParams>>,
@@ -43,10 +66,18 @@ impl Program {
         error_message_attributes: Vec<Attribute>,
         instruction_locations: Option<HashMap<usize, InstructionLocation>>,
     ) -> Result<Program, ProgramError> {
-        Ok(Self {
+        let shared_program_data = SharedProgramData {
             builtins,
-            prime,
             data,
+            hints,
+            main,
+            start: None,
+            end: None,
+            error_message_attributes,
+            instruction_locations,
+        };
+        Ok(Self {
+            shared_program_data: Arc::new(shared_program_data),
             constants: {
                 let mut constants = HashMap::new();
                 for (key, value) in identifiers.iter() {
@@ -61,14 +92,8 @@ impl Program {
 
                 constants
             },
-            main,
-            start: None,
-            end: None,
-            hints,
             reference_manager,
             identifiers,
-            error_message_attributes,
-            instruction_locations,
         })
     }
 
@@ -85,21 +110,13 @@ impl Program {
 
 impl Default for Program {
     fn default() -> Self {
-        Program {
-            builtins: Vec::new(),
-            prime: PRIME_STR.to_string(),
-            data: Vec::new(),
+        Self {
+            shared_program_data: Arc::new(SharedProgramData::default()),
             constants: HashMap::new(),
-            main: None,
-            start: None,
-            end: None,
-            hints: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
             identifiers: HashMap::new(),
-            error_message_attributes: Vec::new(),
-            instruction_locations: None,
         }
     }
 }
@@ -133,7 +150,6 @@ mod tests {
 
         let program = Program::new(
             builtins.clone(),
-            felt::PRIME_STR.to_string(),
             data.clone(),
             None,
             HashMap::new(),
@@ -144,9 +160,9 @@ mod tests {
         )
         .unwrap();
 
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, None);
+        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());
     }
 
@@ -196,7 +212,6 @@ mod tests {
 
         let program = Program::new(
             builtins.clone(),
-            felt::PRIME_STR.to_string(),
             data.clone(),
             None,
             HashMap::new(),
@@ -207,9 +222,9 @@ mod tests {
         )
         .unwrap();
 
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, None);
+        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.constants,
@@ -266,7 +281,6 @@ mod tests {
 
         let program = Program::new(
             builtins,
-            felt::PRIME_STR.to_string(),
             data,
             None,
             HashMap::new(),
@@ -356,10 +370,9 @@ mod tests {
             },
         );
 
-        assert_eq!(program.prime, PRIME_STR.to_string());
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, Some(0));
+        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);
     }
 
@@ -456,12 +469,14 @@ mod tests {
             },
         );
 
-        assert_eq!(program.prime, PRIME_STR.to_string());
-        assert_eq!(program.builtins, builtins);
-        assert_eq!(program.data, data);
-        assert_eq!(program.main, None);
+        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.error_message_attributes, error_message_attributes)
+        assert_eq!(
+            program.shared_program_data.error_message_attributes,
+            error_message_attributes
+        )
     }
 
     #[test]
@@ -506,23 +521,25 @@ mod tests {
     #[test]
     #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
     fn default_program() {
-        let program = Program {
+        let shared_program_data = SharedProgramData {
             builtins: Vec::new(),
-            prime: PRIME_STR.to_string(),
             data: Vec::new(),
-            constants: HashMap::new(),
+            hints: HashMap::new(),
             main: None,
             start: None,
             end: None,
-            hints: HashMap::new(),
+            error_message_attributes: Vec::new(),
+            instruction_locations: None,
+        };
+        let program = Program {
+            shared_program_data: Arc::new(shared_program_data),
+            constants: HashMap::new(),
             reference_manager: ReferenceManager {
                 references: Vec::new(),
             },
             identifiers: HashMap::new(),
-            error_message_attributes: Vec::new(),
-            instruction_locations: None,
         };
 
-        assert_eq!(program, Program::default())
+        assert_eq!(program, Program::default());
     }
 }