From 1b4d1d55924760e92d95b6353acaa2f1aaf16400 Mon Sep 17 00:00:00 2001 From: Edgar Date: Fri, 15 Dec 2023 18:45:31 +0100 Subject: [PATCH] Criterion Benchmark Libfuncs and more (#385) * bench libfuncs * bench cairo vm too * trigger * trigger * allow unused which is used * fix unique id issue * fix box and nullable from_jit * fixes * clippy --- Cargo.lock | 1 + Cargo.toml | 5 ++ benches/compile_time.rs | 18 +++---- benches/libfuncs.rs | 100 +++++++++++++++++++++++++++++++++++++++ benches/util.rs | 30 ++++++++---- src/libfuncs/box.rs | 18 +++++++ src/libfuncs/gas.rs | 28 ++++++++++- src/libfuncs/nullable.rs | 25 +++++++++- src/values.rs | 31 ++++++++++-- tests/cases.rs | 4 +- tests/common.rs | 23 ++++++++- 11 files changed, 252 insertions(+), 31 deletions(-) create mode 100644 benches/libfuncs.rs diff --git a/Cargo.lock b/Cargo.lock index 251c58956..24fc109ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -886,6 +886,7 @@ dependencies = [ "thiserror", "tracing", "tracing-subscriber", + "walkdir", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 2760e33bc..a4ef281af 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,6 +71,7 @@ pretty_assertions_sorted = "1.2.3" proptest = "1.2" tempfile = "3.6" test-case = "3.2.1" +walkdir = "2" [build-dependencies] cc = "1.0" @@ -95,5 +96,9 @@ opt-level = 1 name = "compile_time" harness = false +[[bench]] +name = "libfuncs" +harness = false + [workspace] members = ["runtime"] diff --git a/benches/compile_time.rs b/benches/compile_time.rs index eb820f3fc..bbdfc5955 100644 --- a/benches/compile_time.rs +++ b/benches/compile_time.rs @@ -5,12 +5,12 @@ use util::prepare_programs; mod util; pub fn bench_compile_time(c: &mut Criterion) { + let programs = prepare_programs("programs/compile_benches"); + { let mut c = c.benchmark_group("Compilation With Independent Context"); - let programs = prepare_programs(); - - for (program, filename) in programs { + for (program, filename) in &programs { c.bench_with_input(BenchmarkId::new(filename, 1), &program, |b, program| { b.iter(|| { let native_context = NativeContext::new(); @@ -24,11 +24,9 @@ pub fn bench_compile_time(c: &mut Criterion) { { let mut c = c.benchmark_group("Compilation With Shared Context"); - let programs = prepare_programs(); - let native_context = NativeContext::new(); - for (program, filename) in programs { + for (program, filename) in &programs { c.bench_with_input(BenchmarkId::new(filename, 1), &program, |b, program| { b.iter(|| { native_context.compile(program).unwrap(); @@ -41,9 +39,7 @@ pub fn bench_compile_time(c: &mut Criterion) { { let mut c = c.benchmark_group("Compilation With Independent Context To Object Code"); - let programs = prepare_programs(); - - for (program, filename) in programs { + for (program, filename) in &programs { c.bench_with_input(BenchmarkId::new(filename, 1), &program, |b, program| { b.iter(|| { let native_context = NativeContext::new(); @@ -59,11 +55,9 @@ pub fn bench_compile_time(c: &mut Criterion) { { let mut c = c.benchmark_group("Compilation With Shared Context To Object Code"); - let programs = prepare_programs(); - let native_context = NativeContext::new(); - for (program, filename) in programs { + for (program, filename) in &programs { c.bench_with_input(BenchmarkId::new(filename, 1), &program, |b, program| { b.iter(|| { let module = native_context.compile(black_box(program)).unwrap(); diff --git a/benches/libfuncs.rs b/benches/libfuncs.rs new file mode 100644 index 000000000..ffab14410 --- /dev/null +++ b/benches/libfuncs.rs @@ -0,0 +1,100 @@ +use cairo_lang_runner::StarknetState; +use cairo_native::{context::NativeContext, executor::NativeExecutor}; +use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion}; +use util::{create_vm_runner, prepare_programs}; + +mod util; + +pub fn bench_libfuncs(c: &mut Criterion) { + let programs = prepare_programs("tests/cases"); + + { + let mut c = c.benchmark_group("Libfunc Execution Time"); + + for (program, filename) in &programs { + if filename == "tests/cases/felt_ops/div.cairo" { + continue; // todo: enable when libfuncs felt252_div and felt252_div_const are implemented + } + + let entry = program + .funcs + .iter() + .find(|f| { + if let Some(name) = &f.id.debug_name { + name.ends_with("main") + } else { + false + } + }) + .expect("failed to find entry point"); + + let vm_runner = create_vm_runner(program); + + c.bench_with_input( + BenchmarkId::new(filename, "SierraCasmRunner"), + &program, + |b, _program| { + b.iter(|| { + let res = vm_runner + .run_function_with_starknet_context( + entry, + &[], + Some(usize::MAX), + StarknetState::default(), + ) + .expect("should run correctly"); + black_box(res) + }) + }, + ); + + c.bench_with_input( + BenchmarkId::new(filename, "jit-cold"), + &program, + |b, program| { + let native_context = NativeContext::new(); + b.iter(|| { + let module = native_context.compile(program).unwrap(); + // pass manager internally verifies the MLIR output is correct. + let native_executor = NativeExecutor::new(module); + + // Execute the program. + let result = native_executor + .execute(&entry.id, &[], Some(u64::MAX as u128)) + .unwrap(); + black_box(result) + }) + }, + ); + + c.bench_with_input( + BenchmarkId::new(filename, "jit-hot"), + program, + |b, program| { + let native_context = NativeContext::new(); + let module = native_context.compile(program).unwrap(); + // pass manager internally verifies the MLIR output is correct. + let native_executor = NativeExecutor::new(module); + + // warmup + for _ in 0..5 { + native_executor + .execute(&entry.id, &[], Some(u64::MAX as u128)) + .unwrap(); + } + + b.iter(|| { + // Execute the program. + let result = native_executor + .execute(&entry.id, &[], Some(u64::MAX as u128)) + .unwrap(); + black_box(result) + }) + }, + ); + } + } +} + +criterion_group!(benches, bench_libfuncs); +criterion_main!(benches); diff --git a/benches/util.rs b/benches/util.rs index 3f9f0d51b..decb2d0aa 100644 --- a/benches/util.rs +++ b/benches/util.rs @@ -1,22 +1,32 @@ -use std::{path::Path, sync::Arc}; +use std::sync::Arc; +use cairo_lang_runner::SierraCasmRunner; use cairo_lang_sierra::program::Program; +use walkdir::WalkDir; -pub fn prepare_programs() -> impl Iterator, String)> { - let programs = Path::new("programs/compile_benches") - .read_dir() - .unwrap() +pub fn prepare_programs(path: &str) -> Vec<(Arc, String)> { + WalkDir::new(path) + .into_iter() .filter_map(|entry| { - let path = entry.unwrap().path(); + let e = entry.unwrap(); + let path = e.path(); match path.extension().map(|x| x.to_str().unwrap()) { Some("cairo") => Some(( - cairo_native::utils::cairo_to_sierra(&path), - path.file_name().unwrap().to_str().unwrap().to_string(), + cairo_native::utils::cairo_to_sierra(path), + path.display().to_string(), )), _ => None, } }) - .collect::>(); // collect so iter is not lazy evaluated on bench + .collect::>() +} - programs.into_iter() +#[allow(unused)] // its used but clippy doesn't detect it well +pub fn create_vm_runner(program: &Program) -> SierraCasmRunner { + SierraCasmRunner::new( + program.clone(), + Some(Default::default()), + Default::default(), + ) + .unwrap() } diff --git a/src/libfuncs/box.rs b/src/libfuncs/box.rs index 9ccfc07b5..dc147a95b 100644 --- a/src/libfuncs/box.rs +++ b/src/libfuncs/box.rs @@ -1,4 +1,6 @@ //! # Box libfuncs +//! +//! A heap allocated value, its internally a pointer that can't be null. use super::{LibfuncBuilder, LibfuncHelper}; use crate::{ @@ -166,4 +168,20 @@ mod test { run_program_assert_output(&program, "run_test", &[], &[JITValue::Uint32(2)]); } + + #[test] + fn run_box() { + let program = load_cairo!( + use box::BoxTrait; + use box::BoxImpl; + + fn run_test() -> Box { + let x: u32 = 2_u32; + let box_x: Box = BoxTrait::new(x); + box_x + } + ); + + run_program_assert_output(&program, "run_test", &[], &[JITValue::Uint32(2)]); + } } diff --git a/src/libfuncs/gas.rs b/src/libfuncs/gas.rs index a442130e6..5cb505e0f 100644 --- a/src/libfuncs/gas.rs +++ b/src/libfuncs/gas.rs @@ -49,7 +49,9 @@ where build_withdraw_gas(context, registry, entry, location, helper, metadata, info) } GasConcreteLibfunc::RedepositGas(_) => todo!(), - GasConcreteLibfunc::GetAvailableGas(_) => todo!(), + GasConcreteLibfunc::GetAvailableGas(info) => { + build_get_available_gas(context, registry, entry, location, helper, metadata, info) + } GasConcreteLibfunc::BuiltinWithdrawGas(info) => { build_builtin_withdraw_gas(context, registry, entry, location, helper, metadata, info) } @@ -59,6 +61,30 @@ where } } +/// Generate MLIR operations for the `get_builtin_costs` libfunc. +pub fn build_get_available_gas<'ctx, 'this, TType, TLibfunc>( + _context: &'ctx Context, + _registry: &ProgramRegistry, + entry: &'this Block<'ctx>, + location: Location<'ctx>, + helper: &LibfuncHelper<'ctx, 'this>, + _metadata: &mut MetadataStorage, + _info: &SignatureOnlyConcreteLibfunc, +) -> Result<()> +where + TType: GenericType, + TLibfunc: GenericLibfunc, + ::Concrete: TypeBuilder, + ::Concrete: LibfuncBuilder, +{ + entry.append_operation(helper.br( + 0, + &[entry.argument(0)?.into(), entry.argument(0)?.into()], + location, + )); + Ok(()) +} + /// Generate MLIR operations for the `withdraw_gas` libfunc. pub fn build_withdraw_gas<'ctx, 'this, TType, TLibfunc>( context: &'ctx Context, diff --git a/src/libfuncs/nullable.rs b/src/libfuncs/nullable.rs index e2a1d8a29..b72e856c3 100644 --- a/src/libfuncs/nullable.rs +++ b/src/libfuncs/nullable.rs @@ -1,4 +1,6 @@ //! # Nullable libfuncs +//! +//! Like a Box but it can be null. use super::{LibfuncBuilder, LibfuncHelper}; use crate::{ @@ -170,7 +172,10 @@ where #[cfg(test)] mod test { - use crate::utils::test::{jit_struct, load_cairo, run_program_assert_output}; + use crate::{ + utils::test::{jit_struct, load_cairo, run_program_assert_output}, + values::JITValue, + }; #[test] fn run_null() { @@ -189,6 +194,24 @@ mod test { run_program_assert_output(&program, "run_test", &[], &[jit_struct!()]); } + #[test] + fn run_null_jit() { + let program = load_cairo!( + use nullable::null; + use nullable::match_nullable; + use nullable::FromNullableResult; + use nullable::nullable_from_box; + use box::BoxTrait; + + fn run_test() -> Nullable { + let a: Nullable = null(); + a + } + ); + + run_program_assert_output(&program, "run_test", &[], &[JITValue::Null]); + } + #[test] fn run_not_null() { let program = load_cairo!( diff --git a/src/values.rs b/src/values.rs index c8d34699b..bcc967e33 100644 --- a/src/values.rs +++ b/src/values.rs @@ -29,6 +29,8 @@ use crate::{ /// They map to the cairo/sierra types. /// /// The debug_name field on some variants is `Some` when receiving a [`JITValue`] as a result. +/// +/// A Boxed value or a non-null Nullable value is returned with it's inner value. #[derive(Educe, Debug, Clone)] #[educe(PartialEq, Eq)] pub enum JITValue { @@ -63,6 +65,8 @@ pub enum JITValue { Sint128(i128), EcPoint(Felt, Felt), EcState(Felt, Felt, Felt, Felt), + /// Used as return value for Nullables that are null. + Null, } // Conversions @@ -474,6 +478,9 @@ impl JITValue { ptr } + JITValue::Null => { + unimplemented!("null is meant as return value for nullable for now") + } } }) } @@ -521,7 +528,12 @@ impl JITValue { Self::Array(array_value) } - CoreTypeConcrete::Box(info) => JITValue::from_jit(ptr, &info.ty, registry), + CoreTypeConcrete::Box(info) => { + let inner = *ptr.cast::>().as_ptr(); + let value = JITValue::from_jit(inner, &info.ty, registry); + libc::free(inner.as_ptr().cast()); + value + } CoreTypeConcrete::EcPoint(_) => { let data = ptr.cast::<[[u32; 8]; 2]>().as_ref(); @@ -554,7 +566,20 @@ impl JITValue { CoreTypeConcrete::Sint64(_) => JITValue::Sint64(*ptr.cast::().as_ref()), CoreTypeConcrete::Sint128(_) => JITValue::Sint128(*ptr.cast::().as_ref()), CoreTypeConcrete::NonZero(info) => JITValue::from_jit(ptr, &info.ty, registry), - CoreTypeConcrete::Nullable(_) => todo!(), + CoreTypeConcrete::Nullable(info) => { + let inner_ptr = *ptr.cast::<*mut ()>().as_ptr(); + if inner_ptr.is_null() { + JITValue::Null + } else { + let value = JITValue::from_jit( + NonNull::new_unchecked(inner_ptr).cast(), + &info.ty, + registry, + ); + libc::free(inner_ptr.cast()); + value + } + } CoreTypeConcrete::Uninitialized(_) => todo!(), CoreTypeConcrete::Enum(info) => { let tag_layout = crate::utils::get_integer_layout(match info.variants.len() { @@ -708,7 +733,7 @@ impl ValueBuilder for CoreTypeConcrete { match self { CoreTypeConcrete::Array(_) => true, CoreTypeConcrete::Bitwise(_) => false, - CoreTypeConcrete::Box(_) => todo!(), + CoreTypeConcrete::Box(_) => false, CoreTypeConcrete::EcOp(_) => false, CoreTypeConcrete::EcPoint(_) => true, CoreTypeConcrete::EcState(_) => true, diff --git a/tests/cases.rs b/tests/cases.rs index 1eabb8812..da1f0ef86 100644 --- a/tests/cases.rs +++ b/tests/cases.rs @@ -72,7 +72,7 @@ mod common; #[test_case("tests/cases/structs/nested.cairo")] #[test_case("tests/cases/structs/struct_snapshot_deconstruct.cairo")] // gas -#[test_case("tests/cases/gas/available_gas.cairo" => ignore["unimplemented"])] +#[test_case("tests/cases/gas/available_gas.cairo")] // bool #[test_case("tests/cases/bool/and.cairo")] #[test_case("tests/cases/bool/eq.cairo")] @@ -91,7 +91,7 @@ mod common; #[test_case("tests/cases/array/pop_front_valid.cairo")] #[test_case("tests/cases/array/slice.cairo")] // nullable -#[test_case("tests/cases/nullable/test_nullable.cairo" => ignore["unimplemented"])] +#[test_case("tests/cases/nullable/test_nullable.cairo")] fn test_cases(program_path: &str) { compare_inputless_program(program_path) } diff --git a/tests/common.rs b/tests/common.rs index 5f16bf472..07a483f76 100644 --- a/tests/common.rs +++ b/tests/common.rs @@ -436,7 +436,7 @@ pub fn compare_outputs( } else { panic!("invalid type") }; - prop_assert_eq!(vm_value, native_value) + prop_assert_eq!(vm_value, native_value, "mismatch on u8 value") } CoreTypeConcrete::Uint16(_) => { prop_assert!(vm_rets.peek().is_some(), "cairo-vm missing next value"); @@ -504,7 +504,26 @@ pub fn compare_outputs( reg, )?; } - CoreTypeConcrete::Nullable(_) => todo!(), + CoreTypeConcrete::Nullable(info) => { + prop_assert!(native_rets.peek().is_some()); + + // check for null + if vm_rets.peek().unwrap().as_str() == "0" { + vm_rets.next(); + prop_assert_eq!( + native_rets.next().expect("cairo-native missing next value"), + &JITValue::Null, + "native should return null" + ); + } else { + check_next_type( + reg.get_type(&info.ty).expect("type should exist"), + &mut [native_rets.next().unwrap()].into_iter(), + vm_rets, + reg, + )?; + } + } CoreTypeConcrete::RangeCheck(_) => { // runner: ignore // native: null