Skip to content
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.

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

Add try_push_uninit and test #278

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@
# Generated by Cargo
/Cargo.lock
/target
.idea

16 changes: 16 additions & 0 deletions src/arrayvec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,22 @@ impl<T, const CAP: usize> ArrayVec<T, CAP> {
ArrayVecImpl::try_push(self, element)
}

/// Push a new uninitialised value to the end of the vector and return
/// a mutable reference to it.
///
/// This is useful if the backed value is large and won't fit on stack.
///
/// Capacity error carries the data - since we don't have data then we need a different error type.
Copy link
Owner

Choose a reason for hiding this comment

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

please use CapacityError<()>

pub unsafe fn try_push_uninit(&mut self) -> Result<*mut T, ()> {
Copy link
Owner

Choose a reason for hiding this comment

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

Let's use a NonNull<T> for the pointer. While we think of the best shape of API for this.

let len = self.len();
if len >= Self::CAPACITY {
return Err(())
}
let new_ptr = self.as_mut_ptr().add(len);
self.set_len(len + 1);
Ok(new_ptr)
}

/// Push `element` to the end of the vector without checking the capacity.
///
/// It is up to the caller to ensure the capacity of the vector is
Expand Down
49 changes: 48 additions & 1 deletion tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::mem;
use arrayvec::CapacityError;

use std::collections::HashMap;

use std::sync::RwLock;

#[test]
fn test_simple() {
Expand Down Expand Up @@ -777,3 +777,50 @@ fn test_arraystring_zero_filled_has_some_sanity_checks() {
assert_eq!(string.as_str(), "\0\0\0\0");
assert_eq!(string.len(), 4);
}

/// 3 MB struct
struct ReallyBigStruct {
pub(crate) field_one: [u8; 1_000_000],
pub(crate) field_two: [u8; 1_000_000],
pub(crate) field_three: [u8; 1_000_000],
}

/// Const initialised struct outside of stack
/// We need to initialise this outside of the stack, since otherwise there is a memory copy from
/// the stack into the heap.
/// With a static initialisation, we do not have a stack copy.
static large_struct: RwLock<ArrayVec<ReallyBigStruct, 100>> = RwLock::new(ArrayVec::new_const());

#[test]
fn test_push_uninit() {
let mut lock = large_struct.write().unwrap();
let ptr = unsafe { lock.try_push_uninit().unwrap() };
let field_one = unsafe {&mut (*ptr).field_one};
*field_one = [1; 1_000_000];
let field_two = unsafe {&mut (*ptr).field_two};
*field_two = [2; 1_000_000];
let field_three = unsafe {&mut (*ptr).field_three};
*field_three = [3; 1_000_000];
Copy link
Owner

@bluss bluss Oct 16, 2024

Choose a reason for hiding this comment

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

Raw pointer is not a good interface because the user needs to remember to use field_three.write(...). Assigning with *field_three = ... only works for Copy types. The point is that the assignment first drops the existing value before copying (nominally) the new value into its place. MaybeUninit is a good place to look.


assert_eq!(lock.len(), 1);
assert_eq!(lock[0].field_one[3], 1);
assert_eq!(lock[0].field_two[999], 2);
assert_eq!(lock[0].field_three[999_999], 3);

// Push a second value
let ptr = unsafe { lock.try_push_uninit().unwrap() };
let field_one = unsafe {&mut (*ptr).field_one};
*field_one = [4; 1_000_000];
let field_two = unsafe {&mut (*ptr).field_two};
*field_two = [5; 1_000_000];
let field_three = unsafe {&mut (*ptr).field_three};
*field_three = [6; 1_000_000];

assert_eq!(lock.len(), 2);
assert_eq!(lock[0].field_one[3], 1);
assert_eq!(lock[0].field_two[999], 2);
assert_eq!(lock[0].field_three[999_999], 3);
assert_eq!(lock[1].field_one[3], 4);
assert_eq!(lock[1].field_two[999], 5);
assert_eq!(lock[1].field_three[999_999], 6);
}
Loading