Merge pull request #291 from PyO3/minor-simplifications

RFC: Simplify constructors of one-dimensional arrays

Author: adamreichold

benches/array.rs

@@ -0,0 +1,145 @@
+#![feature(test)]
+
+extern crate test;
+use test::{black_box, Bencher};
+
+use std::ops::Range;
+
+use numpy::PyArray1;
+use pyo3::{Python, ToPyObject};
+
+struct Iter(Range<usize>);
+
+impl Iterator for Iter {
+    type Item = usize;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+}
+
+fn from_iter(bencher: &mut Bencher, size: usize) {
+    iter_with_gil(bencher, |py| {
+        let iter = black_box(Iter(0..size));
+
+        PyArray1::from_iter(py, iter);
+    });
+}
+
+#[bench]
+fn from_iter_small(bencher: &mut Bencher) {
+    from_iter(bencher, 2_usize.pow(5));
+}
+
+#[bench]
+fn from_iter_medium(bencher: &mut Bencher) {
+    from_iter(bencher, 2_usize.pow(10));
+}
+
+#[bench]
+fn from_iter_large(bencher: &mut Bencher) {
+    from_iter(bencher, 2_usize.pow(15));
+}
+
+struct ExactIter(Range<usize>);
+
+impl Iterator for ExactIter {
+    type Item = usize;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        self.0.size_hint()
+    }
+}
+
+impl ExactSizeIterator for ExactIter {
+    fn len(&self) -> usize {
+        self.0.len()
+    }
+}
+
+fn from_exact_iter(bencher: &mut Bencher, size: usize) {
+    iter_with_gil(bencher, |py| {
+        let iter = black_box(ExactIter(0..size));
+
+        PyArray1::from_exact_iter(py, iter);
+    });
+}
+
+#[bench]
+fn from_exact_iter_small(bencher: &mut Bencher) {
+    from_exact_iter(bencher, 2_usize.pow(5));
+}
+
+#[bench]
+fn from_exact_iter_medium(bencher: &mut Bencher) {
+    from_exact_iter(bencher, 2_usize.pow(10));
+}
+
+#[bench]
+fn from_exact_iter_large(bencher: &mut Bencher) {
+    from_exact_iter(bencher, 2_usize.pow(15));
+}
+
+fn from_slice(bencher: &mut Bencher, size: usize) {
+    let vec = (0..size).collect::<Vec<_>>();
+
+    iter_with_gil(bencher, |py| {
+        let slice = black_box(&vec);
+
+        PyArray1::from_slice(py, slice);
+    });
+}
+
+#[bench]
+fn from_slice_small(bencher: &mut Bencher) {
+    from_slice(bencher, 2_usize.pow(5));
+}
+
+#[bench]
+fn from_slice_medium(bencher: &mut Bencher) {
+    from_slice(bencher, 2_usize.pow(10));
+}
+
+#[bench]
+fn from_slice_large(bencher: &mut Bencher) {
+    from_slice(bencher, 2_usize.pow(15));
+}
+
+fn from_object_slice(bencher: &mut Bencher, size: usize) {
+    let vec = Python::with_gil(|py| (0..size).map(|val| val.to_object(py)).collect::<Vec<_>>());
+
+    iter_with_gil(bencher, |py| {
+        let slice = black_box(&vec);
+
+        PyArray1::from_slice(py, slice);
+    });
+}
+
+#[bench]
+fn from_object_slice_small(bencher: &mut Bencher) {
+    from_object_slice(bencher, 2_usize.pow(5));
+}
+
+#[bench]
+fn from_object_slice_medium(bencher: &mut Bencher) {
+    from_object_slice(bencher, 2_usize.pow(10));
+}
+
+#[bench]
+fn from_object_slice_large(bencher: &mut Bencher) {
+    from_object_slice(bencher, 2_usize.pow(15));
+}
+
+fn iter_with_gil(bencher: &mut Bencher, mut f: impl FnMut(Python)) {
+    Python::with_gil(|py| {
+        bencher.iter(|| {
+            let pool = unsafe { py.new_pool() };
+
+            f(pool.python());
+        });
+    });
+}

src/array.rs

@@ -356,10 +356,6 @@ impl<T, D> PyArray<T, D> {
         let ptr = self.as_array_ptr();
         (*ptr).data as *mut _
     }
-
-    pub(crate) unsafe fn copy_ptr(&self, other: *const T, len: usize) {
-        ptr::copy_nonoverlapping(other, self.data(), len)
-    }
 }
 
 struct InvertedAxes(u32);
@@ -959,7 +955,7 @@ impl<T: Element> PyArray<T, Ix1> {
         unsafe {
             let array = PyArray::new(py, [slice.len()], false);
             if T::IS_COPY {
-                array.copy_ptr(slice.as_ptr(), slice.len());
+                ptr::copy_nonoverlapping(slice.as_ptr(), array.data(), slice.len());
             } else {
                 let data_ptr = array.data();
                 for (i, item) in slice.iter().enumerate() {
@@ -983,7 +979,7 @@ impl<T: Element> PyArray<T, Ix1> {
     /// });
     /// ```
     pub fn from_vec<'py>(py: Python<'py>, vec: Vec<T>) -> &'py Self {
-        IntoPyArray::into_pyarray(vec, py)
+        vec.into_pyarray(py)
     }
 
     /// Construct one-dimension PyArray from a type which implements
@@ -1000,20 +996,8 @@ impl<T: Element> PyArray<T, Ix1> {
     /// });
     /// ```
     pub fn from_exact_iter(py: Python<'_>, iter: impl ExactSizeIterator<Item = T>) -> &Self {
-        // NumPy will always zero-initialize object pointers,
-        // so the array can be dropped safely if the iterator panics.
-        unsafe {
-            let len = iter.len();
-            let array = Self::new(py, [len], false);
-            let mut idx = 0;
-            for item in iter {
-                assert!(idx < len);
-                array.uget_raw([idx]).write(item);
-                idx += 1;
-            }
-            assert!(idx == len);
-            array
-        }
+        let data = iter.collect::<Box<[_]>>();
+        data.into_pyarray(py)
     }
 
     /// Construct one-dimension PyArray from a type which implements
@@ -1032,29 +1016,8 @@ impl<T: Element> PyArray<T, Ix1> {
     /// });
     /// ```
     pub fn from_iter(py: Python<'_>, iter: impl IntoIterator<Item = T>) -> &Self {
-        let iter = iter.into_iter();
-        let (min_len, max_len) = iter.size_hint();
-        let mut capacity = max_len.unwrap_or_else(|| min_len.max(512 / mem::size_of::<T>()));
-        unsafe {
-            // NumPy will always zero-initialize object pointers,
-            // so the array can be dropped safely if the iterator panics.
-            let array = Self::new(py, [capacity], false);
-            let mut length = 0;
-            for (i, item) in iter.enumerate() {
-                length += 1;
-                if length > capacity {
-                    capacity *= 2;
-                    array
-                        .resize(capacity)
-                        .expect("PyArray::from_iter: Failed to allocate memory");
-                }
-                array.uget_raw([i]).write(item);
-            }
-            if capacity > length {
-                array.resize(length).unwrap()
-            }
-            array
-        }
+        let data = iter.into_iter().collect::<Vec<_>>();
+        data.into_pyarray(py)
     }
 
     /// Extends or trancates the length of 1 dimension PyArray.
@@ -1339,8 +1302,6 @@ impl<T: Element + AsPrimitive<f64>> PyArray<T, Ix1> {
 mod tests {
     use super::*;
 
-    use std::ops::Range;
-
     #[test]
     fn test_get_unchecked() {
         pyo3::Python::with_gil(|py| {
@@ -1370,36 +1331,4 @@ mod tests {
             py_run!(py, arr, "assert arr.dtype.hasobject");
         });
     }
-
-    struct InsincereIterator(Range<usize>, usize);
-
-    impl Iterator for InsincereIterator {
-        type Item = usize;
-
-        fn next(&mut self) -> Option<Self::Item> {
-            self.0.next()
-        }
-    }
-
-    impl ExactSizeIterator for InsincereIterator {
-        fn len(&self) -> usize {
-            self.1
-        }
-    }
-
-    #[test]
-    #[should_panic]
-    fn from_exact_iter_too_short() {
-        Python::with_gil(|py| {
-            PyArray::from_exact_iter(py, InsincereIterator(0..3, 5));
-        });
-    }
-
-    #[test]
-    #[should_panic]
-    fn from_exact_iter_too_long() {
-        Python::with_gil(|py| {
-            PyArray::from_exact_iter(py, InsincereIterator(0..5, 3));
-        });
-    }
 }

src/convert.rs

@@ -1,7 +1,7 @@
 //! Defines conversion traits between Rust types and NumPy data types.
 #![deny(missing_docs)]
 
-use std::{mem, os::raw::c_int};
+use std::{mem, os::raw::c_int, ptr};
 
 use ndarray::{ArrayBase, Data, Dimension, IntoDimension, Ix1, OwnedRepr};
 use pyo3::Python;
@@ -143,12 +143,12 @@ where
         let len = self.len();
         match self.order() {
             Some(order) if A::IS_COPY => {
-                // if the array is contiguous, copy it by `copy_ptr`.
+                // if the array is contiguous, copy it by `copy_nonoverlapping`.
                 let strides = self.npy_strides();
                 unsafe {
                     let array =
                         PyArray::new_(py, self.raw_dim(), strides.as_ptr(), order.to_flag());
-                    array.copy_ptr(self.as_ptr(), len);
+                    ptr::copy_nonoverlapping(self.as_ptr(), array.data(), len);
                     array
                 }
             }