Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#### Upcoming Changes

* opt(breaking): Optimize Cairo 0 Execution [#2206](https://github.com/lambdaclass/cairo-vm/pull/2206)

#### [2.5.0] - 2025-09-11

* breaking: Store constants in Hint Data [#2191](https://github.com/lambdaclass/cairo-vm/pull/2191)
Expand Down
2 changes: 1 addition & 1 deletion vm/src/hint_processor/hint_processor_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fn get_ids_data(
reference_ids: &HashMap<String, usize>,
references: &[HintReference],
) -> Result<HashMap<String, HintReference>, VirtualMachineError> {
let mut ids_data = HashMap::<String, HintReference>::new();
let mut ids_data = HashMap::<String, HintReference>::with_capacity(reference_ids.len());
for (path, ref_id) in reference_ids {
let name = path
.rsplit('.')
Expand Down
52 changes: 52 additions & 0 deletions vm/src/vm/vm_memory/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,58 @@ impl Memory {
self.mark_as_accessed(key);
Ok(())
}

/// Inserts multiple values into contiguous memory addresses.
///
/// If the address isn't contiguous with previously inserted data, memory gaps will be represented by NONE values
///
/// Returns an error if:
/// - The segment index given by the address corresponds to a non-allocated segment.
/// - An inserted value is inconsistent with the current value at the memory cell
pub fn insert_all(
&mut self,
key: Relocatable,
vals: &[MaybeRelocatable],
) -> Result<(), MemoryError> {
let segment = self.get_segment(key)?;
let (_, value_offset) = from_relocatable_to_indexes(key);

// Allocate space for all new elements.
if segment.len() < value_offset + vals.len() {
segment.reserve(value_offset + vals.len() - segment.len());
}
Comment on lines +764 to +766
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think we can remove the if since the documentation of reserve() says:

Does nothing if capacity is already sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe they refer to different things:

  • segment.len() < value_offset + vals.len() checks whether the length of the segment is enough for holding the new elements. If not, it reserves capacity for the additional elements.
  • the inner function checks whether the capacity of the segment is enough for holding the new elements. The capacity refers to the vector's allocated memory.

The if is required, as we only need to reserve new elements if the segment's length is not enough already. Without the condition, we would be sometimes be calling reserve with a negative argument (which would cause underflow as we are using a usize).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...we would be sometimes be calling reserve with a negative argument.

I thought that value_offset is always higher than segment.len().

Maybe I have the wrong understanding about segments, but if the lenght of a segment is 5. Doesn't it mean it has 5 allocated elements? If that is the case, having an offset lower than the lenght means it would want to write on already used memory which is something that it cannot be done, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that value_offset is always higher than segment.len().

I think that in load_data that is usually the case, but I'm not sure it would happen always. Consider the following segment:

[NONE, NONE, NONE, 10, 20]

We may want to call insert_all to insert 3 elements at the start of the segment. In that case, there is no need to reserve more space. Note that having NONE is completely valid in a segment, those are commonly known as "memory gaps".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, I forgot you could have those. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, insert_all is generic and supports the use case of inserting multiple elements at the middle of a segment.

If we make sure that load_data can only insert elements at the end of a segment, we could have another method (i.e. extend_at), only used for when inserting elements at the end of a segment. This could improve performance.

// Fill middle with NONE.
if segment.len() < value_offset {
segment.resize(value_offset, MemoryCell::NONE);
}
// Insert new elements.
let last_element_to_replace = segment.len().min(value_offset + vals.len());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn´t the last index always value_offset + vals.len()? I don´t get in which case the segments len would be higher than that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of splice is a bit tricky.

It receives two arguments:

  • The range to replace.
  • the elements to replace it with.

The length of the range and the length of the replacement does not need to coincide. For example, consider the following array:

[0, 1, 2, 3, 4, 5]

If we want to insert [6,7,8] at index 4, we would be inserting 3 elements, but replacing only 2. The splice call would look like this:

splice(4..6, [6,7,8])

The result would look like this:

[0, 1, 2, 3, 6, 7, 8]

The following, instead, fails with index out of bounds, because we are replacing an element that does not exist.

splice(4..7, [6,7,8])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh I see. Awesome, thanks!

let replaced = segment.splice(
value_offset..last_element_to_replace,
vals.iter().map(|v| MemoryCell::new(v.clone())),
);

// Check for inconsistencies.
let inconsistent_cell = replaced.enumerate().find(|(idx, replaced)| {
replaced
.get_value()
.is_some_and(|replaced| replaced != vals[*idx])
});
if let Some((insertions_idx, current_value)) = inconsistent_cell {
return Err(MemoryError::InconsistentMemory(Box::new((
(key + insertions_idx)?,
current_value.into(),
vals[insertions_idx].clone(),
))));
}

// Validate inserted memory cells
for i in 0..vals.len() {
self.validate_memory_cell((key + i)?)?;
}

Ok(())
}
}

impl From<&Memory> for CairoPieMemory {
Expand Down
6 changes: 2 additions & 4 deletions vm/src/vm/vm_memory/memory_segments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,8 @@ impl MemorySegmentManager {
ptr: Relocatable,
data: &[MaybeRelocatable],
) -> Result<Relocatable, MemoryError> {
// Starting from the end ensures any necessary resize
// is performed once with enough room for everything
for (num, value) in data.iter().enumerate().rev() {
self.memory.insert((ptr + num)?, value)?;
if !data.is_empty() {
self.memory.insert_all(ptr, data)?;
}
(ptr + data.len()).map_err(MemoryError::Math)
}
Expand Down
Loading