From c0890dbaed227980fedf0052d944d3e048fb7ecd Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 12 Apr 2019 13:32:43 +0300 Subject: [PATCH 01/25] Implemented `as_contiguous` method --- src/impl_methods.rs | 16 ++++++++++++++++ tests/array.rs | 11 +++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 444d98525..e325bc692 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -43,6 +43,7 @@ use crate::iter::{ Windows }; use crate::stacking::stack; +use crate::OwnedRepr; /// # Methods For All Array Types impl ArrayBase @@ -1225,6 +1226,21 @@ where D::is_contiguous(&self.dim, &self.strides) } + pub fn as_contiguous(&self) -> ArrayBase, D> + where S: Data, + A: Clone + { + if self.is_standard_layout() { + self.to_owned() + } else { + let v = self.iter().map(|x| x.clone()).collect::>(); + unsafe { + // Safe because we use shape and content of existing array here. + ArrayBase::from_shape_vec_unchecked(self.dim(), v) + } + } + } + /// Return a pointer to the first element in the array. /// /// Raw access to array elements needs to follow the strided indexing diff --git a/tests/array.rs b/tests/array.rs index bbe05bbda..492839d8d 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1891,3 +1891,14 @@ fn array_macros() { let empty2: Array = array![[]]; assert_eq!(empty2, array![[]]); } + +#[test] +fn test_as_contiguous() { + let d = Ix2(2, 2); + let shape = d.strides(Ix2(1, 2)); + let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); + assert!(!arr.is_standard_layout()); + + let cont_arr = arr.as_contiguous(); + assert!(cont_arr.is_standard_layout()); +} From 4ea2c7d3552faece3b76b0e406d94aa818226c15 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 12 Apr 2019 14:47:46 +0300 Subject: [PATCH 02/25] Added content check for the new array to the tests --- tests/array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/array.rs b/tests/array.rs index 492839d8d..339c61c90 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1894,11 +1894,11 @@ fn array_macros() { #[test] fn test_as_contiguous() { - let d = Ix2(2, 2); - let shape = d.strides(Ix2(1, 2)); + let shape = Ix2(2, 2).strides(Ix2(1, 2)); let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); assert!(!arr.is_standard_layout()); let cont_arr = arr.as_contiguous(); assert!(cont_arr.is_standard_layout()); + assert!(arr.iter().zip(cont_arr.iter()).all(|(x1, x2)| x1 == x2)); } From 5cdae915b29b528a23f2700ea8bb5be55f655962 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 26 Apr 2019 18:18:19 +0300 Subject: [PATCH 03/25] Return CowArray from as_contiguous instead of Array --- src/data_traits.rs | 83 +++++++++++++++++++++++++++++++++++++++++++++ src/impl_methods.rs | 11 +++--- src/lib.rs | 78 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 5 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index bd4522968..ec83f3151 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -16,6 +16,7 @@ use crate::{ Dimension, RawViewRepr, ViewRepr, + CowRepr, OwnedRepr, OwnedRcRepr, OwnedArcRepr, @@ -412,3 +413,85 @@ unsafe impl DataOwned for OwnedArcRepr { self } } + +unsafe impl<'a, A> RawData for CowRepr<'a, A> + where A: Clone +{ + type Elem = A; + fn _data_slice(&self) -> Option<&[A]> { + match self { + CowRepr::View(view) => view._data_slice(), + CowRepr::Temp(data) => data._data_slice(), + } + } + private_impl!{} +} + +unsafe impl<'a, A> RawDataMut for CowRepr<'a, A> + where A: Clone +{ + #[inline] + fn try_ensure_unique(array: &mut ArrayBase) + where Self: Sized, + D: Dimension + { + array.ensure_temp(); + } + + #[inline] + fn try_is_unique(&mut self) -> Option { + Some(self.is_temp()) + } +} + +unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> + where A: Copy +{ + unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { + match self { + CowRepr::View(view) => { + let (new_view, ptr) = view.clone_with_ptr(ptr); + (CowRepr::View(new_view), ptr) + }, + CowRepr::Temp(data) => { + let (new_data, ptr) = data.clone_with_ptr(ptr); + (CowRepr::Temp(new_data), ptr) + }, + } + } + + #[doc(hidden)] + unsafe fn clone_from_with_ptr(&mut self, other: &Self, ptr: *mut Self::Elem) -> *mut Self::Elem { + match self { + CowRepr::View(view) => { + match other { + CowRepr::View(other_view) => view.clone_from_with_ptr(other_view, ptr), + CowRepr::Temp(_) => panic!("Cannot copy `CowRepr::View` from `CowRepr::Temp`"), + } + }, + CowRepr::Temp(data) => { + match other { + CowRepr::View(_) => panic!("Cannot copy `CowRepr::Temp` from `CowRepr::View`"), + CowRepr::Temp(other_data) => data.clone_from_with_ptr(other_data, ptr), + } + }, + } + } +} + +unsafe impl<'a, A> Data for CowRepr<'a, A> + where A: Clone +{ + #[inline] + fn into_owned(self_: ArrayBase, D>) -> ArrayBase, D> + where + A: Clone, + D: Dimension, + { + if self_.data.is_view() { + ViewRepr::into_owned(self_.into_view_array().unwrap()) + } else { + OwnedRepr::into_owned(self_.into_owned_array().unwrap()) + } + } +} diff --git a/src/impl_methods.rs b/src/impl_methods.rs index e325bc692..620da10a2 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -14,7 +14,7 @@ use itertools::{izip, zip}; use crate::imp_prelude::*; -use crate::arraytraits; +use crate::{arraytraits, CowArray}; use crate::dimension; use crate::error::{self, ShapeError, ErrorKind}; use crate::dimension::IntoDimension; @@ -1226,18 +1226,19 @@ where D::is_contiguous(&self.dim, &self.strides) } - pub fn as_contiguous(&self) -> ArrayBase, D> + pub fn as_contiguous(&self) -> CowArray<'_, A, D> where S: Data, A: Clone { if self.is_standard_layout() { - self.to_owned() + CowArray::from_view_array(self.view()) } else { let v = self.iter().map(|x| x.clone()).collect::>(); - unsafe { + let owned_array: Array = unsafe { // Safe because we use shape and content of existing array here. ArrayBase::from_shape_vec_unchecked(self.dim(), v) - } + }; + CowArray::from_owned_array(owned_array) } } diff --git a/src/lib.rs b/src/lib.rs index 8b44c21d2..baba7a983 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1374,6 +1374,30 @@ impl ViewRepr { } } +pub enum CowRepr<'a, A> + where A: Clone +{ + View(ViewRepr<&'a A>), + Temp(OwnedRepr), +} + +impl<'a, A> CowRepr<'a, A> + where A: Clone +{ + pub fn is_view(&self) -> bool { + match self { + CowRepr::View(_) => true, + CowRepr::Temp(_) => false, + } + } + + pub fn is_temp(&self) -> bool { + !self.is_view() + } +} + +pub type CowArray<'a, A, D> = ArrayBase, D>; + mod impl_clone; mod impl_constructors; @@ -1472,6 +1496,60 @@ impl ArrayBase } } +impl<'a, A, D> CowArray<'a, A, D> + where A: Clone, + D: Dimension +{ + fn from_view_array(array: ArrayView<'a, A, D>) -> CowArray<'a, A, D> { + ArrayBase { + data: CowRepr::View(array.data), + ptr: array.ptr, + dim: array.dim, + strides: array.strides, + } + } + + fn from_owned_array(array: Array) -> CowArray<'a, A, D> { + ArrayBase { + data: CowRepr::Temp(array.data), + ptr: array.ptr, + dim: array.dim, + strides: array.strides, + } + } + + fn into_view_array(self) -> Option> { + match self.data { + CowRepr::View(view) => Some(ArrayBase { + data: view, + ptr: self.ptr, + dim: self.dim, + strides: self.strides, + }), + CowRepr::Temp(_) => None, + } + } + + fn into_owned_array(self) -> Option> { + match self.data { + CowRepr::View(_) => None, + CowRepr::Temp(data) => Some(ArrayBase { + data, + ptr: self.ptr, + dim: self.dim, + strides: self.strides, + }) + } + } + + fn ensure_temp(&mut self) { + if self.data.is_view() { + let copied_data: Vec = self.iter().map(|x| x.clone()).collect(); + self.data = CowRepr::Temp(OwnedRepr(copied_data)); + } + } +} + // parallel methods #[cfg(feature="rayon")] From c82359aefed84e8dc4d3e6cdad1f161ba3aa31d4 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 26 Apr 2019 18:22:02 +0300 Subject: [PATCH 04/25] Removed unused import --- src/impl_methods.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 620da10a2..a481c29d4 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -43,7 +43,6 @@ use crate::iter::{ Windows }; use crate::stacking::stack; -use crate::OwnedRepr; /// # Methods For All Array Types impl ArrayBase From 5ce8d31d53e685964bbdeaee635d0fea746b6acd Mon Sep 17 00:00:00 2001 From: "andrei.papou" Date: Sun, 28 Apr 2019 11:31:00 +0300 Subject: [PATCH 05/25] Fixed wording + minor improvements --- src/data_traits.rs | 16 ++++++++-------- src/impl_methods.rs | 10 +++++----- src/lib.rs | 27 ++++++++++++++------------- tests/array.rs | 2 +- 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index ec83f3151..137de0bc4 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -421,7 +421,7 @@ unsafe impl<'a, A> RawData for CowRepr<'a, A> fn _data_slice(&self) -> Option<&[A]> { match self { CowRepr::View(view) => view._data_slice(), - CowRepr::Temp(data) => data._data_slice(), + CowRepr::Owned(data) => data._data_slice(), } } private_impl!{} @@ -435,12 +435,12 @@ unsafe impl<'a, A> RawDataMut for CowRepr<'a, A> where Self: Sized, D: Dimension { - array.ensure_temp(); + array.ensure_is_owned(); } #[inline] fn try_is_unique(&mut self) -> Option { - Some(self.is_temp()) + Some(self.is_owned()) } } @@ -453,9 +453,9 @@ unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> let (new_view, ptr) = view.clone_with_ptr(ptr); (CowRepr::View(new_view), ptr) }, - CowRepr::Temp(data) => { + CowRepr::Owned(data) => { let (new_data, ptr) = data.clone_with_ptr(ptr); - (CowRepr::Temp(new_data), ptr) + (CowRepr::Owned(new_data), ptr) }, } } @@ -466,13 +466,13 @@ unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> CowRepr::View(view) => { match other { CowRepr::View(other_view) => view.clone_from_with_ptr(other_view, ptr), - CowRepr::Temp(_) => panic!("Cannot copy `CowRepr::View` from `CowRepr::Temp`"), + CowRepr::Owned(_) => panic!("Cannot copy `CowRepr::View` from `CowRepr::Temp`"), } }, - CowRepr::Temp(data) => { + CowRepr::Owned(data) => { match other { CowRepr::View(_) => panic!("Cannot copy `CowRepr::Temp` from `CowRepr::View`"), - CowRepr::Temp(other_data) => data.clone_from_with_ptr(other_data, ptr), + CowRepr::Owned(other_data) => data.clone_from_with_ptr(other_data, ptr), } }, } diff --git a/src/impl_methods.rs b/src/impl_methods.rs index a481c29d4..4dd85bf58 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -14,7 +14,7 @@ use itertools::{izip, zip}; use crate::imp_prelude::*; -use crate::{arraytraits, CowArray}; +use crate::{arraytraits, ArrayCow}; use crate::dimension; use crate::error::{self, ShapeError, ErrorKind}; use crate::dimension::IntoDimension; @@ -1225,19 +1225,19 @@ where D::is_contiguous(&self.dim, &self.strides) } - pub fn as_contiguous(&self) -> CowArray<'_, A, D> - where S: Data, + pub fn as_standard_layout(&self) -> ArrayCow<'_, A, D> + where S: Data, A: Clone { if self.is_standard_layout() { - CowArray::from_view_array(self.view()) + ArrayCow::from_view_array(self.view()) } else { let v = self.iter().map(|x| x.clone()).collect::>(); let owned_array: Array = unsafe { // Safe because we use shape and content of existing array here. ArrayBase::from_shape_vec_unchecked(self.dim(), v) }; - CowArray::from_owned_array(owned_array) + ArrayCow::from_owned_array(owned_array) } } diff --git a/src/lib.rs b/src/lib.rs index baba7a983..b76f27e4d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1378,7 +1378,7 @@ pub enum CowRepr<'a, A> where A: Clone { View(ViewRepr<&'a A>), - Temp(OwnedRepr), + Owned(OwnedRepr), } impl<'a, A> CowRepr<'a, A> @@ -1387,16 +1387,16 @@ impl<'a, A> CowRepr<'a, A> pub fn is_view(&self) -> bool { match self { CowRepr::View(_) => true, - CowRepr::Temp(_) => false, + CowRepr::Owned(_) => false, } } - pub fn is_temp(&self) -> bool { + pub fn is_owned(&self) -> bool { !self.is_view() } } -pub type CowArray<'a, A, D> = ArrayBase, D>; +pub type ArrayCow<'a, A, D> = ArrayBase, D>; mod impl_clone; @@ -1496,11 +1496,11 @@ impl ArrayBase } } -impl<'a, A, D> CowArray<'a, A, D> +impl<'a, A, D> ArrayCow<'a, A, D> where A: Clone, D: Dimension { - fn from_view_array(array: ArrayView<'a, A, D>) -> CowArray<'a, A, D> { + fn from_view_array(array: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { ArrayBase { data: CowRepr::View(array.data), ptr: array.ptr, @@ -1509,9 +1509,9 @@ impl<'a, A, D> CowArray<'a, A, D> } } - fn from_owned_array(array: Array) -> CowArray<'a, A, D> { + fn from_owned_array(array: Array) -> ArrayCow<'a, A, D> { ArrayBase { - data: CowRepr::Temp(array.data), + data: CowRepr::Owned(array.data), ptr: array.ptr, dim: array.dim, strides: array.strides, @@ -1526,14 +1526,14 @@ impl<'a, A, D> CowArray<'a, A, D> dim: self.dim, strides: self.strides, }), - CowRepr::Temp(_) => None, + CowRepr::Owned(_) => None, } } fn into_owned_array(self) -> Option> { match self.data { CowRepr::View(_) => None, - CowRepr::Temp(data) => Some(ArrayBase { + CowRepr::Owned(data) => Some(ArrayBase { data, ptr: self.ptr, dim: self.dim, @@ -1542,10 +1542,11 @@ impl<'a, A, D> CowArray<'a, A, D> } } - fn ensure_temp(&mut self) { + fn ensure_is_owned(&mut self) { if self.data.is_view() { - let copied_data: Vec = self.iter().map(|x| x.clone()).collect(); - self.data = CowRepr::Temp(OwnedRepr(copied_data)); + let mut copied_data: Vec = self.iter().map(|x| x.clone()).collect(); + self.ptr = copied_data.as_mut_ptr(); + self.data = CowRepr::Owned(OwnedRepr(copied_data)); } } } diff --git a/tests/array.rs b/tests/array.rs index 339c61c90..ed5099c4e 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1898,7 +1898,7 @@ fn test_as_contiguous() { let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); assert!(!arr.is_standard_layout()); - let cont_arr = arr.as_contiguous(); + let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); assert!(arr.iter().zip(cont_arr.iter()).all(|(x1, x2)| x1 == x2)); } From 2a67f25e43792e146ac6fbe211261ba0f42140cf Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Tue, 30 Apr 2019 18:05:40 +0300 Subject: [PATCH 06/25] Added more tests + 2 methods for ArrayCow --- src/lib.rs | 8 ++++++++ tests/array.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b76f27e4d..a85307c7b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1549,6 +1549,14 @@ impl<'a, A, D> ArrayCow<'a, A, D> self.data = CowRepr::Owned(OwnedRepr(copied_data)); } } + + pub fn is_view(&self) -> bool { + self.data.is_view() + } + + pub fn is_owned(&self) -> bool { + self.data.is_owned() + } } diff --git a/tests/array.rs b/tests/array.rs index ed5099c4e..32995fc2e 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -4,7 +4,7 @@ extern crate ndarray; extern crate defmac; extern crate itertools; -use ndarray::{Slice, SliceInfo, SliceOrIndex}; +use ndarray::{Slice, SliceInfo, SliceOrIndex, Data}; use ndarray::prelude::*; use ndarray::{ rcarr2, @@ -1894,11 +1894,63 @@ fn array_macros() { #[test] fn test_as_contiguous() { + fn is_content_identical(arr1: &ArrayBase, arr2: &ArrayBase) -> bool + where A: Clone + PartialEq, + S1: Data, + S2: Data, + D: Dimension + { + arr1.iter().zip(arr2.iter()).all(|(x1, x2)| x1 == x2) + } + + // F layout let shape = Ix2(2, 2).strides(Ix2(1, 2)); let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); assert!(!arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); + + // C layout + let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); + assert!(arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_view()); + // F layout view + let shape = Ix2(2, 2).strides(Ix2(1, 2)); + let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); + let arr_view = arr.view(); + assert!(!arr_view.is_standard_layout()); let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(arr.iter().zip(cont_arr.iter()).all(|(x1, x2)| x1 == x2)); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); + + // C layout view + let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); + let arr_view = arr.view(); + assert!(arr_view.is_standard_layout()); + let cont_arr = arr_view.as_standard_layout(); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_view()); + + // 0 dimensional array + let arr_view = ArrayView1::::from(&[]); + assert!(arr_view.is_standard_layout()); + let cont_arr = arr_view.as_standard_layout(); + assert!(is_content_identical(&arr_view, &cont_arr)); + assert!(cont_arr.is_view()); + + // Custom layout + let shape = Ix4(1, 2, 3, 2).strides(Ix4(12, 1, 2, 6)); + let arr_data: Vec = (0..12).collect(); + let arr = Array::::from_shape_vec(shape, arr_data).unwrap(); + assert!(!arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); } From 2d453c7af1be04abb50e475e8dd1858f36c2d188 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 3 May 2019 13:49:06 +0300 Subject: [PATCH 07/25] Split single test into multiple ones Moved all `as_standard_layout`-related tests to the separate module --- tests/array.rs | 121 ++++++++++++++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 52 deletions(-) diff --git a/tests/array.rs b/tests/array.rs index 32995fc2e..ca83a2695 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1892,8 +1892,10 @@ fn array_macros() { assert_eq!(empty2, array![[]]); } -#[test] -fn test_as_contiguous() { +#[cfg(test)] +mod as_standard_layout_tests { + use super::*; + fn is_content_identical(arr1: &ArrayBase, arr2: &ArrayBase) -> bool where A: Clone + PartialEq, S1: Data, @@ -1903,54 +1905,69 @@ fn test_as_contiguous() { arr1.iter().zip(arr2.iter()).all(|(x1, x2)| x1 == x2) } - // F layout - let shape = Ix2(2, 2).strides(Ix2(1, 2)); - let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); - assert!(!arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); - assert!(cont_arr.is_owned()); - - // C layout - let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); - assert!(arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); - assert!(cont_arr.is_view()); - - // F layout view - let shape = Ix2(2, 2).strides(Ix2(1, 2)); - let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); - let arr_view = arr.view(); - assert!(!arr_view.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); - assert!(cont_arr.is_owned()); - - // C layout view - let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); - let arr_view = arr.view(); - assert!(arr_view.is_standard_layout()); - let cont_arr = arr_view.as_standard_layout(); - assert!(is_content_identical(&arr, &cont_arr)); - assert!(cont_arr.is_view()); - - // 0 dimensional array - let arr_view = ArrayView1::::from(&[]); - assert!(arr_view.is_standard_layout()); - let cont_arr = arr_view.as_standard_layout(); - assert!(is_content_identical(&arr_view, &cont_arr)); - assert!(cont_arr.is_view()); - - // Custom layout - let shape = Ix4(1, 2, 3, 2).strides(Ix4(12, 1, 2, 6)); - let arr_data: Vec = (0..12).collect(); - let arr = Array::::from_shape_vec(shape, arr_data).unwrap(); - assert!(!arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(is_content_identical(&arr, &cont_arr)); - assert!(cont_arr.is_owned()); + #[test] + fn test_f_layout() { + let shape = Ix2(2, 2).strides(Ix2(1, 2)); + let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); + assert!(!arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); + } + + #[test] + fn test_c_layout() { + let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); + assert!(arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_view()); + } + + #[test] + fn test_f_layout_view() { + let shape = Ix2(2, 2).strides(Ix2(1, 2)); + let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); + let arr_view = arr.view(); + assert!(!arr_view.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); + } + + #[test] + fn test_c_layout_view() { + let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); + let arr_view = arr.view(); + assert!(arr_view.is_standard_layout()); + let cont_arr = arr_view.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_view()); + } + + #[test] + fn test_zero_dimensional_array() { + let arr_view = ArrayView1::::from(&[]); + assert!(arr_view.is_standard_layout()); + let cont_arr = arr_view.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr_view, &cont_arr)); + assert!(cont_arr.is_view()); + } + + #[test] + fn test_custom_layout() { + let shape = Ix4(1, 2, 3, 2).strides(Ix4(12, 1, 2, 6)); + let arr_data: Vec = (0..12).collect(); + let arr = Array::::from_shape_vec(shape, arr_data).unwrap(); + assert!(!arr.is_standard_layout()); + let cont_arr = arr.as_standard_layout(); + assert!(cont_arr.is_standard_layout()); + assert!(is_content_identical(&arr, &cont_arr)); + assert!(cont_arr.is_owned()); + } } From 0d54de5880ca9eb4b61fa75dd2529c292b10dcce Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:03:31 -0400 Subject: [PATCH 08/25] Fix try_ensure_unique for CowRepr The old implementation incorrectly handled the strides when the array was not in standard layout. (It created a `Vec` corresponding to an array in standard layout but didn't update the strides accordingly.) The new implementation calls `.to_owned()`, which handles the problem of efficiently creating an owned array, and then moves all the field values. Additionally, the `ensure_is_owned` function has been removed because it's equivalent to `DataMut::ensure_unique`. --- src/data_traits.rs | 11 ++++++++++- src/lib.rs | 8 -------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index 137de0bc4..d70ba0a55 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -435,7 +435,16 @@ unsafe impl<'a, A> RawDataMut for CowRepr<'a, A> where Self: Sized, D: Dimension { - array.ensure_is_owned(); + match array.data { + CowRepr::View(_) => { + let owned = array.to_owned(); + array.data = CowRepr::Owned(owned.data); + array.ptr = owned.ptr; + array.dim = owned.dim; + array.strides = owned.strides; + } + CowRepr::Owned(_) => {} + } } #[inline] diff --git a/src/lib.rs b/src/lib.rs index a85307c7b..c0dda3a3a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1542,14 +1542,6 @@ impl<'a, A, D> ArrayCow<'a, A, D> } } - fn ensure_is_owned(&mut self) { - if self.data.is_view() { - let mut copied_data: Vec = self.iter().map(|x| x.clone()).collect(); - self.ptr = copied_data.as_mut_ptr(); - self.data = CowRepr::Owned(OwnedRepr(copied_data)); - } - } - pub fn is_view(&self) -> bool { self.data.is_view() } From 1f4499f7cb60f9169d1b8be21b290c82fd40933b Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:09:03 -0400 Subject: [PATCH 09/25] Remove into_view/owned_array from ArrayCow These functions were used only in the implementation of `CowRepr::to_owned`. They may turn out to be useful in the future, but until there's a clear use case that is not served by `.view()` and `.into_owned()`, let's remove them. --- src/data_traits.rs | 12 ++++++++---- src/lib.rs | 24 ------------------------ 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index d70ba0a55..e4fa5fbec 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -497,10 +497,14 @@ unsafe impl<'a, A> Data for CowRepr<'a, A> A: Clone, D: Dimension, { - if self_.data.is_view() { - ViewRepr::into_owned(self_.into_view_array().unwrap()) - } else { - OwnedRepr::into_owned(self_.into_owned_array().unwrap()) + match self_.data { + CowRepr::View(_) => self_.to_owned(), + CowRepr::Owned(data) => ArrayBase { + data, + ptr: self_.ptr, + dim: self_.dim, + strides: self_.strides, + }, } } } diff --git a/src/lib.rs b/src/lib.rs index c0dda3a3a..b4691e224 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1518,30 +1518,6 @@ impl<'a, A, D> ArrayCow<'a, A, D> } } - fn into_view_array(self) -> Option> { - match self.data { - CowRepr::View(view) => Some(ArrayBase { - data: view, - ptr: self.ptr, - dim: self.dim, - strides: self.strides, - }), - CowRepr::Owned(_) => None, - } - } - - fn into_owned_array(self) -> Option> { - match self.data { - CowRepr::View(_) => None, - CowRepr::Owned(data) => Some(ArrayBase { - data, - ptr: self.ptr, - dim: self.dim, - strides: self.strides, - }) - } - } - pub fn is_view(&self) -> bool { self.data.is_view() } From 2ede464a7ffa9e117f147d6f85b254ce993d6e41 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:13:23 -0400 Subject: [PATCH 10/25] Relax to A: Clone for RawDataClone impl of CowRepr --- src/data_traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index e4fa5fbec..7c7b41bea 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -454,7 +454,7 @@ unsafe impl<'a, A> RawDataMut for CowRepr<'a, A> } unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> - where A: Copy + where A: Clone { unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { match self { From 17e074b68cead9b9ebef41fc803646b28f4530ad Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:14:15 -0400 Subject: [PATCH 11/25] Implement DataMut for CowRepr --- src/data_traits.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/data_traits.rs b/src/data_traits.rs index 7c7b41bea..a381158ed 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -508,3 +508,5 @@ unsafe impl<'a, A> Data for CowRepr<'a, A> } } } + +unsafe impl<'a, A> DataMut for CowRepr<'a, A> where A: Clone {} From 5506113c44b9a0509d85c4b7d1353127b6fa9041 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:16:44 -0400 Subject: [PATCH 12/25] Impl clone_from_with_ptr for differing CowRepr variants The old implementation panicked when the variants were different; the new implementation performs the necessary clone instead of panicking. The variant of `self` after calling `clone_from_with_ptr` should match the variant of `other`. If the variants are initially different, the data from `other` needs to be cloned, and `self` needs to be changed to that variant. --- src/data_traits.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index a381158ed..d1dbc26d0 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -471,19 +471,23 @@ unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> #[doc(hidden)] unsafe fn clone_from_with_ptr(&mut self, other: &Self, ptr: *mut Self::Elem) -> *mut Self::Elem { - match self { - CowRepr::View(view) => { - match other { - CowRepr::View(other_view) => view.clone_from_with_ptr(other_view, ptr), - CowRepr::Owned(_) => panic!("Cannot copy `CowRepr::View` from `CowRepr::Temp`"), - } + match (&mut *self, other) { + (CowRepr::View(self_), CowRepr::View(other)) => { + self_.clone_from_with_ptr(other, ptr) }, - CowRepr::Owned(data) => { - match other { - CowRepr::View(_) => panic!("Cannot copy `CowRepr::Temp` from `CowRepr::View`"), - CowRepr::Owned(other_data) => data.clone_from_with_ptr(other_data, ptr), - } + (CowRepr::Owned(self_), CowRepr::Owned(other)) => { + self_.clone_from_with_ptr(other, ptr) }, + (_, CowRepr::Owned(other)) => { + let (cloned, ptr) = other.clone_with_ptr(ptr); + *self = CowRepr::Owned(cloned); + ptr + }, + (_, CowRepr::View(other)) => { + let (cloned, ptr) = other.clone_with_ptr(ptr); + *self = CowRepr::View(cloned); + ptr + } } } } From 556bd59764b8a097c36adf768460345ba9a63fef Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:34:07 -0400 Subject: [PATCH 13/25] Add Cow types to preludes --- src/lib.rs | 1 + src/prelude.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index b4691e224..6517603ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -205,6 +205,7 @@ mod imp_prelude { DataShared, RawViewRepr, ViewRepr, + CowRepr, Ix, Ixs, }; pub use crate::dimension::DimensionExt; diff --git a/src/prelude.rs b/src/prelude.rs index 80c98e34c..25ab3ae65 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -25,6 +25,7 @@ pub use crate::{ Array, ArcArray, RcArray, + ArrayCow, ArrayView, ArrayViewMut, RawArrayView, From 3101ca3cf3f594c768ab9f2011f5d3a8f60f92a7 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:35:38 -0400 Subject: [PATCH 14/25] Move ArrayCow method impls into separate module This is analogous to `impl_owned_array`, `impl_views`, and `impl_raw_views`. --- src/impl_cow.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 34 +++------------------------------- 2 files changed, 49 insertions(+), 31 deletions(-) create mode 100644 src/impl_cow.rs diff --git a/src/impl_cow.rs b/src/impl_cow.rs new file mode 100644 index 000000000..4481a4b6b --- /dev/null +++ b/src/impl_cow.rs @@ -0,0 +1,46 @@ +// Copyright 2019 ndarray developers. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use crate::imp_prelude::*; + +/// Methods specific to `ArrayCow`. +/// +/// ***See also all methods for [`ArrayBase`]*** +/// +/// [`ArrayBase`]: struct.ArrayBase.html +impl<'a, A, D> ArrayCow<'a, A, D> +where + A: Clone, + D: Dimension, +{ + fn from_view_array(array: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { + ArrayBase { + data: CowRepr::View(array.data), + ptr: array.ptr, + dim: array.dim, + strides: array.strides, + } + } + + fn from_owned_array(array: Array) -> ArrayCow<'a, A, D> { + ArrayBase { + data: CowRepr::Owned(array.data), + ptr: array.ptr, + dim: array.dim, + strides: array.strides, + } + } + + pub fn is_view(&self) -> bool { + self.data.is_view() + } + + pub fn is_owned(&self) -> bool { + self.data.is_owned() + } +} diff --git a/src/lib.rs b/src/lib.rs index 6517603ce..3c17623ad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1497,37 +1497,6 @@ impl ArrayBase } } -impl<'a, A, D> ArrayCow<'a, A, D> - where A: Clone, - D: Dimension -{ - fn from_view_array(array: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { - ArrayBase { - data: CowRepr::View(array.data), - ptr: array.ptr, - dim: array.dim, - strides: array.strides, - } - } - - fn from_owned_array(array: Array) -> ArrayCow<'a, A, D> { - ArrayBase { - data: CowRepr::Owned(array.data), - ptr: array.ptr, - dim: array.dim, - strides: array.strides, - } - } - - pub fn is_view(&self) -> bool { - self.data.is_view() - } - - pub fn is_owned(&self) -> bool { - self.data.is_owned() - } -} - // parallel methods #[cfg(feature="rayon")] @@ -1550,6 +1519,9 @@ mod impl_views; // Array raw view methods mod impl_raw_views; +// Copy-on-write array methods +mod impl_cow; + /// A contiguous array shape of n dimensions. /// /// Either c- or f- memory ordered (*c* a.k.a *row major* is the default). From 131aea75fb2fdb9756860daeaaca588f2e4e6d74 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:38:22 -0400 Subject: [PATCH 15/25] Replace from_view/owned_array with From impls --- src/impl_cow.rs | 40 ++++++++++++++++++++++++++-------------- src/impl_methods.rs | 4 ++-- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/impl_cow.rs b/src/impl_cow.rs index 4481a4b6b..99a1d2e3c 100644 --- a/src/impl_cow.rs +++ b/src/impl_cow.rs @@ -18,16 +18,36 @@ where A: Clone, D: Dimension, { - fn from_view_array(array: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { + pub fn is_view(&self) -> bool { + self.data.is_view() + } + + pub fn is_owned(&self) -> bool { + self.data.is_owned() + } +} + +impl<'a, A, D> From> for ArrayCow<'a, A, D> +where + A: Clone, + D: Dimension, +{ + fn from(view: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { ArrayBase { - data: CowRepr::View(array.data), - ptr: array.ptr, - dim: array.dim, - strides: array.strides, + data: CowRepr::View(view.data), + ptr: view.ptr, + dim: view.dim, + strides: view.strides, } } +} - fn from_owned_array(array: Array) -> ArrayCow<'a, A, D> { +impl<'a, A, D> From> for ArrayCow<'a, A, D> +where + A: Clone, + D: Dimension, +{ + fn from(array: Array) -> ArrayCow<'a, A, D> { ArrayBase { data: CowRepr::Owned(array.data), ptr: array.ptr, @@ -35,12 +55,4 @@ where strides: array.strides, } } - - pub fn is_view(&self) -> bool { - self.data.is_view() - } - - pub fn is_owned(&self) -> bool { - self.data.is_owned() - } } diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 4dd85bf58..ad0f3b716 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -1230,14 +1230,14 @@ where A: Clone { if self.is_standard_layout() { - ArrayCow::from_view_array(self.view()) + ArrayCow::from(self.view()) } else { let v = self.iter().map(|x| x.clone()).collect::>(); let owned_array: Array = unsafe { // Safe because we use shape and content of existing array here. ArrayBase::from_shape_vec_unchecked(self.dim(), v) }; - ArrayCow::from_owned_array(owned_array) + ArrayCow::from(owned_array) } } From 0d68ea634b6abcf90a443943174d8e4153a9215c Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:45:52 -0400 Subject: [PATCH 16/25] Add docs for ArrayCow --- src/lib.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3c17623ad..b3d073d9e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1231,6 +1231,20 @@ pub type ArcArray = ArrayBase, D>; /// and so on. pub type Array = ArrayBase, D>; +/// An array with copy-on-write behavior. +/// +/// An `ArrayCow` represents either a uniquely owned array or a view of an +/// array. The `'a` corresponds to the lifetime of the view variant. +/// +/// Array views have all the methods of an array (see [`ArrayBase`][ab]). +/// +/// See also [`ArcArray`](type.ArcArray.html), which also provides +/// copy-on-write behavior but has a reference-counted pointer to the data +/// instead of either a view or a uniquely owned copy. +/// +/// [ab]: struct.ArrayBase.html +pub type ArrayCow<'a, A, D> = ArrayBase, D>; + /// A read-only array view. /// /// An array view represents an array or a part of it, created from @@ -1397,8 +1411,6 @@ impl<'a, A> CowRepr<'a, A> } } -pub type ArrayCow<'a, A, D> = ArrayBase, D>; - mod impl_clone; mod impl_constructors; From b84b3ec7c6577c852aae98e01d29b2534f494e6a Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 19:49:05 -0400 Subject: [PATCH 17/25] Remove A: Clone bound from CowRepr In the vast majority of cases, `A` will implement `Clone`. However, `ArrayCow` may still be useful for cases where `A` does not implement `Clone`. --- src/data_traits.rs | 8 ++------ src/impl_cow.rs | 3 --- src/lib.rs | 8 ++------ 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/data_traits.rs b/src/data_traits.rs index d1dbc26d0..f4df024c6 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -414,9 +414,7 @@ unsafe impl DataOwned for OwnedArcRepr { } } -unsafe impl<'a, A> RawData for CowRepr<'a, A> - where A: Clone -{ +unsafe impl<'a, A> RawData for CowRepr<'a, A> { type Elem = A; fn _data_slice(&self) -> Option<&[A]> { match self { @@ -492,9 +490,7 @@ unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> } } -unsafe impl<'a, A> Data for CowRepr<'a, A> - where A: Clone -{ +unsafe impl<'a, A> Data for CowRepr<'a, A> { #[inline] fn into_owned(self_: ArrayBase, D>) -> ArrayBase, D> where diff --git a/src/impl_cow.rs b/src/impl_cow.rs index 99a1d2e3c..47b2af574 100644 --- a/src/impl_cow.rs +++ b/src/impl_cow.rs @@ -15,7 +15,6 @@ use crate::imp_prelude::*; /// [`ArrayBase`]: struct.ArrayBase.html impl<'a, A, D> ArrayCow<'a, A, D> where - A: Clone, D: Dimension, { pub fn is_view(&self) -> bool { @@ -29,7 +28,6 @@ where impl<'a, A, D> From> for ArrayCow<'a, A, D> where - A: Clone, D: Dimension, { fn from(view: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { @@ -44,7 +42,6 @@ where impl<'a, A, D> From> for ArrayCow<'a, A, D> where - A: Clone, D: Dimension, { fn from(array: Array) -> ArrayCow<'a, A, D> { diff --git a/src/lib.rs b/src/lib.rs index b3d073d9e..a96c3d63b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1389,16 +1389,12 @@ impl ViewRepr { } } -pub enum CowRepr<'a, A> - where A: Clone -{ +pub enum CowRepr<'a, A> { View(ViewRepr<&'a A>), Owned(OwnedRepr), } -impl<'a, A> CowRepr<'a, A> - where A: Clone -{ +impl<'a, A> CowRepr<'a, A> { pub fn is_view(&self) -> bool { match self { CowRepr::View(_) => true, From ebd6cc4517917167d06d200e6e651b5086d133a2 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 20:05:15 -0400 Subject: [PATCH 18/25] Use .raw_dim() instead of .dim() The compiler should have a slightly easier time making this efficient because there are fewer type conversions this way. --- src/impl_methods.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index ad0f3b716..3de251d81 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -1235,7 +1235,7 @@ where let v = self.iter().map(|x| x.clone()).collect::>(); let owned_array: Array = unsafe { // Safe because we use shape and content of existing array here. - ArrayBase::from_shape_vec_unchecked(self.dim(), v) + ArrayBase::from_shape_vec_unchecked(self.raw_dim(), v) }; ArrayCow::from(owned_array) } From b14b3ded36bb4d9708dcc996847dd3198679752a Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Sun, 5 May 2019 20:10:20 -0400 Subject: [PATCH 19/25] Replace .map(|x| x.clone()) with .cloned() --- src/impl_methods.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 3de251d81..a8a4ecea6 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -1232,7 +1232,7 @@ where if self.is_standard_layout() { ArrayCow::from(self.view()) } else { - let v = self.iter().map(|x| x.clone()).collect::>(); + let v = self.iter().cloned().collect::>(); let owned_array: Array = unsafe { // Safe because we use shape and content of existing array here. ArrayBase::from_shape_vec_unchecked(self.raw_dim(), v) From 1417bcbe92ab57f50e26b89749c978c7b0d1e888 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 31 May 2019 12:14:16 +0300 Subject: [PATCH 20/25] Removed ArrayCow changes --- src/data_traits.rs | 98 ---------------------------------------------- src/impl_cow.rs | 55 -------------------------- src/lib.rs | 36 ----------------- src/prelude.rs | 1 - 4 files changed, 190 deletions(-) delete mode 100644 src/impl_cow.rs diff --git a/src/data_traits.rs b/src/data_traits.rs index f4df024c6..bd4522968 100644 --- a/src/data_traits.rs +++ b/src/data_traits.rs @@ -16,7 +16,6 @@ use crate::{ Dimension, RawViewRepr, ViewRepr, - CowRepr, OwnedRepr, OwnedRcRepr, OwnedArcRepr, @@ -413,100 +412,3 @@ unsafe impl DataOwned for OwnedArcRepr { self } } - -unsafe impl<'a, A> RawData for CowRepr<'a, A> { - type Elem = A; - fn _data_slice(&self) -> Option<&[A]> { - match self { - CowRepr::View(view) => view._data_slice(), - CowRepr::Owned(data) => data._data_slice(), - } - } - private_impl!{} -} - -unsafe impl<'a, A> RawDataMut for CowRepr<'a, A> - where A: Clone -{ - #[inline] - fn try_ensure_unique(array: &mut ArrayBase) - where Self: Sized, - D: Dimension - { - match array.data { - CowRepr::View(_) => { - let owned = array.to_owned(); - array.data = CowRepr::Owned(owned.data); - array.ptr = owned.ptr; - array.dim = owned.dim; - array.strides = owned.strides; - } - CowRepr::Owned(_) => {} - } - } - - #[inline] - fn try_is_unique(&mut self) -> Option { - Some(self.is_owned()) - } -} - -unsafe impl<'a, A> RawDataClone for CowRepr<'a, A> - where A: Clone -{ - unsafe fn clone_with_ptr(&self, ptr: *mut Self::Elem) -> (Self, *mut Self::Elem) { - match self { - CowRepr::View(view) => { - let (new_view, ptr) = view.clone_with_ptr(ptr); - (CowRepr::View(new_view), ptr) - }, - CowRepr::Owned(data) => { - let (new_data, ptr) = data.clone_with_ptr(ptr); - (CowRepr::Owned(new_data), ptr) - }, - } - } - - #[doc(hidden)] - unsafe fn clone_from_with_ptr(&mut self, other: &Self, ptr: *mut Self::Elem) -> *mut Self::Elem { - match (&mut *self, other) { - (CowRepr::View(self_), CowRepr::View(other)) => { - self_.clone_from_with_ptr(other, ptr) - }, - (CowRepr::Owned(self_), CowRepr::Owned(other)) => { - self_.clone_from_with_ptr(other, ptr) - }, - (_, CowRepr::Owned(other)) => { - let (cloned, ptr) = other.clone_with_ptr(ptr); - *self = CowRepr::Owned(cloned); - ptr - }, - (_, CowRepr::View(other)) => { - let (cloned, ptr) = other.clone_with_ptr(ptr); - *self = CowRepr::View(cloned); - ptr - } - } - } -} - -unsafe impl<'a, A> Data for CowRepr<'a, A> { - #[inline] - fn into_owned(self_: ArrayBase, D>) -> ArrayBase, D> - where - A: Clone, - D: Dimension, - { - match self_.data { - CowRepr::View(_) => self_.to_owned(), - CowRepr::Owned(data) => ArrayBase { - data, - ptr: self_.ptr, - dim: self_.dim, - strides: self_.strides, - }, - } - } -} - -unsafe impl<'a, A> DataMut for CowRepr<'a, A> where A: Clone {} diff --git a/src/impl_cow.rs b/src/impl_cow.rs deleted file mode 100644 index 47b2af574..000000000 --- a/src/impl_cow.rs +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright 2019 ndarray developers. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use crate::imp_prelude::*; - -/// Methods specific to `ArrayCow`. -/// -/// ***See also all methods for [`ArrayBase`]*** -/// -/// [`ArrayBase`]: struct.ArrayBase.html -impl<'a, A, D> ArrayCow<'a, A, D> -where - D: Dimension, -{ - pub fn is_view(&self) -> bool { - self.data.is_view() - } - - pub fn is_owned(&self) -> bool { - self.data.is_owned() - } -} - -impl<'a, A, D> From> for ArrayCow<'a, A, D> -where - D: Dimension, -{ - fn from(view: ArrayView<'a, A, D>) -> ArrayCow<'a, A, D> { - ArrayBase { - data: CowRepr::View(view.data), - ptr: view.ptr, - dim: view.dim, - strides: view.strides, - } - } -} - -impl<'a, A, D> From> for ArrayCow<'a, A, D> -where - D: Dimension, -{ - fn from(array: Array) -> ArrayCow<'a, A, D> { - ArrayBase { - data: CowRepr::Owned(array.data), - ptr: array.ptr, - dim: array.dim, - strides: array.strides, - } - } -} diff --git a/src/lib.rs b/src/lib.rs index a96c3d63b..8b44c21d2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -205,7 +205,6 @@ mod imp_prelude { DataShared, RawViewRepr, ViewRepr, - CowRepr, Ix, Ixs, }; pub use crate::dimension::DimensionExt; @@ -1231,20 +1230,6 @@ pub type ArcArray = ArrayBase, D>; /// and so on. pub type Array = ArrayBase, D>; -/// An array with copy-on-write behavior. -/// -/// An `ArrayCow` represents either a uniquely owned array or a view of an -/// array. The `'a` corresponds to the lifetime of the view variant. -/// -/// Array views have all the methods of an array (see [`ArrayBase`][ab]). -/// -/// See also [`ArcArray`](type.ArcArray.html), which also provides -/// copy-on-write behavior but has a reference-counted pointer to the data -/// instead of either a view or a uniquely owned copy. -/// -/// [ab]: struct.ArrayBase.html -pub type ArrayCow<'a, A, D> = ArrayBase, D>; - /// A read-only array view. /// /// An array view represents an array or a part of it, created from @@ -1389,24 +1374,6 @@ impl ViewRepr { } } -pub enum CowRepr<'a, A> { - View(ViewRepr<&'a A>), - Owned(OwnedRepr), -} - -impl<'a, A> CowRepr<'a, A> { - pub fn is_view(&self) -> bool { - match self { - CowRepr::View(_) => true, - CowRepr::Owned(_) => false, - } - } - - pub fn is_owned(&self) -> bool { - !self.is_view() - } -} - mod impl_clone; mod impl_constructors; @@ -1527,9 +1494,6 @@ mod impl_views; // Array raw view methods mod impl_raw_views; -// Copy-on-write array methods -mod impl_cow; - /// A contiguous array shape of n dimensions. /// /// Either c- or f- memory ordered (*c* a.k.a *row major* is the default). diff --git a/src/prelude.rs b/src/prelude.rs index 25ab3ae65..80c98e34c 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -25,7 +25,6 @@ pub use crate::{ Array, ArcArray, RcArray, - ArrayCow, ArrayView, ArrayViewMut, RawArrayView, From 4c2d2dddceb2907c28a87b0c46bbbd4253e851f9 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 31 May 2019 12:28:20 +0300 Subject: [PATCH 21/25] Updated the tests --- tests/array.rs | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/tests/array.rs b/tests/array.rs index a1d2aa233..769c7b69a 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -9,7 +9,7 @@ use itertools::{enumerate, zip, Itertools}; use ndarray::indices; use ndarray::prelude::*; use ndarray::{arr3, multislice, rcarr2}; -use ndarray::{Data, Slice, SliceInfo, SliceOrIndex}; +use ndarray::{Slice, SliceInfo, SliceOrIndex}; macro_rules! assert_panics { ($body:expr) => { @@ -1974,15 +1974,6 @@ fn array_macros() { mod as_standard_layout_tests { use super::*; - fn is_content_identical(arr1: &ArrayBase, arr2: &ArrayBase) -> bool - where A: Clone + PartialEq, - S1: Data, - S2: Data, - D: Dimension - { - arr1.iter().zip(arr2.iter()).all(|(x1, x2)| x1 == x2) - } - #[test] fn test_f_layout() { let shape = Ix2(2, 2).strides(Ix2(1, 2)); @@ -1990,7 +1981,7 @@ mod as_standard_layout_tests { assert!(!arr.is_standard_layout()); let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); + assert_eq!(arr, cont_arr); assert!(cont_arr.is_owned()); } @@ -2000,7 +1991,7 @@ mod as_standard_layout_tests { assert!(arr.is_standard_layout()); let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); + assert_eq!(arr, cont_arr); assert!(cont_arr.is_view()); } @@ -2012,7 +2003,7 @@ mod as_standard_layout_tests { assert!(!arr_view.is_standard_layout()); let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); + assert_eq!(arr, cont_arr); assert!(cont_arr.is_owned()); } @@ -2023,7 +2014,7 @@ mod as_standard_layout_tests { assert!(arr_view.is_standard_layout()); let cont_arr = arr_view.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); + assert_eq!(arr, cont_arr); assert!(cont_arr.is_view()); } @@ -2033,7 +2024,7 @@ mod as_standard_layout_tests { assert!(arr_view.is_standard_layout()); let cont_arr = arr_view.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr_view, &cont_arr)); + assert_eq!(arr_view, cont_arr); assert!(cont_arr.is_view()); } @@ -2045,7 +2036,7 @@ mod as_standard_layout_tests { assert!(!arr.is_standard_layout()); let cont_arr = arr.as_standard_layout(); assert!(cont_arr.is_standard_layout()); - assert!(is_content_identical(&arr, &cont_arr)); + assert_eq!(arr, cont_arr); assert!(cont_arr.is_owned()); } } From ac51cfd937fe0e1162cb1dcab4fc9db45ec54aab Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Fri, 31 May 2019 12:52:13 +0300 Subject: [PATCH 22/25] Fixed rustfmt errors --- src/impl_methods.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 485dcf426..d672fefb3 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -14,7 +14,6 @@ use itertools::{izip, zip}; use crate::imp_prelude::*; -use crate::{arraytraits, CowArray}; use crate::dimension; use crate::dimension::IntoDimension; use crate::dimension::{ @@ -22,6 +21,7 @@ use crate::dimension::{ }; use crate::error::{self, ErrorKind, ShapeError}; use crate::zip::Zip; +use crate::{arraytraits, CowArray}; use crate::iter::{ AxisChunksIter, AxisChunksIterMut, AxisIter, AxisIterMut, ExactChunks, ExactChunksMut, @@ -1219,8 +1219,9 @@ where } pub fn as_standard_layout(&self) -> CowArray<'_, A, D> - where S: Data, - A: Clone + where + S: Data, + A: Clone, { if self.is_standard_layout() { CowArray::from(self.view()) From c2ae416b8ea3578d93951fd4afa7747b1e0710b1 Mon Sep 17 00:00:00 2001 From: andrei-papou Date: Mon, 3 Jun 2019 16:23:45 +0300 Subject: [PATCH 23/25] Changes: - reduced the amount of the boilerplate code in related tests - added explicit assertion about the size of the source array when copying it to the standard layout. - added documentation string to the new method - removed redundant import --- src/impl_methods.rs | 16 ++++++++++---- tests/array.rs | 51 +++++++++++++++++++++------------------------ 2 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index d672fefb3..0f0e7010a 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -14,6 +14,7 @@ use itertools::{izip, zip}; use crate::imp_prelude::*; +use crate::arraytraits; use crate::dimension; use crate::dimension::IntoDimension; use crate::dimension::{ @@ -21,7 +22,6 @@ use crate::dimension::{ }; use crate::error::{self, ErrorKind, ShapeError}; use crate::zip::Zip; -use crate::{arraytraits, CowArray}; use crate::iter::{ AxisChunksIter, AxisChunksIterMut, AxisIter, AxisIterMut, ExactChunks, ExactChunksMut, @@ -1218,6 +1218,11 @@ where D::is_contiguous(&self.dim, &self.strides) } + /// Return a representation of the array in the standard layout. + /// + /// Return the COW view of the array if it is in the standard layout. + /// Otherwise copy the array, transform the copy to the standard layout and return it as + /// an owned COW array. pub fn as_standard_layout(&self) -> CowArray<'_, A, D> where S: Data, @@ -1226,10 +1231,13 @@ where if self.is_standard_layout() { CowArray::from(self.view()) } else { - let v = self.iter().cloned().collect::>(); + let v: Vec = self.iter().cloned().collect(); + let dim = self.dim.clone(); + assert_eq!(v.len(), dim.size()); let owned_array: Array = unsafe { - // Safe because we use shape and content of existing array here. - ArrayBase::from_shape_vec_unchecked(self.raw_dim(), v) + // Safe because the shape and element type are from the existing array + // and the strides are the default strides. + Array::from_shape_vec_unchecked(dim, v) }; CowArray::from(owned_array) } diff --git a/tests/array.rs b/tests/array.rs index 769c7b69a..178381a66 100644 --- a/tests/array.rs +++ b/tests/array.rs @@ -1973,38 +1973,44 @@ fn array_macros() { #[cfg(test)] mod as_standard_layout_tests { use super::*; + use ndarray::Data; + use std::fmt::Debug; + + fn test_as_standard_layout_for(orig: ArrayBase) + where + S: Data, + S::Elem: Clone + Debug + PartialEq, + D: Dimension, + { + let orig_is_standard = orig.is_standard_layout(); + let out = orig.as_standard_layout(); + assert!(out.is_standard_layout()); + assert_eq!(out, orig); + assert_eq!(orig_is_standard, out.is_view()); + } #[test] fn test_f_layout() { - let shape = Ix2(2, 2).strides(Ix2(1, 2)); + let shape = (2, 2).f(); let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); assert!(!arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr, cont_arr); - assert!(cont_arr.is_owned()); + test_as_standard_layout_for(arr); } #[test] fn test_c_layout() { let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); assert!(arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr, cont_arr); - assert!(cont_arr.is_view()); + test_as_standard_layout_for(arr); } #[test] fn test_f_layout_view() { - let shape = Ix2(2, 2).strides(Ix2(1, 2)); + let shape = (2, 2).f(); let arr = Array::::from_shape_vec(shape, vec![1, 2, 3, 4]).unwrap(); let arr_view = arr.view(); assert!(!arr_view.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr, cont_arr); - assert!(cont_arr.is_owned()); + test_as_standard_layout_for(arr); } #[test] @@ -2012,32 +2018,23 @@ mod as_standard_layout_tests { let arr = Array::::from_shape_vec((2, 2), vec![1, 2, 3, 4]).unwrap(); let arr_view = arr.view(); assert!(arr_view.is_standard_layout()); - let cont_arr = arr_view.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr, cont_arr); - assert!(cont_arr.is_view()); + test_as_standard_layout_for(arr_view); } #[test] fn test_zero_dimensional_array() { let arr_view = ArrayView1::::from(&[]); assert!(arr_view.is_standard_layout()); - let cont_arr = arr_view.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr_view, cont_arr); - assert!(cont_arr.is_view()); + test_as_standard_layout_for(arr_view); } #[test] fn test_custom_layout() { - let shape = Ix4(1, 2, 3, 2).strides(Ix4(12, 1, 2, 6)); + let shape = (1, 2, 3, 2).strides((12, 1, 2, 6)); let arr_data: Vec = (0..12).collect(); let arr = Array::::from_shape_vec(shape, arr_data).unwrap(); assert!(!arr.is_standard_layout()); - let cont_arr = arr.as_standard_layout(); - assert!(cont_arr.is_standard_layout()); - assert_eq!(arr, cont_arr); - assert!(cont_arr.is_owned()); + test_as_standard_layout_for(arr); } } From 02d181d5156db1555db6028a6639927e93fd5e75 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Tue, 18 Jun 2019 20:44:03 -0400 Subject: [PATCH 24/25] Refine as_standard_layout docs --- src/impl_methods.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 0f0e7010a..20470d08e 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -1218,11 +1218,12 @@ where D::is_contiguous(&self.dim, &self.strides) } - /// Return a representation of the array in the standard layout. + /// Return a standard-layout array containing the data, cloning if + /// necessary. /// - /// Return the COW view of the array if it is in the standard layout. - /// Otherwise copy the array, transform the copy to the standard layout and return it as - /// an owned COW array. + /// If `self` is in standard layout, a COW view of the data is returned + /// without cloning. Otherwise, the data is cloned, and the returned array + /// owns the cloned data. pub fn as_standard_layout(&self) -> CowArray<'_, A, D> where S: Data, From f669f0fa6b3a08678e6db1b1c03786f13adfe815 Mon Sep 17 00:00:00 2001 From: Jim Turner Date: Tue, 18 Jun 2019 21:09:23 -0400 Subject: [PATCH 25/25] Add as_standard_layout example --- src/impl_methods.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/impl_methods.rs b/src/impl_methods.rs index 20470d08e..19775809b 100644 --- a/src/impl_methods.rs +++ b/src/impl_methods.rs @@ -1224,6 +1224,22 @@ where /// If `self` is in standard layout, a COW view of the data is returned /// without cloning. Otherwise, the data is cloned, and the returned array /// owns the cloned data. + /// + /// ``` + /// use ndarray::Array2; + /// + /// let standard = Array2::::zeros((3, 4)); + /// assert!(standard.is_standard_layout()); + /// let cow_view = standard.as_standard_layout(); + /// assert!(cow_view.is_view()); + /// assert!(cow_view.is_standard_layout()); + /// + /// let fortran = standard.reversed_axes(); + /// assert!(!fortran.is_standard_layout()); + /// let cow_owned = fortran.as_standard_layout(); + /// assert!(cow_owned.is_owned()); + /// assert!(cow_owned.is_standard_layout()); + /// ``` pub fn as_standard_layout(&self) -> CowArray<'_, A, D> where S: Data,