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

IArray: avoid heap allocation if there is only one element #38

Merged
merged 6 commits into from
Oct 29, 2023
Merged
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
39 changes: 39 additions & 0 deletions src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub enum IArray<T: ImplicitClone + 'static> {
Static(&'static [T]),
/// A reference counted slice.
Rc(Rc<[T]>),
/// A single element.
Single([T; 1]),
}

// TODO add insta tests
Expand All @@ -21,6 +23,7 @@ impl<T: fmt::Debug + ImplicitClone + 'static> fmt::Debug for IArray<T> {
match self {
Self::Static(a) => a.fmt(f),
Self::Rc(a) => a.fmt(f),
Self::Single(x) => x.fmt(f),
}
}
}
Expand All @@ -30,6 +33,7 @@ impl<T: ImplicitClone + 'static> Clone for IArray<T> {
match self {
Self::Static(a) => Self::Static(a),
Self::Rc(a) => Self::Rc(a.clone()),
Self::Single(x) => Self::Single(x.clone()),
}
}
}
Expand Down Expand Up @@ -73,6 +77,12 @@ impl<T: ImplicitClone + 'static> From<&IArray<T>> for IArray<T> {
}
}

impl<T: ImplicitClone + 'static> From<[T; 1]> for IArray<T> {
fn from(a: [T; 1]) -> IArray<T> {
IArray::Single(a)
}
}

impl<T: ImplicitClone + 'static> IArray<T> {
/// Returns an iterator over the slice.
///
Expand All @@ -93,6 +103,7 @@ impl<T: ImplicitClone + 'static> IArray<T> {
match self {
Self::Static(a) => a.iter().cloned(),
Self::Rc(a) => a.iter().cloned(),
Self::Single(a) => a.iter().cloned(),
}
}

Expand All @@ -111,6 +122,7 @@ impl<T: ImplicitClone + 'static> IArray<T> {
match self {
Self::Static(a) => a.len(),
Self::Rc(a) => a.len(),
Self::Single(_) => 1,
}
}

Expand All @@ -131,6 +143,7 @@ impl<T: ImplicitClone + 'static> IArray<T> {
match self {
Self::Static(a) => a.is_empty(),
Self::Rc(a) => a.is_empty(),
Self::Single(_) => false,
}
}

Expand All @@ -151,6 +164,7 @@ impl<T: ImplicitClone + 'static> IArray<T> {
match self {
Self::Static(a) => a,
Self::Rc(a) => a,
Self::Single(a) => a,
}
}

Expand All @@ -169,6 +183,8 @@ impl<T: ImplicitClone + 'static> IArray<T> {
match self {
Self::Static(a) => a.get(index).cloned(),
Self::Rc(a) => a.get(index).cloned(),
Self::Single(a) if index == 0 => Some(a[0].clone()),
Self::Single(_) => None,
}
}

Expand All @@ -195,12 +211,17 @@ impl<T: ImplicitClone + 'static> IArray<T> {
/// // Static references are immutable
/// let mut v3 = IArray::<u8>::Static(&[1,2,3]);
/// assert!(v3.get_mut().is_none());
///
/// // Single items always return a mutable reference
/// let mut v4 = IArray::<u8>::Single([1]);
/// assert!(v4.get_mut().is_some());
/// ```
#[inline]
pub fn get_mut(&mut self) -> Option<&mut [T]> {
match self {
Self::Rc(ref mut rc) => Rc::get_mut(rc),
Self::Static(_) => None,
Self::Single(ref mut a) => Some(a),
Copy link
Collaborator

Choose a reason for hiding this comment

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

change is not reflected in the doc above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in f80eb14

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think its different semantics from Rc::get_mut, since every clone of single item arrays will now be mutable and different from original, so iarray != iarray.clone()...some mutation via get_mut()... (Rc prevents mutation via get_mut() if there is a clone), but I guess its fine to do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Leaving this discussion as unresolved as I think it's interesting for the future

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess its fine to do that?

no idea xD tbh I wasn't too much in favor of adding mutable API. I was a bit in a doubt about that

}
}

Expand Down Expand Up @@ -244,6 +265,13 @@ impl<T: ImplicitClone + 'static> IArray<T> {
_ => unreachable!(),
}
}
Self::Single(slice) => {
*self = Self::Rc(slice.iter().cloned().collect());
match self {
Self::Rc(rc) => Rc::get_mut(rc).unwrap(),
_ => unreachable!(),
}
}
}
}
}
Expand All @@ -256,6 +284,8 @@ where
match self {
Self::Static(a) => a.eq(other),
Self::Rc(a) => a.eq(*other),
Self::Single(a) if N == 1 => a[0].eq(&other[0]),
Self::Single(_) => false,
}
}
}
Expand All @@ -268,6 +298,8 @@ where
match self {
Self::Static(a) => a.eq(other),
Self::Rc(a) => a.eq(other),
Self::Single(a) if N == 1 => a[0].eq(&other[0]),
Self::Single(_) => false,
}
}
}
Expand All @@ -280,6 +312,7 @@ where
match self {
Self::Static(a) => a.eq(&other),
Self::Rc(a) => a.eq(other),
Self::Single(a) => a.eq(other),
}
}
}
Expand All @@ -292,6 +325,7 @@ where
match self {
Self::Static(a) => a.eq(other),
Self::Rc(a) => a.eq(*other),
Self::Single(a) => a.eq(*other),
}
}
}
Expand Down Expand Up @@ -367,5 +401,10 @@ mod test_array {
fn from() {
let x: IArray<u32> = IArray::Static(&[]);
let _out = IArray::from(&x);

let _array: IArray<u32> = IArray::from(&[1, 2, 3][..]);
let _array: IArray<u32> = IArray::from(vec![1, 2, 3]);
let _array: IArray<u32> = IArray::from(Rc::from(vec![1, 2, 3]));
let _array: IArray<u32> = IArray::from([1]);
}
}
Loading