Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

perf!: move identifiers to SharedProgramData #1023

Merged
merged 2 commits into from
Apr 20, 2023
Merged

perf!: move identifiers to SharedProgramData #1023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
401d690
perf!: move `identifiers` to `SharedProgramData`
Oppen Apr 20, 2023
44bca19
Merge branch 'main' into perf/share_program_identifiers
Oppen Apr 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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