Skip to content

Commit

Permalink
perf: remove unneeded bitshift in validated addresses insertion (#1208)
Browse files Browse the repository at this point in the history
* Remove unneeded plus one

* Clean up `validate_*`'s code

* Replace `insert` with `push` in extend

* Change approach

The +1 in resize is used to avoid having to allocate in the case the next push falls outside capacity.
Instead, we use a replace in the place of the insert.

* Update changelog

---------

Co-authored-by: Pedro Fontana <[email protected]>
  • Loading branch information
MegaRedHand and pefontana authored Jul 31, 2023
1 parent 1642e70 commit e7a0022
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 15 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

* fix: CLI errors bad formatting and handling

* perf: replace insertion with bit-setting in validated addresses [#1208](https://github.com/lambdaclass/cairo-vm/pull/1208)

* fix: return error when a parsed hint's PC is invalid [#1340](https://github.com/lambdaclass/cairo-vm/pull/1340)

* chore(deps): bump _cairo-lang_ dependencies to v2.1.0-rc2 [#1345](https://github.com/lambdaclass/cairo-vm/pull/1345)
Expand Down
29 changes: 14 additions & 15 deletions vm/src/vm/vm_memory/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ impl AddressSet {
if offset >= self.0[segment].len() {
self.0[segment].resize(offset + 1, false);
}

self.0[segment].insert(offset, true);
self.0[segment].replace(offset, true);
}
}
}
Expand Down Expand Up @@ -319,10 +318,8 @@ impl Memory {
.and_then(|x| self.validation_rules.get(x))
{
if !self.validated_addresses.contains(&addr) {
{
self.validated_addresses
.extend(rule.0(self, addr)?.as_slice());
}
self.validated_addresses
.extend(rule.0(self, addr)?.as_slice());
}
}
Ok(())
Expand All @@ -331,15 +328,17 @@ impl Memory {
///Applies validation_rules to the current memory
pub fn validate_existing_memory(&mut self) -> Result<(), MemoryError> {
for (index, rule) in self.validation_rules.iter().enumerate() {
if let Some(rule) = rule {
if index < self.data.len() {
for offset in 0..self.data[index].len() {
let addr = Relocatable::from((index as isize, offset));
if !self.validated_addresses.contains(&addr) {
self.validated_addresses
.extend(rule.0(self, addr)?.as_slice());
}
}
if index >= self.data.len() {
continue;
}
let Some(rule) = rule else {
continue;
};
for offset in 0..self.data[index].len() {
let addr = Relocatable::from((index as isize, offset));
if !self.validated_addresses.contains(&addr) {
self.validated_addresses
.extend(rule.0(self, addr)?.as_slice());
}
}
}
Expand Down

1 comment on commit e7a0022

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.30.

Benchmark suite Current: e7a0022 Previous: 1642e70 Ratio
add_u64_with_felt/1 3 ns/iter (± 0) 2 ns/iter (± 0) 1.50
add_u64_with_felt/2 3 ns/iter (± 0) 2 ns/iter (± 0) 1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @unbalancedparentheses

Please sign in to comment.