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

perf: rework MemoryCell for cache efficiency #1672

Merged
merged 9 commits into from
May 6, 2024
Merged

perf: rework MemoryCell for cache efficiency #1672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
96835a2
perf: rework `MemoryCell` for cache efficiency
Oppen Mar 12, 2024
4efb05b
Merge branch 'main' into perf/compact_memory_cell_main
fmoletta Mar 26, 2024
bd19b74
Merge branch 'main' into perf/compact_memory_cell_main
Apr 15, 2024
9a8b871
Merge branch 'main' into perf/compact_memory_cell_main
pefontana Apr 16, 2024
8828880
Merge branch 'main' of https://github.com/lambdaclass/cairo-vm into p…
fmoletta Apr 26, 2024
d33a3c7
fmt
fmoletta Apr 26, 2024
b304f9a
Merge branch 'main' into perf/compact_memory_cell_main
fmoletta Apr 30, 2024
c8df124
Merge branch 'main' into perf/compact_memory_cell_main
fmoletta May 2, 2024
f8398ca
Merge branch 'main' into perf/compact_memory_cell_main
pefontana May 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

#### Upcoming Changes

* perf: use a more compact representation for `MemoryCell` [#1672](https://github.com/lambdaclass/cairo-vm/pull/1672)
* BREAKING: `Memory::get_value` will now always return `Cow::Owned` variants, code that relied on `Cow::Borrowed` may break

#### [1.0.0-rc2] - 2024-05-02


* `cairo1-run` CLI: Allow loading arguments from file[#1739](https://github.com/lambdaclass/cairo-vm/pull/1739)

* BREAKING: Remove unused `CairoRunner` field `original_steps`[#1742](https://github.com/lambdaclass/cairo-vm/pull/1742)
Expand Down
19 changes: 9 additions & 10 deletions vm/src/vm/runners/builtin_runner/ec_op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::air_private_input::{PrivateInput, PrivateInputEcOp};
use crate::stdlib::{borrow::Cow, prelude::*};
use crate::stdlib::prelude::*;
use crate::stdlib::{cell::RefCell, collections::HashMap};
use crate::types::instance_definitions::ec_op_instance_def::{
CELLS_PER_EC_OP, INPUT_CELLS_PER_EC_OP, SCALAR_HEIGHT,
Expand Down Expand Up @@ -122,14 +122,13 @@ impl EcOpBuiltinRunner {

//All input cells should be filled, and be integer values
//If an input cell is not filled, return None
let mut input_cells = Vec::<&Felt252>::with_capacity(INPUT_CELLS_PER_EC_OP as usize);
let mut input_cells = Vec::<Felt252>::with_capacity(INPUT_CELLS_PER_EC_OP as usize);
for i in 0..INPUT_CELLS_PER_EC_OP as usize {
match memory.get(&(instance + i)?) {
None => return Ok(None),
Some(addr) => {
input_cells.push(match addr {
// Only relocatable values can be owned
Cow::Borrowed(MaybeRelocatable::Int(ref num)) => num,
input_cells.push(match addr.as_ref() {
MaybeRelocatable::Int(num) => *num,
_ => {
return Err(RunnerError::Memory(MemoryError::ExpectedInteger(
Box::new((instance + i)?),
Expand All @@ -149,21 +148,21 @@ impl EcOpBuiltinRunner {
// Assert that if the current address is part of a point, the point is on the curve
for pair in &EC_POINT_INDICES[0..2] {
if !EcOpBuiltinRunner::point_on_curve(
input_cells[pair.0],
input_cells[pair.1],
&input_cells[pair.0],
&input_cells[pair.1],
&alpha,
&beta,
) {
return Err(RunnerError::PointNotOnCurve(Box::new((
*input_cells[pair.0],
*input_cells[pair.1],
input_cells[pair.0],
input_cells[pair.1],
))));
};
}
let result = EcOpBuiltinRunner::ec_op_impl(
(input_cells[0].to_owned(), input_cells[1].to_owned()),
(input_cells[2].to_owned(), input_cells[3].to_owned()),
input_cells[4],
&input_cells[4],
SCALAR_HEIGHT,
)?;
self.cache.borrow_mut().insert(x_addr, result.0);
Expand Down
15 changes: 12 additions & 3 deletions vm/src/vm/runners/builtin_runner/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,11 @@ impl BuiltinRunner {
for i in 0..n {
for j in 0..n_input_cells {
let offset = cells_per_instance * i + j;
if let None | Some(None) = builtin_segment.get(offset) {
if builtin_segment
.get(offset)
.filter(|x| x.is_some())
.is_none()
{
missing_offsets.push(offset)
}
}
Expand All @@ -463,7 +467,11 @@ impl BuiltinRunner {
for i in 0..n {
for j in n_input_cells..cells_per_instance {
let offset = cells_per_instance * i + j;
if let None | Some(None) = builtin_segment.get(offset) {
if builtin_segment
.get(offset)
.filter(|x| x.is_some())
.is_none()
{
vm.verify_auto_deductions_for_addr(
Relocatable::from((builtin_segment_index as isize, offset)),
self,
Expand Down Expand Up @@ -651,6 +659,7 @@ mod tests {
use crate::types::program::Program;
use crate::vm::errors::memory_errors::InsufficientAllocatedCellsError;
use crate::vm::runners::cairo_runner::CairoRunner;
use crate::vm::vm_memory::memory::MemoryCell;
use crate::{utils::test_utils::*, vm::vm_core::VirtualMachine};
use assert_matches::assert_matches;

Expand Down Expand Up @@ -1321,7 +1330,7 @@ mod tests {

let mut vm = vm!();

vm.segments.memory.data = vec![vec![None, None, None]];
vm.segments.memory.data = vec![vec![MemoryCell::NONE, MemoryCell::NONE, MemoryCell::NONE]];

assert_matches!(builtin.run_security_checks(&vm), Ok(()));
}
Expand Down
7 changes: 3 additions & 4 deletions vm/src/vm/runners/builtin_runner/range_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ impl<const N_PARTS: u64> RangeCheckBuiltinRunner<N_PARTS> {
// Split value into n_parts parts of less than _INNER_RC_BOUND size.
for value in range_check_segment {
rc_bounds = value
.as_ref()?
.get_value()
.get_value()?
.get_int_ref()?
.to_le_digits()
// TODO: maybe skip leading zeros
Expand Down Expand Up @@ -151,8 +150,8 @@ impl<const N_PARTS: u64> RangeCheckBuiltinRunner<N_PARTS> {
pub fn air_private_input(&self, memory: &Memory) -> Vec<PrivateInput> {
let mut private_inputs = vec![];
if let Some(segment) = memory.data.get(self.base) {
for (index, val) in segment.iter().enumerate() {
if let Some(value) = val.as_ref().and_then(|cell| cell.get_value().get_int()) {
for (index, cell) in segment.iter().enumerate() {
if let Some(value) = cell.get_value().and_then(|value| value.get_int()) {
private_inputs.push(PrivateInput::Value(PrivateInputValue { index, value }))
}
}
Expand Down
40 changes: 20 additions & 20 deletions vm/src/vm/runners/cairo_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -944,13 +944,13 @@ impl CairoRunner {
self.relocated_memory.push(None);
for (index, segment) in vm.segments.memory.data.iter().enumerate() {
for (seg_offset, cell) in segment.iter().enumerate() {
match cell {
match cell.get_value() {
Some(cell) => {
let relocated_addr = relocate_address(
Relocatable::from((index as isize, seg_offset)),
relocation_table,
)?;
let value = relocate_value(cell.get_value().clone(), relocation_table)?;
let value = relocate_value(cell, relocation_table)?;
if self.relocated_memory.len() <= relocated_addr {
self.relocated_memory.resize(relocated_addr + 1, None);
}
Expand Down Expand Up @@ -4098,11 +4098,11 @@ mod tests {

vm.segments.memory.data = vec![
vec![
Some(MemoryCell::new(Felt252::from(0x8000_8023_8012u64).into())),
Some(MemoryCell::new(Felt252::from(0xBFFF_8000_0620u64).into())),
Some(MemoryCell::new(Felt252::from(0x8FFF_8000_0750u64).into())),
MemoryCell::new(Felt252::from(0x8000_8023_8012u64).into()),
MemoryCell::new(Felt252::from(0xBFFF_8000_0620u64).into()),
MemoryCell::new(Felt252::from(0x8FFF_8000_0750u64).into()),
],
vec![Some(MemoryCell::new((0isize, 0usize).into())); 128 * 1024],
vec![MemoryCell::new((0isize, 0usize).into()); 128 * 1024],
];

cairo_runner
Expand All @@ -4125,9 +4125,9 @@ mod tests {
let cairo_runner = cairo_runner!(program);
let mut vm = vm!();

vm.segments.memory.data = vec![vec![Some(MemoryCell::new(mayberelocatable!(
vm.segments.memory.data = vec![vec![MemoryCell::new(mayberelocatable!(
0x80FF_8000_0530u64
)))]];
))]];
vm.builtin_runners =
vec![RangeCheckBuiltinRunner::<RC_N_PARTS_STANDARD>::new(Some(12), true).into()];

Expand Down Expand Up @@ -4162,9 +4162,9 @@ mod tests {
let mut vm = vm!();
vm.builtin_runners = vec![];
vm.current_step = 10000;
vm.segments.memory.data = vec![vec![Some(MemoryCell::new(mayberelocatable!(
vm.segments.memory.data = vec![vec![MemoryCell::new(mayberelocatable!(
0x80FF_8000_0530u64
)))]];
))]];
vm.trace = Some(vec![TraceEntry {
pc: (0, 0).into(),
ap: 0,
Expand All @@ -4185,9 +4185,9 @@ mod tests {
let mut vm = vm!();
vm.builtin_runners =
vec![RangeCheckBuiltinRunner::<RC_N_PARTS_STANDARD>::new(Some(8), true).into()];
vm.segments.memory.data = vec![vec![Some(MemoryCell::new(mayberelocatable!(
vm.segments.memory.data = vec![vec![MemoryCell::new(mayberelocatable!(
0x80FF_8000_0530u64
)))]];
))]];
vm.trace = Some(vec![TraceEntry {
pc: (0, 0).into(),
ap: 0,
Expand Down Expand Up @@ -4253,9 +4253,9 @@ mod tests {
let mut vm = vm!();
vm.builtin_runners =
vec![RangeCheckBuiltinRunner::<RC_N_PARTS_STANDARD>::new(Some(8), true).into()];
vm.segments.memory.data = vec![vec![Some(MemoryCell::new(mayberelocatable!(
vm.segments.memory.data = vec![vec![MemoryCell::new(mayberelocatable!(
0x80FF_8000_0530u64
)))]];
))]];
vm.trace = Some(vec![TraceEntry {
pc: (0, 0).into(),
ap: 0,
Expand Down Expand Up @@ -4750,7 +4750,7 @@ mod tests {
vm.builtin_runners.push(output_builtin.into());
vm.segments.memory.data = vec![
vec![],
vec![Some(MemoryCell::new(MaybeRelocatable::from((0, 0))))],
vec![MemoryCell::new(MaybeRelocatable::from((0, 0)))],
vec![],
];
vm.set_ap(1);
Expand Down Expand Up @@ -4780,8 +4780,8 @@ mod tests {
let output_builtin = OutputBuiltinRunner::new(true);
vm.builtin_runners.push(output_builtin.into());
vm.segments.memory.data = vec![
vec![Some(MemoryCell::new(MaybeRelocatable::from((0, 0))))],
vec![Some(MemoryCell::new(MaybeRelocatable::from((0, 1))))],
vec![MemoryCell::new(MaybeRelocatable::from((0, 0)))],
vec![MemoryCell::new(MaybeRelocatable::from((0, 1)))],
vec![],
];
vm.set_ap(1);
Expand Down Expand Up @@ -4814,10 +4814,10 @@ mod tests {
vm.builtin_runners.push(bitwise_builtin.into());
cairo_runner.initialize_segments(&mut vm, None);
vm.segments.memory.data = vec![
vec![Some(MemoryCell::new(MaybeRelocatable::from((0, 0))))],
vec![MemoryCell::new(MaybeRelocatable::from((0, 0)))],
vec![
Some(MemoryCell::new(MaybeRelocatable::from((2, 0)))),
Some(MemoryCell::new(MaybeRelocatable::from((3, 5)))),
MemoryCell::new(MaybeRelocatable::from((2, 0))),
MemoryCell::new(MaybeRelocatable::from((3, 5))),
],
vec![],
];
Expand Down
4 changes: 2 additions & 2 deletions vm/src/vm/security.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ pub fn verify_secure_runner(
// Asumption: If temporary memory is empty, this means no temporary memory addresses were generated and all addresses in memory are real
if !vm.segments.memory.temp_data.is_empty() {
for value in vm.segments.memory.data.iter().flatten() {
match value.as_ref().map(|x| x.get_value()) {
match value.get_value() {
Some(MaybeRelocatable::RelocatableValue(addr)) if addr.segment_index < 0 => {
return Err(VirtualMachineError::InvalidMemoryValueTemporaryAddress(
Box::new(*addr),
Box::new(addr),
))
}
_ => {}
Expand Down
60 changes: 30 additions & 30 deletions vm/src/vm/vm_core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,12 +695,12 @@ impl VirtualMachine {
)
.map_err(VirtualMachineError::RunnerError)?
{
let value = value.as_ref().map(|x| x.get_value());
if Some(&deduced_memory_cell) != value && value.is_some() {
let value = value.get_value();
if Some(&deduced_memory_cell) != value.as_ref() && value.is_some() {
return Err(VirtualMachineError::InconsistentAutoDeduction(Box::new((
builtin.name(),
deduced_memory_cell,
value.cloned(),
value,
))));
}
}
Expand Down Expand Up @@ -3039,8 +3039,8 @@ mod tests {
//Check that the following addresses have been accessed:
// Addresses have been copied from python execution:
let mem = vm.segments.memory.data;
assert!(mem[1][0].as_ref().unwrap().is_accessed());
assert!(mem[1][1].as_ref().unwrap().is_accessed());
assert!(mem[1][0].is_accessed());
assert!(mem[1][1].is_accessed());
}

#[test]
Expand Down Expand Up @@ -3137,15 +3137,15 @@ mod tests {
//Check that the following addresses have been accessed:
// Addresses have been copied from python execution:
let mem = &vm.segments.memory.data;
assert!(mem[0][1].as_ref().unwrap().is_accessed());
assert!(mem[0][4].as_ref().unwrap().is_accessed());
assert!(mem[0][6].as_ref().unwrap().is_accessed());
assert!(mem[1][0].as_ref().unwrap().is_accessed());
assert!(mem[1][1].as_ref().unwrap().is_accessed());
assert!(mem[1][2].as_ref().unwrap().is_accessed());
assert!(mem[1][3].as_ref().unwrap().is_accessed());
assert!(mem[1][4].as_ref().unwrap().is_accessed());
assert!(mem[1][5].as_ref().unwrap().is_accessed());
assert!(mem[0][1].is_accessed());
assert!(mem[0][4].is_accessed());
assert!(mem[0][6].is_accessed());
assert!(mem[1][0].is_accessed());
assert!(mem[1][1].is_accessed());
assert!(mem[1][2].is_accessed());
assert!(mem[1][3].is_accessed());
assert!(mem[1][4].is_accessed());
assert!(mem[1][5].is_accessed());
assert_eq!(
vm.segments
.memory
Expand Down Expand Up @@ -4273,11 +4273,11 @@ mod tests {
//Check that the following addresses have been accessed:
// Addresses have been copied from python execution:
let mem = &vm.segments.memory.data;
assert!(mem[0][0].as_ref().unwrap().is_accessed());
assert!(mem[0][1].as_ref().unwrap().is_accessed());
assert!(mem[0][2].as_ref().unwrap().is_accessed());
assert!(mem[0][10].as_ref().unwrap().is_accessed());
assert!(mem[1][1].as_ref().unwrap().is_accessed());
assert!(mem[0][0].is_accessed());
assert!(mem[0][1].is_accessed());
assert!(mem[0][2].is_accessed());
assert!(mem[0][10].is_accessed());
assert!(mem[1][1].is_accessed());
assert_eq!(
vm.segments
.memory
Expand Down Expand Up @@ -4499,8 +4499,8 @@ mod tests {
//Check that the following addresses have been accessed:
// Addresses have been copied from python execution:
let mem = vm.segments.memory.data;
assert!(mem[1][0].as_ref().unwrap().is_accessed());
assert!(mem[1][1].as_ref().unwrap().is_accessed());
assert!(mem[1][0].is_accessed());
assert!(mem[1][1].is_accessed());
}

#[test]
Expand Down Expand Up @@ -4600,15 +4600,15 @@ mod tests {
//Check that the following addresses have been accessed:
// Addresses have been copied from python execution:
let mem = &vm.segments.memory.data;
assert!(mem[4][1].as_ref().unwrap().is_accessed());
assert!(mem[4][4].as_ref().unwrap().is_accessed());
assert!(mem[4][6].as_ref().unwrap().is_accessed());
assert!(mem[1][0].as_ref().unwrap().is_accessed());
assert!(mem[1][1].as_ref().unwrap().is_accessed());
assert!(mem[1][2].as_ref().unwrap().is_accessed());
assert!(mem[1][3].as_ref().unwrap().is_accessed());
assert!(mem[1][4].as_ref().unwrap().is_accessed());
assert!(mem[1][5].as_ref().unwrap().is_accessed());
assert!(mem[4][1].is_accessed());
assert!(mem[4][4].is_accessed());
assert!(mem[4][6].is_accessed());
assert!(mem[1][0].is_accessed());
assert!(mem[1][1].is_accessed());
assert!(mem[1][2].is_accessed());
assert!(mem[1][3].is_accessed());
assert!(mem[1][4].is_accessed());
assert!(mem[1][5].is_accessed());
assert_eq!(
vm.segments
.memory
Expand Down
Loading
Loading