Skip to content

Commit

Permalink
perf!: move identifiers to SharedProgramData (#1023)
Browse files Browse the repository at this point in the history 
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)`.
  • Loading branch information
@Oppen
Oppen committed Apr 20, 2023
1 parent a178759 commit 6ca401a
Show file tree
Hide file tree
Showing 5 changed files with 190 additions and 121 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

#### 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 `Program::get_identifier(&self, id: &str) -> &Identifier` to get a single identifier by name

* Implement hints on field_arithmetic lib[#985](https://github.com/lambdaclass/cairo-rs/pull/983)

`BuiltinHintProcessor` now supports the following hint:
Expand Down Expand Up @@ -77,7 +82,7 @@
ids.res.high = res_split[1]
```

* Add methor `Program::data_len(&self) -> usize` to get the number of data cells in a given program [#1022](https://github.com/lambdaclass/cairo-rs/pull/1022)
* Add method `Program::data_len(&self) -> usize` to get the number of data cells in a given program [#1022](https://github.com/lambdaclass/cairo-rs/pull/1022)

* Add missing hint on uint256_improvements lib [#1013](https://github.com/lambdaclass/cairo-rs/pull/1013):

Expand Down
28 changes: 13 additions & 15 deletions src/serde/deserialize_program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
})
}

Expand Down
134 changes: 102 additions & 32 deletions src/types/program.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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,
})
}

Expand Down Expand Up @@ -123,6 +120,10 @@ impl Program {
pub fn data_len(&self) -> usize {
self.shared_program_data.data.len()
}

pub fn get_identifier(&self, id: &str) -> Option<&Identifier> {
self.shared_program_data.identifiers.get(id)
}
}

impl Default for Program {
Expand All @@ -133,7 +134,6 @@ impl Default for Program {
reference_manager: ReferenceManager {
references: Vec::new(),
},
identifiers: HashMap::new(),
}
}
}
Expand Down Expand Up @@ -181,7 +181,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]
Expand Down Expand Up @@ -243,7 +243,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())]
Expand Down Expand Up @@ -359,6 +359,76 @@ mod tests {
assert_eq!(program.data_len(), data.len());
}

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn get_identifier() {
let reference_manager = ReferenceManager {
references: Vec::new(),
};

let builtins: Vec<BuiltinName> = Vec::new();

let data: Vec<MaybeRelocatable> = vec![
mayberelocatable!(5189976364521848832),
mayberelocatable!(1000),
mayberelocatable!(5189976364521848832),
mayberelocatable!(2000),
mayberelocatable!(5201798304953696256),
mayberelocatable!(2345108766317314046),
];

let mut identifiers: HashMap<String, Identifier> = HashMap::new();

identifiers.insert(
String::from("__main__.main"),
Identifier {
pc: Some(0),
type_: Some(String::from("function")),
value: None,
full_name: None,
members: None,
cairo_type: None,
},
);

identifiers.insert(
String::from("__main__.main.SIZEOF_LOCALS"),
Identifier {
pc: None,
type_: Some(String::from("const")),
value: Some(Felt252::zero()),
full_name: None,
members: None,
cairo_type: None,
},
);

let program = Program::new(
builtins,
data,
None,
HashMap::new(),
reference_manager,
identifiers.clone(),
Vec::new(),
None,
)
.unwrap();

assert_eq!(
program.get_identifier("__main__.main"),
identifiers.get("__main__.main"),
);
assert_eq!(
program.get_identifier("__main__.main.SIZEOF_LOCALS"),
identifiers.get("__main__.main.SIZEOF_LOCALS"),
);
assert_eq!(
program.get_identifier("missing"),
identifiers.get("missing"),
);
}

#[test]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn new_program_with_invalid_identifiers() {
Expand Down Expand Up @@ -497,7 +567,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.
Expand Down Expand Up @@ -596,7 +666,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
Expand Down Expand Up @@ -654,14 +724,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());
Expand Down
16 changes: 4 additions & 12 deletions src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
$(
Expand Down Expand Up @@ -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!())
}
Expand All @@ -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])
Expand All @@ -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!(
Expand Down

0 comments on commit 6ca401a

Please sign in to comment.