Optimize nth and nth_back for BoundListIterator

newsfragments/4810.added.md

@@ -0,0 +1 @@
+Optimizes `nth` and `nth_back` for `BoundListIterator`

noxfile.py

@@ -351,7 +351,7 @@ def test_emscripten(session: nox.Session):
         session,
         "bash",
         "-c",
-        f"source {info.builddir/'emsdk/emsdk_env.sh'} && cargo test",
+        f"source {info.builddir / 'emsdk/emsdk_env.sh'} && cargo test",
     )
 
 
@@ -951,7 +951,7 @@ def set(
             f"""\
 implementation={implementation}
 version={version}
-build_flags={','.join(build_flags)}
+build_flags={",".join(build_flags)}
 suppress_build_script_link_lines=true
 """
         )

pyo3-benches/benches/bench_list.rs

@@ -39,7 +39,33 @@ fn list_get_item(b: &mut Bencher<'_>) {
     });
 }
 
-#[cfg(not(any(Py_LIMITED_API, Py_GIL_DISABLED)))]
+fn list_nth(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        const LEN: usize = 50;
+        let list = PyList::new_bound(py, 0..LEN);
+        let mut sum = 0;
+        b.iter(|| {
+            for i in 0..LEN {
+                sum += list.iter().nth(i).unwrap().extract::<usize>().unwrap();
+            }
+        });
+    });
+}
+
+fn list_nth_back(b: &mut Bencher<'_>) {
+    Python::with_gil(|py| {
+        const LEN: usize = 50;
+        let list = PyList::new_bound(py, 0..LEN);
+        let mut sum = 0;
+        b.iter(|| {
+            for i in 0..LEN {
+                sum += list.iter().nth_back(i).unwrap().extract::<usize>().unwrap();
+            }
+        });
+    });
+}
+
+#[cfg(not(Py_LIMITED_API))]
 fn list_get_item_unchecked(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 50_000;
@@ -66,6 +92,8 @@ fn sequence_from_list(b: &mut Bencher<'_>) {
 fn criterion_benchmark(c: &mut Criterion) {
     c.bench_function("iter_list", iter_list);
     c.bench_function("list_new", list_new);
+    c.bench_function("list_nth", list_nth);
+    c.bench_function("list_nth_back", list_nth_back);
     c.bench_function("list_get_item", list_get_item);
     #[cfg(not(any(Py_LIMITED_API, Py_GIL_DISABLED)))]
     c.bench_function("list_get_item_unchecked", list_get_item_unchecked);

src/types/list.rs

@@ -1,5 +1,3 @@
-use std::iter::FusedIterator;
-
 use crate::err::{self, PyResult};
 use crate::ffi::{self, Py_ssize_t};
 use crate::ffi_ptr_ext::FfiPtrExt;
@@ -8,6 +6,7 @@ use crate::types::{PySequence, PyTuple};
 use crate::{
     Borrowed, Bound, BoundObject, IntoPyObject, IntoPyObjectExt, PyAny, PyErr, PyObject, Python,
 };
+use std::iter::FusedIterator;
 
 use crate::types::any::PyAnyMethods;
 use crate::types::sequence::PySequenceMethods;
@@ -547,6 +546,31 @@ impl<'py> BoundListIterator<'py> {
         }
     }
 
+    /// # Safety
+    ///
+    /// On the free-threaded build, caller must verify they have exclusive
+    /// access to the list by holding a lock or by holding the innermost
+    /// critical section on the list.
+    #[inline]
+    #[cfg(not(Py_LIMITED_API))]
+    #[deny(unsafe_op_in_unsafe_fn)]
+    unsafe fn nth_unchecked(
+        index: &mut Index,
+        length: &mut Length,
+        list: &Bound<'py, PyList>,
+        n: usize,
+    ) -> Option<Bound<'py, PyAny>> {
+        let length = length.0.min(list.len());
+        let target_index = index.0 + n;
+        if index.0 + n < length {
+            let item = unsafe { list.get_item_unchecked(target_index) };
+            index.0 = target_index + 1;
+            Some(item)
+        } else {
+            None
+        }
+    }
+
     /// # Safety
     ///
     /// On the free-threaded build, caller must verify they have exclusive
@@ -589,6 +613,31 @@ impl<'py> BoundListIterator<'py> {
         }
     }
 
+    /// # Safety
+    ///
+    /// On the free-threaded build, caller must verify they have exclusive
+    /// access to the list by holding a lock or by holding the innermost
+    /// critical section on the list.
+    #[inline]
+    #[cfg(not(Py_LIMITED_API))]
+    #[deny(unsafe_op_in_unsafe_fn)]
+    unsafe fn nth_back_unchecked(
+        index: &mut Index,
+        length: &mut Length,
+        list: &Bound<'py, PyList>,
+        n: usize,
+    ) -> Option<Bound<'py, PyAny>> {
+        let length_size = length.0.min(list.len());
+        if index.0 + n < length_size {
+            let target_index = length_size - n - 1;
+            let item = unsafe { list.get_item_unchecked(target_index) };
+            *length = Length(target_index);
+            Some(item)
+        } else {
+            None
+        }
+    }
+
     #[cfg(not(Py_LIMITED_API))]
     fn with_critical_section<R>(
         &mut self,
@@ -625,6 +674,14 @@ impl<'py> Iterator for BoundListIterator<'py> {
         }
     }
 
+    #[inline]
+    #[cfg(all(Py_GIL_DISABLED, not(feature = "nightly")))]
+    fn nth(&mut self, n: usize) -> Option<Self::Item> {
+        self.with_critical_section(|index, length, list| unsafe {
+            Self::nth_unchecked(index, length, list, n)
+        })
+    }
+
     #[inline]
     fn size_hint(&self) -> (usize, Option<usize>) {
         let len = self.len();
@@ -750,6 +807,27 @@ impl<'py> Iterator for BoundListIterator<'py> {
             None
         })
     }
+
+    #[inline]
+    #[cfg(all(Py_GIL_DISABLED, feature = "nightly"))]
+    fn advance_by(&mut self, n: usize) -> Result<(), NonZero<usize>> {
+        self.with_critical_section(|index, length, list| {
+            let length = length.0.min(list.len());
+            let target_index = index.0 + n;
+            if index.0 + n < length {
+                let item = list.get_item(target_index);
+                match item {
+                    Ok(_) => {
+                        index.0 = target_index;
+                        Ok(())
+                    }
+                    Err(_) => Err(NonZero::new(n - index.0)),
+                }
+            } else {
+                Err(NonZero::new(n - index.0))
+            }
+        })
+    }
 }
 
 impl DoubleEndedIterator for BoundListIterator<'_> {
@@ -772,6 +850,14 @@ impl DoubleEndedIterator for BoundListIterator<'_> {
         }
     }
 
+    #[inline]
+    #[cfg(all(Py_GIL_DISABLED, not(feature = "nightly")))]
+    fn nth_back(&mut self, n: usize) -> Option<Self::Item> {
+        self.with_critical_section(|index, length, list| unsafe {
+            Self::nth_back_unchecked(index, length, list, n)
+        })
+    }
+
     #[inline]
     #[cfg(all(Py_GIL_DISABLED, not(feature = "nightly")))]
     fn rfold<B, F>(mut self, init: B, mut f: F) -> B
@@ -1502,4 +1588,104 @@ mod tests {
             assert!(tuple.eq(tuple_expected).unwrap());
         })
     }
+
+    #[test]
+    fn test_iter_nth() {
+        Python::with_gil(|py| {
+            let v = vec![6, 7, 8, 9, 10];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            iter.next();
+            assert_eq!(iter.nth(1).unwrap().extract::<i32>().unwrap(), 8);
+            assert_eq!(iter.nth(1).unwrap().extract::<i32>().unwrap(), 10);
+            assert!(iter.nth(1).is_none());
+
+            let v: Vec<i32> = vec![];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            iter.next();
+            assert!(iter.nth(1).is_none());
+
+            let v = vec![1, 2, 3];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            assert!(iter.nth(10).is_none());
+
+            let v = vec![6, 7, 8, 9, 10];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+            let mut iter = list.iter();
+            assert_eq!(iter.next().unwrap().extract::<i32>().unwrap(), 6);
+            assert_eq!(iter.nth(2).unwrap().extract::<i32>().unwrap(), 9);
+            assert_eq!(iter.next().unwrap().extract::<i32>().unwrap(), 10);
+
+            let mut iter = list.iter();
+            iter.nth_back(1);
+            assert_eq!(iter.nth(2).unwrap().extract::<i32>().unwrap(), 8);
+            assert!(iter.next().is_none());
+        });
+    }
+
+    #[test]
+    fn test_iter_nth_back() {
+        Python::with_gil(|py| {
+            let v = vec![1, 2, 3, 4, 5];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            assert_eq!(iter.nth_back(0).unwrap().extract::<i32>().unwrap(), 5);
+            assert_eq!(iter.nth_back(1).unwrap().extract::<i32>().unwrap(), 3);
+            assert!(iter.nth_back(2).is_none());
+
+            let v: Vec<i32> = vec![];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            assert!(iter.nth_back(0).is_none());
+            assert!(iter.nth_back(1).is_none());
+
+            let v = vec![1, 2, 3];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            assert!(iter.nth_back(5).is_none());
+
+            let v = vec![1, 2, 3, 4, 5];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            iter.next_back(); // Consume the last element
+            assert_eq!(iter.nth_back(1).unwrap().extract::<i32>().unwrap(), 3);
+            assert_eq!(iter.next_back().unwrap().extract::<i32>().unwrap(), 2);
+            assert_eq!(iter.nth_back(0).unwrap().extract::<i32>().unwrap(), 1);
+
+            let v = vec![1, 2, 3, 4, 5];
+            let ob = (&v).into_pyobject(py).unwrap();
+            let list = ob.downcast::<PyList>().unwrap();
+
+            let mut iter = list.iter();
+            assert_eq!(iter.nth_back(1).unwrap().extract::<i32>().unwrap(), 4);
+            assert_eq!(iter.nth_back(2).unwrap().extract::<i32>().unwrap(), 1);
+
+            let mut iter2 = list.iter();
+            iter2.next_back();
+            assert_eq!(iter2.nth_back(1).unwrap().extract::<i32>().unwrap(), 3);
+            assert_eq!(iter2.next_back().unwrap().extract::<i32>().unwrap(), 2);
+
+            let mut iter3 = list.iter();
+            iter3.nth(1);
+            assert_eq!(iter3.nth_back(2).unwrap().extract::<i32>().unwrap(), 3);
+            assert!(iter3.nth_back(0).is_none());
+        });
+    }
 }