Skip to content

Refactor arrays to avoid clones according to the cost model. #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Feb 22, 2025
Merged

Refactor arrays to avoid clones according to the cost model. #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2cb7501
Partial array refactor.
azteca1998 Feb 13, 2025
aa2b36e
More progress.
azteca1998 Feb 14, 2025
fbe1935
Implement `array_pop_*`.
azteca1998 Feb 14, 2025
11070f1
Some fixes.
azteca1998 Feb 14, 2025
b05dbae
Implement `array_slice`.
azteca1998 Feb 14, 2025
0ff06e4
Fix contract executor.
azteca1998 Feb 14, 2025
966bcd3
Fix other bugs.
azteca1998 Feb 14, 2025
207ff9e
Fix AOT contract runner.
azteca1998 Feb 14, 2025
ac87223
Fix array from_ptr drops.
azteca1998 Feb 14, 2025
749f5c0
Fix starknet module.
azteca1998 Feb 14, 2025
14c3777
Fix stuff.
azteca1998 Feb 18, 2025
e212ad8
Fix bug.
azteca1998 Feb 18, 2025
5505c9c
Restore makefile.
azteca1998 Feb 18, 2025
610ba01
Merge branch 'main' into refactor-array-cows
edg-l Feb 20, 2025
ff67e7a
Fix libfunc name mistype.
azteca1998 Feb 21, 2025
86d7968
Merge branch 'main' into refactor-array-cows
gabrielbosio Feb 21, 2025
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
139 changes: 66 additions & 73 deletions src/executor/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ use std::{
fs::{self, File},
io,
path::{Path, PathBuf},
ptr::NonNull,
ptr::{self, NonNull},
sync::Arc,
};
use tempfile::NamedTempFile;
Expand Down Expand Up @@ -375,52 +375,59 @@ impl AotContractExecutor {
}

let felt_layout = get_integer_layout(252).pad_to_align();
let refcount_offset = get_integer_layout(32)
.align_to(felt_layout.align())
.unwrap()
.pad_to_align()
.size();
let refcount_offset = crate::types::array::calc_data_prefix_offset(felt_layout);

let ptr = match args.len() {
let len_u32: u32 = args
.len()
.try_into()
.to_native_assert_error("number of arguments should fit into a u32")?;
let array_ptr = match args.len() {
0 => std::ptr::null_mut(),
_ => unsafe {
let ptr: *mut () =
let array_ptr: *mut () =
libc_malloc(felt_layout.size() * args.len() + refcount_offset).cast();

// Write reference count.
ptr.cast::<u32>().write(1);
ptr.byte_add(refcount_offset)
array_ptr.cast::<(u32, u32)>().write((1, len_u32));
array_ptr.byte_add(refcount_offset)
},
};
let len: u32 = args
.len()
.try_into()
.to_native_assert_error("number of arguments should fit into a u32")?;

ptr.to_bytes(&mut invoke_data, |_| unreachable!())?;
if cfg!(target_arch = "aarch64") {
0u32.to_bytes(&mut invoke_data, |_| unreachable!())?; // start
len.to_bytes(&mut invoke_data, |_| unreachable!())?; // end
len.to_bytes(&mut invoke_data, |_| unreachable!())?; // cap
} else if cfg!(target_arch = "x86_64") {
(0u32 as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // start
(len as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // end
(len as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // cap
} else {
unreachable!("unsupported architecture");
}

for (idx, elem) in args.iter().enumerate() {
let f = elem.to_bytes_le();
unsafe {
std::ptr::copy_nonoverlapping(
f.as_ptr().cast::<u8>(),
ptr.byte_add(idx * felt_layout.size()).cast::<u8>(),
array_ptr.byte_add(idx * felt_layout.size()).cast::<u8>(),
felt_layout.size(),
)
};
}

// Make double pointer.
let array_ptr_ptr = if array_ptr.is_null() {
ptr::null_mut()
} else {
unsafe {
let array_ptr_ptr = libc_malloc(size_of::<*mut ()>()).cast::<*mut ()>();
array_ptr_ptr.write(array_ptr);
array_ptr_ptr
}
};

array_ptr_ptr.to_bytes(&mut invoke_data, |_| unreachable!())?;
if cfg!(target_arch = "aarch64") {
0u32.to_bytes(&mut invoke_data, |_| unreachable!())?; // start
len_u32.to_bytes(&mut invoke_data, |_| unreachable!())?; // end
len_u32.to_bytes(&mut invoke_data, |_| unreachable!())?; // cap
} else if cfg!(target_arch = "x86_64") {
(0u32 as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // start
(len_u32 as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // end
(len_u32 as u64).to_bytes(&mut invoke_data, |_| unreachable!())?; // cap
} else {
unreachable!("unsupported architecture");
}

// Pad invoke data to the 16 byte boundary avoid segfaults.
#[cfg(target_arch = "aarch64")]
const REGISTER_BYTES: usize = 64;
Expand Down Expand Up @@ -520,62 +527,48 @@ impl AotContractExecutor {
let tag = *unsafe { enum_ptr.cast::<u8>().as_ref() } as usize;
let tag = tag & 0x01; // Filter out bits that are not part of the enum's tag.

// layout of both enum variants, both are a array of felts
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment no longer valid? If it is, could you add it back?

let value_layout = unsafe { Layout::from_size_align_unchecked(24, 8) };
let value_ptr = unsafe {
enum_ptr
.cast::<u8>()
.add(tag_layout.extend(value_layout)?.1)
};
let mut value_ptr = unsafe { enum_ptr.byte_add(tag_layout.extend(value_layout)?.1).cast() };

let value_ptr = &mut value_ptr.cast();
let array_ptr_ptr = unsafe { *read_value::<*mut NonNull<()>>(&mut value_ptr) };
let array_start = unsafe { *read_value::<u32>(&mut value_ptr) };
let array_end = unsafe { *read_value::<u32>(&mut value_ptr) };
let _array_capacity = unsafe { *read_value::<u32>(&mut value_ptr) };

let array_ptr: *mut u8 = unsafe { *read_value(value_ptr) };
let start: u32 = unsafe { *read_value(value_ptr) };
let end: u32 = unsafe { *read_value(value_ptr) };
let _cap: u32 = unsafe { *read_value(value_ptr) };
let mut array_value = Vec::with_capacity((array_end - array_start) as usize);
if !array_ptr_ptr.is_null() {
let array_ptr = unsafe { array_ptr_ptr.read() };

let elem_stride = felt_layout.pad_to_align().size();
let elem_stride = felt_layout.pad_to_align().size();
for i in array_start..array_end {
let cur_elem_ptr = unsafe { array_ptr.byte_add(elem_stride * i as usize) };

// this pointer can be null if the array has a size of 0.
let data_ptr = unsafe { array_ptr.byte_add(elem_stride * start as usize) };
let mut data = unsafe { cur_elem_ptr.cast::<[u8; 32]>().read() };
data[31] &= 0x0F; // Filter out first 4 bits (they're outside an i252).

assert!(end >= start);
let num_elems = (end - start) as usize;
let mut array_value = Vec::with_capacity(num_elems);

for i in 0..num_elems {
// safe to create a NonNull because if the array has elements, the data_ptr can't be null.
let cur_elem_ptr = NonNull::new(unsafe { data_ptr.byte_add(elem_stride * i) })
.to_native_assert_error("data_ptr should not be null")?;
let data = unsafe { cur_elem_ptr.cast::<[u8; 32]>().as_mut() };
data[31] &= 0x0F; // Filter out first 4 bits (they're outside an i252).
let data = Felt::from_bytes_le_slice(data);

array_value.push(data);
}
array_value.push(Felt::from_bytes_le(&data));
}

if !array_ptr.is_null() {
unsafe {
let ptr = array_ptr.byte_sub(refcount_offset);
assert_eq!(ptr.cast::<u32>().read(), 1);

libc_free(ptr.cast());
let array_ptr = array_ptr.byte_sub(refcount_offset);
assert_eq!(array_ptr.cast::<u32>().read(), 1);
libc_free(array_ptr.as_ptr().cast());
libc_free(array_ptr_ptr.cast());
}
}

let error_msg = if tag != 0 {
let bytes_err: Vec<_> = array_value
.iter()
.flat_map(|felt| felt.to_bytes_be().to_vec())
// remove null chars
.filter(|b| *b != 0)
.collect();
let str_error = decode_error_message(&bytes_err);

Some(str_error)
} else {
None
let error_msg = match tag {
0 => None,
_ => {
Some(decode_error_message(
&array_value
.iter()
.flat_map(|felt| felt.to_bytes_be().to_vec())
// remove null chars
.filter(|b| *b != 0)
.collect::<Vec<_>>(),
))
}
};

// Restore the original builtin costs pointer.
Expand Down
Loading
Loading