perf!: refactor Program to reduce clone time

Extract most fields (see the code for the details) in `Program` into a
new `SharedProgramData` structure, then add a field
`shared_program_data: Arc<SharedProgramData>` to `Program`, so cloning
doesn't deep copy them. These were selected based on how often the
runner needed to access them directly, as the indirection and heap
access proved to come with a runtime cost. Frequently accessed fields
are still copied because of that.

The break comes from hiding some symbols (as they were moved to the new
structure), but those shouldn't have been exposed in the first place, so
I expect no breakage for real-world programs (cue Hyrum's law).

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 #### Upcoming Changes
 
+* 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.
+
 * Add missing `\n` character in traceback string [#997](https://github.com/lambdaclass/cairo-rs/pull/997)
     * BugFix: Add missing `\n` character after traceback lines when the filename is missing ("Unknown Location")
 

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::{
@@ -8,33 +8,56 @@ use crate::{
     types::{errors::program_errors::ProgramError, relocatable::MaybeRelocatable},
 };
 use felt::{Felt252, PRIME_STR};
-use serde::{Deserialize, Serialize};
 
 #[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,
         })
     }
 
@@ -81,25 +106,21 @@ impl Program {
     pub fn from_bytes(bytes: &[u8], entrypoint: Option<&str>) -> Result<Program, ProgramError> {
         deserialize_and_parse_program(bytes, entrypoint)
     }
+
+    pub fn primer() -> &'static str {
+        PRIME_STR
+    }
 }
 
 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 +154,6 @@ mod tests {
 
         let program = Program::new(
             builtins.clone(),
-            felt::PRIME_STR.to_string(),
             data.clone(),
             None,
             HashMap::new(),
@@ -144,9 +164,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 +216,6 @@ mod tests {
 
         let program = Program::new(
             builtins.clone(),
-            felt::PRIME_STR.to_string(),
             data.clone(),
             None,
             HashMap::new(),
@@ -207,9 +226,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 +285,6 @@ mod tests {
 
         let program = Program::new(
             builtins,
-            felt::PRIME_STR.to_string(),
             data,
             None,
             HashMap::new(),
@@ -356,10 +374,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 +473,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 +525,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());
     }
 }