diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 178d7b52ba..a5ff5b1350 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 0dad49eb86..dca54a60b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) @@ -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`: diff --git a/Makefile b/Makefile index 8925012f37..168be69660 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/vm/Cargo.toml b/vm/Cargo.toml index ed3e45a2df..9d42795907 100644 --- a/vm/Cargo.toml +++ b/vm/Cargo.toml @@ -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 = [ diff --git a/vm/src/hint_processor/hint_processor_definition.rs b/vm/src/hint_processor/hint_processor_definition.rs index 7ab8bd7973..6b9b892574 100644 --- a/vm/src/hint_processor/hint_processor_definition.rs +++ b/vm/src/hint_processor/hint_processor_definition.rs @@ -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, @@ -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, diff --git a/vm/src/tests/run_deprecated_contract_class_simplified.rs b/vm/src/tests/run_deprecated_contract_class_simplified.rs index 7b46ae1191..c8717aed13 100644 --- a/vm/src/tests/run_deprecated_contract_class_simplified.rs +++ b/vm/src/tests/run_deprecated_contract_class_simplified.rs @@ -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: diff --git a/vm/src/types/program.rs b/vm/src/types/program.rs index b6ccf89a11..d5bc7f86ef 100644 --- a/vm/src/types/program.rs +++ b/vm/src/types/program.rs @@ -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 @@ -108,6 +108,9 @@ impl<'a> Arbitrary<'a> for SharedProgramData { pub(crate) struct HintsCollection { hints: Vec, /// 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, + #[cfg(feature = "extensive_hints")] pub(crate) hints_ranges: HashMap, } @@ -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(), }); }; @@ -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[..]); } @@ -153,11 +163,24 @@ impl HintsCollection { pub fn iter_hints(&self) -> impl Iterator { self.hints.iter() } + + #[cfg(not(feature = "extensive_hints"))] + pub fn get_hint_range_for_pc(&self, pc: usize) -> Option { + self.hints_ranges.get(pc).cloned() + } } impl From<&HintsCollection> for BTreeMap> { 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()); } @@ -166,6 +189,10 @@ impl From<&HintsCollection> for BTreeMap> { } /// 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))] @@ -418,14 +445,31 @@ impl TryFrom for Program { #[cfg(test)] impl HintsCollection { pub fn iter(&self) -> impl Iterator { - 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 } } @@ -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] @@ -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] @@ -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 @@ -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 { diff --git a/vm/src/vm/runners/cairo_runner.rs b/vm/src/vm/runners/cairo_runner.rs index fdbdeae0ac..c934809952 100644 --- a/vm/src/vm/runners/cairo_runner.rs +++ b/vm/src/vm/runners/cairo_runner.rs @@ -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 @@ -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, )?; @@ -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) { @@ -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, )?; diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index 8e18713eb1..2f098adfeb 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1,5 +1,5 @@ use crate::stdlib::{any::Any, borrow::Cow, collections::HashMap, prelude::*}; - +#[cfg(feature = "extensive_hints")] use crate::types::program::HintRange; use crate::{ hint_processor::hint_processor_definition::HintProcessor, @@ -25,6 +25,7 @@ use crate::{ }; use core::cmp::Ordering; +#[cfg(feature = "extensive_hints")] use core::num::NonZeroUsize; use felt::Felt252; use num_traits::{ToPrimitive, Zero}; @@ -440,6 +441,23 @@ impl VirtualMachine { decode_instruction(instruction) } + #[cfg(not(feature = "extensive_hints"))] + pub fn step_hint( + &mut self, + hint_processor: &mut dyn HintProcessor, + exec_scopes: &mut ExecutionScopes, + hint_datas: &[Box], + constants: &HashMap, + ) -> Result<(), VirtualMachineError> { + for (hint_index, hint_data) in hint_datas.iter().enumerate() { + hint_processor + .execute_hint(self, exec_scopes, hint_data, constants) + .map_err(|err| VirtualMachineError::Hint(Box::new((hint_index, err))))? + } + Ok(()) + } + + #[cfg(feature = "extensive_hints")] pub fn step_hint( &mut self, hint_processor: &mut dyn HintProcessor, @@ -517,14 +535,16 @@ impl VirtualMachine { &mut self, hint_processor: &mut dyn HintProcessor, exec_scopes: &mut ExecutionScopes, - hint_datas: &mut Vec>, - hint_ranges: &mut HashMap, + #[cfg(feature = "extensive_hints")] hint_datas: &mut Vec>, + #[cfg(not(feature = "extensive_hints"))] hint_datas: &[Box], + #[cfg(feature = "extensive_hints")] hint_ranges: &mut HashMap, constants: &HashMap, ) -> Result<(), VirtualMachineError> { self.step_hint( hint_processor, exec_scopes, hint_datas, + #[cfg(feature = "extensive_hints")] hint_ranges, constants, )?; @@ -2692,6 +2712,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new(), ), @@ -2922,6 +2943,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new(), ), @@ -3005,6 +3027,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ), @@ -3108,6 +3131,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ), @@ -3130,6 +3154,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ), @@ -3153,6 +3178,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ), @@ -3673,7 +3699,7 @@ mod tests { #[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)] fn test_step_for_preset_memory_with_alloc_hint() { let mut vm = vm!(true); - let mut hint_data = vec![any_box!(HintProcessorData::new_default( + let hint_data = vec![any_box!(HintProcessorData::new_default( "memory[ap] = segments.add()".to_string(), HashMap::new(), ))]; @@ -3709,13 +3735,23 @@ mod tests { ((1, 1), (3, 0)) ]; + #[cfg(feature = "extensive_hints")] + let mut hint_data = hint_data; + //Run Steps for _ in 0..6 { + #[cfg(not(feature = "extensive_hints"))] + let mut hint_data = if vm.run_context.pc == (0, 0).into() { + &hint_data[0..] + } else { + &hint_data[0..0] + }; assert_matches!( vm.step( &mut hint_processor, exec_scopes_ref!(), &mut hint_data, + #[cfg(feature = "extensive_hints")] &mut HashMap::from([( Relocatable::from((0, 0)), (0_usize, NonZeroUsize::new(1).unwrap()) @@ -4354,6 +4390,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ), @@ -4440,6 +4477,7 @@ mod tests { &mut hint_processor, exec_scopes_ref!(), &mut Vec::new(), + #[cfg(feature = "extensive_hints")] &mut HashMap::new(), &HashMap::new() ),