Skip to content

Commit

Permalink
perf: Add extensive_hints feature to prevent performance regression…
Browse files Browse the repository at this point in the history
… for the common use-case (#1503)

* Gate hint changes behind new feature

* Add feature documentation

* Add load_program feature to workflows

* Fix

* Fixes

* Rename feature

* Add changelog

* Fix hint name in comment

* Update workflow

* Fix test

* Fix test

* Two separate step_hint methods for each feature
  • Loading branch information
fmoletta authored Dec 5, 2023
1 parent db708e3 commit 8a2ef24
Show file tree
Hide file tree
Showing 9 changed files with 167 additions and 24 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ jobs:
strategy:
fail-fast: false
matrix:
special_features: ["", "lambdaworks-felt"]
special_features: ["", "lambdaworks-felt", "extensive_hints"]
target: [ test#1, test#2, test#3, test#4, test-no_std#1, test-no_std#2, test-no_std#3, test-no_std#4, test-wasm ]
name: Run tests
runs-on: ubuntu-22.04
Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

#### Upcoming Changes

* perf: Add `extensive_hints` feature to prevent performance regression for the common use-case [#1503] (https://github.com/lambdaclass/cairo-vm/pull/1503)

* Gates changes added by #1491 under the feature flag `extensive_hints`

* chore: remove cancel-duplicates workflow [#1497](https://github.com/lambdaclass/cairo-vm/pull/1497)

* feat: Handle `pc`s outside of program segment in `VmException` [#1501] (https://github.com/lambdaclass/cairo-vm/pull/1501)
Expand All @@ -18,6 +22,7 @@
* Add `relocated_trace` field & `relocate_trace` method to `CairoRunner`.
* Swap `TraceEntry` for `RelocatedTraceEntry` type in `write_encoded_trace` & `PublicInput::new` signatures.
* Now takes into account the program counter's segment index when building the execution trace instead of assuming it to be 0.

* feat: Add HintProcessor::execute_hint_extensive + refactor hint_ranges [#1491](https://github.com/lambdaclass/cairo-vm/pull/1491)

* Add trait method `HintProcessorLogic::execute_hint_extensive`:
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ test-no_std: cairo_proof_programs cairo_test_programs
test-wasm: cairo_proof_programs cairo_test_programs
# NOTE: release mode is needed to avoid "too many locals" error
wasm-pack test --release --node vm --no-default-features
test-extensive_hints: cairo_proof_programs cairo_test_programs
cargo llvm-cov nextest --no-report --workspace --features "test_utils, cairo-1-hints, extensive_hints"

check-fmt:
cargo fmt --all -- --check
Expand Down
3 changes: 3 additions & 0 deletions vm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ cairo-1-hints = [
]
arbitrary = ["dep:arbitrary", "felt/arbitrary", "felt/std", "std"]
lambdaworks-felt = ["felt/lambdaworks-felt"]
# Allows extending the set of hints for the current vm run from within a hint.
# For a usage example checkout vm/src/tests/run_deprecated_contract_class_simplified.rs
extensive_hints = []

# Note that these features are not retro-compatible with the cairo Python VM.
test_utils = [
Expand Down
5 changes: 2 additions & 3 deletions vm/src/hint_processor/hint_processor_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ use arbitrary::Arbitrary;

pub trait HintProcessorLogic {
// Executes the hint which's data is provided by a dynamic structure previously created by compile_hint
// Note: The method used by the vm to execute hints is `execute_hint_extensive`.
// The default implementation for `execute_hint_extensive` calls this method, so it can be ignored unless loading hints during the vm run is needed.
// If you chose to implement `execute_hint_extensive` instead of using the default implementation, then this method will not be used.
// Note: if the `extensive_hints` feature is activated the method used by the vm to execute hints is `execute_hint_extensive`, which's default implementation calls this method.
fn execute_hint(
&mut self,
vm: &mut VirtualMachine,
Expand Down Expand Up @@ -53,6 +51,7 @@ pub trait HintProcessorLogic {
}))
}

#[cfg(feature = "extensive_hints")]
// Executes the hint which's data is provided by a dynamic structure previously created by compile_hint
// Also returns a map of hints to be loaded after the current hint is executed
// Note: This is the method used by the vm to execute hints,
Expand Down
1 change: 1 addition & 0 deletions vm/src/tests/run_deprecated_contract_class_simplified.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg(feature = "extensive_hints")]
/* This file contains a test that runs the program: cairo_programs/starknet_os_deprecated_cc.cairo
For testsing purposes, the contract ran by this program is hardcoded, with values taken from compiling:
Expand Down
94 changes: 78 additions & 16 deletions vm/src/types/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ use felt::{Felt252, PRIME_STR};
#[cfg(feature = "std")]
use std::path::Path;

#[cfg(feature = "extensive_hints")]
use super::relocatable::Relocatable;
#[cfg(all(feature = "arbitrary", feature = "std"))]
use arbitrary::{Arbitrary, Unstructured};

use super::relocatable::Relocatable;

// NOTE: `Program` has been split in two containing some data that will be deep-copied
// and some that will be allocated on the heap inside an `Arc<_>`.
// This is because it has been reported that cloning the whole structure when creating
Expand Down Expand Up @@ -108,6 +108,9 @@ impl<'a> Arbitrary<'a> for SharedProgramData {
pub(crate) struct HintsCollection {
hints: Vec<HintParams>,
/// This maps a PC to the range of hints in `hints` that correspond to it.
#[cfg(not(feature = "extensive_hints"))]
pub(crate) hints_ranges: Vec<HintRange>,
#[cfg(feature = "extensive_hints")]
pub(crate) hints_ranges: HashMap<Relocatable, HintRange>,
}

Expand All @@ -124,7 +127,7 @@ impl HintsCollection {
let Some((max_hint_pc, full_len)) = bounds else {
return Ok(HintsCollection {
hints: Vec::new(),
hints_ranges: HashMap::new(),
hints_ranges: Default::default(),
});
};

Expand All @@ -133,13 +136,20 @@ impl HintsCollection {
}

let mut hints_values = Vec::with_capacity(full_len);
let mut hints_ranges = HashMap::new();

#[cfg(not(feature = "extensive_hints"))]
let mut hints_ranges = vec![None; max_hint_pc + 1];
#[cfg(feature = "extensive_hints")]
let mut hints_ranges = HashMap::default();
for (pc, hs) in hints.iter().filter(|(_, hs)| !hs.is_empty()) {
let range = (
hints_values.len(),
NonZeroUsize::new(hs.len()).expect("empty vecs already filtered"),
);
#[cfg(not(feature = "extensive_hints"))]
{
hints_ranges[*pc] = Some(range)
};
#[cfg(feature = "extensive_hints")]
hints_ranges.insert(Relocatable::from((0_isize, *pc)), range);
hints_values.extend_from_slice(&hs[..]);
}
Expand All @@ -153,11 +163,24 @@ impl HintsCollection {
pub fn iter_hints(&self) -> impl Iterator<Item = &HintParams> {
self.hints.iter()
}

#[cfg(not(feature = "extensive_hints"))]
pub fn get_hint_range_for_pc(&self, pc: usize) -> Option<HintRange> {
self.hints_ranges.get(pc).cloned()
}
}

impl From<&HintsCollection> for BTreeMap<usize, Vec<HintParams>> {
fn from(hc: &HintsCollection) -> Self {
let mut hint_map = BTreeMap::new();
#[cfg(not(feature = "extensive_hints"))]
for (i, r) in hc.hints_ranges.iter().enumerate() {
let Some(r) = r else {
continue;
};
hint_map.insert(i, hc.hints[r.0..r.0 + r.1.get()].to_owned());
}
#[cfg(feature = "extensive_hints")]
for (pc, r) in hc.hints_ranges.iter() {
hint_map.insert(pc.offset, hc.hints[r.0..r.0 + r.1.get()].to_owned());
}
Expand All @@ -166,6 +189,10 @@ impl From<&HintsCollection> for BTreeMap<usize, Vec<HintParams>> {
}

/// Represents a range of hints corresponding to a PC as a tuple `(start, length)`.
#[cfg(not(feature = "extensive_hints"))]
/// Is [`None`] if the range is empty, and it is [`Some`] tuple `(start, length)` otherwise.
type HintRange = Option<(usize, NonZeroUsize)>;
#[cfg(feature = "extensive_hints")]
pub type HintRange = (usize, NonZeroUsize);

#[cfg_attr(all(feature = "arbitrary", feature = "std"), derive(Arbitrary))]
Expand Down Expand Up @@ -418,14 +445,31 @@ impl TryFrom<CasmContractClass> for Program {
#[cfg(test)]
impl HintsCollection {
pub fn iter(&self) -> impl Iterator<Item = (usize, &[HintParams])> {
self.hints_ranges.iter().filter_map(|(pc, (start, len))| {
#[cfg(not(feature = "extensive_hints"))]
let iter = self
.hints_ranges
.iter()
.enumerate()
.filter_map(|(pc, range)| {
range.and_then(|(start, len)| {
let end = start + len.get();
if end <= self.hints.len() {
Some((pc, &self.hints[start..end]))
} else {
None
}
})
});
#[cfg(feature = "extensive_hints")]
let iter = self.hints_ranges.iter().filter_map(|(pc, (start, len))| {
let end = start + len.get();
if end <= self.hints.len() {
Some((pc.offset, &self.hints[*start..end]))
} else {
None
}
})
});
iter
}
}

Expand Down Expand Up @@ -479,10 +523,11 @@ mod tests {
program.shared_program_data.hints_collection.hints,
Vec::new()
);
assert_eq!(
program.shared_program_data.hints_collection.hints_ranges,
HashMap::new()
);
assert!(program
.shared_program_data
.hints_collection
.hints_ranges
.is_empty());
}

#[test]
Expand Down Expand Up @@ -525,10 +570,11 @@ mod tests {
program.shared_program_data.hints_collection.hints,
Vec::new()
);
assert_eq!(
program.shared_program_data.hints_collection.hints_ranges,
HashMap::new()
);
assert!(program
.shared_program_data
.hints_collection
.hints_ranges
.is_empty());
}

#[test]
Expand Down Expand Up @@ -583,6 +629,22 @@ mod tests {
assert_eq!(program.shared_program_data.main, None);
assert_eq!(program.shared_program_data.identifiers, HashMap::new());

#[cfg(not(feature = "extensive_hints"))]
let program_hints: HashMap<_, _> = program
.shared_program_data
.hints_collection
.hints_ranges
.iter()
.enumerate()
.filter_map(|(pc, r)| r.map(|(s, l)| (pc, (s, s + l.get()))))
.map(|(pc, (s, e))| {
(
pc,
program.shared_program_data.hints_collection.hints[s..e].to_vec(),
)
})
.collect();
#[cfg(feature = "extensive_hints")]
let program_hints: HashMap<_, _> = program
.shared_program_data
.hints_collection
Expand Down Expand Up @@ -1236,7 +1298,7 @@ mod tests {
fn default_program() {
let hints_collection = HintsCollection {
hints: Vec::new(),
hints_ranges: HashMap::new(),
hints_ranges: Default::default(),
};

let shared_program_data = SharedProgramData {
Expand Down
33 changes: 33 additions & 0 deletions vm/src/vm/runners/cairo_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,11 @@ impl CairoRunner {
hint_processor: &mut dyn HintProcessor,
) -> Result<(), VirtualMachineError> {
let references = &self.program.shared_program_data.reference_manager;
#[cfg(not(feature = "extensive_hints"))]
let hint_data = self.get_hint_data(references, hint_processor)?;
#[cfg(feature = "extensive_hints")]
let mut hint_data = self.get_hint_data(references, hint_processor)?;
#[cfg(feature = "extensive_hints")]
let mut hint_ranges = self
.program
.shared_program_data
Expand All @@ -566,7 +570,18 @@ impl CairoRunner {
vm.step(
hint_processor,
&mut self.exec_scopes,
#[cfg(feature = "extensive_hints")]
&mut hint_data,
#[cfg(not(feature = "extensive_hints"))]
self.program
.shared_program_data
.hints_collection
.get_hint_range_for_pc(vm.run_context.pc.offset)
.and_then(|range| {
range.and_then(|(start, length)| hint_data.get(start..start + length.get()))
})
.unwrap_or(&[]),
#[cfg(feature = "extensive_hints")]
&mut hint_ranges,
&self.program.constants,
)?;
Expand All @@ -588,13 +603,27 @@ impl CairoRunner {
hint_processor: &mut dyn HintProcessor,
) -> Result<(), VirtualMachineError> {
let references = &self.program.shared_program_data.reference_manager;
#[cfg(not(feature = "extensive_hints"))]
let hint_data = self.get_hint_data(references, hint_processor)?;
#[cfg(feature = "extensive_hints")]
let mut hint_data = self.get_hint_data(references, hint_processor)?;
#[cfg(feature = "extensive_hints")]
let mut hint_ranges = self
.program
.shared_program_data
.hints_collection
.hints_ranges
.clone();
#[cfg(not(feature = "extensive_hints"))]
let hint_data = &self
.program
.shared_program_data
.hints_collection
.get_hint_range_for_pc(vm.run_context.pc.offset)
.and_then(|range| {
range.and_then(|(start, length)| hint_data.get(start..start + length.get()))
})
.unwrap_or(&[]);

for remaining_steps in (1..=steps).rev() {
if self.final_pc.as_ref() == Some(&vm.run_context.pc) {
Expand All @@ -604,7 +633,11 @@ impl CairoRunner {
vm.step(
hint_processor,
&mut self.exec_scopes,
#[cfg(feature = "extensive_hints")]
&mut hint_data,
#[cfg(not(feature = "extensive_hints"))]
hint_data,
#[cfg(feature = "extensive_hints")]
&mut hint_ranges,
&self.program.constants,
)?;
Expand Down
Loading

0 comments on commit 8a2ef24

Please sign in to comment.