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.

Sign up for GitHub

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

Use NonNull in ReleasePool and add bench_dict #274

Merged
merged 3 commits into from
Nov 15, 2018
Merged

Use NonNull in ReleasePool and add bench_dict #274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d442e5
Use NonNull in release pool
kngwyu Nov 14, 2018
6222232
Add bench_dict
kngwyu Nov 14, 2018
a73bd06
Replace NonNull::new_unchecked with new().unwrap() in test
kngwyu Nov 15, 2018
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
21 changes: 21 additions & 0 deletions benches/bench_dict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#![feature(test)]
extern crate pyo3;
extern crate test;
use test::Bencher;

use pyo3::{prelude::*, types::IntoPyDict};

#[bench]
fn iter_dict(b: &mut Bencher) {
let gil = Python::acquire_gil();
let py = gil.python();
const LEN: usize = 1_000_000;
let dict = (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict(py);
let mut sum = 0;
b.iter(|| {
for (k, _v) in dict.iter() {
let i: u64 = k.extract().unwrap();
sum += i;
}
});
}
6 changes: 6 additions & 0 deletions src/object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ impl PyObject {
PyObject(ptr)
}

pub(crate) unsafe fn into_nonnull(self) -> NonNull<ffi::PyObject> {
let res = self.0;
std::mem::forget(self); // Avoid Drop
res
}

/// Creates a `PyObject` instance for the given FFI pointer.
/// This moves ownership over the pointer into the `PyObject`.
/// Undefined behavior if the pointer is NULL or invalid.
Expand Down
102 changes: 51 additions & 51 deletions src/python.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::types::{PyDict, PyModule, PyObjectRef, PyType};
use std;
use std::ffi::CString;
use std::marker::PhantomData;
use std::ptr::NonNull;
use std::os::raw::c_int;

/// Marker type that indicates that the GIL is currently held.
Expand Down Expand Up @@ -308,7 +309,7 @@ impl<'p> Python<'p> {
{
let p;
unsafe {
p = pythonrun::register_owned(self, obj.into_ptr());
p = pythonrun::register_owned(self, obj.into_nonnull());
}
<T as PyTryFrom>::try_from(p)
}
Expand All @@ -318,17 +319,15 @@ impl<'p> Python<'p> {
where
T: PyTypeInfo,
{
let p = pythonrun::register_owned(self, obj.into_ptr());
let p = pythonrun::register_owned(self, obj.into_nonnull());
self.unchecked_downcast(p)
}

/// Register `ffi::PyObject` pointer in release pool

pub unsafe fn from_borrowed_ptr_to_obj(self, ptr: *mut ffi::PyObject) -> &'p PyObjectRef {
if ptr.is_null() {
crate::err::panic_after_error();
} else {
pythonrun::register_borrowed(self, ptr)
match NonNull::new(ptr) {
Some(p) => pythonrun::register_borrowed(self, p),
None => crate::err::panic_after_error(),
}
}

Expand All @@ -339,11 +338,12 @@ impl<'p> Python<'p> {
where
T: PyTypeInfo,
{
if ptr.is_null() {
crate::err::panic_after_error();
} else {
let p = pythonrun::register_owned(self, ptr);
self.unchecked_downcast(p)
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_owned(self, p);
self.unchecked_downcast(p)
}
None => crate::err::panic_after_error(),
}
}

Expand All @@ -353,56 +353,58 @@ impl<'p> Python<'p> {
where
T: PyTypeInfo,
{
if ptr.is_null() {
crate::err::panic_after_error();
} else {
let p = pythonrun::register_owned(self, ptr);
self.unchecked_mut_downcast(p)
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_owned(self, p);
self.unchecked_mut_downcast(p)
}
None => crate::err::panic_after_error(),
}
}

/// Register owned `ffi::PyObject` pointer in release pool.
/// Returns `Err(PyErr)` if the pointer is `null`.
/// do unchecked downcast to specific type.

pub unsafe fn from_owned_ptr_or_err<T>(self, ptr: *mut ffi::PyObject) -> PyResult<&'p T>
where
T: PyTypeInfo,
{
if ptr.is_null() {
Err(PyErr::fetch(self))
} else {
let p = pythonrun::register_owned(self, ptr);
Ok(self.unchecked_downcast(p))
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_owned(self, p);
Ok(self.unchecked_downcast(p))
}
None => Err(PyErr::fetch(self)),
}
}

/// Register owned `ffi::PyObject` pointer in release pool.
/// Returns `None` if the pointer is `null`.
/// do unchecked downcast to specific type.

pub unsafe fn from_owned_ptr_or_opt<T>(self, ptr: *mut ffi::PyObject) -> Option<&'p T>
where
T: PyTypeInfo,
{
if ptr.is_null() {
None
} else {
let p = pythonrun::register_owned(self, ptr);
Some(self.unchecked_downcast(p))
}
NonNull::new(ptr).map(|p| {
let p = pythonrun::register_owned(self, p);
self.unchecked_downcast(p)
})
}

/// Register borrowed `ffi::PyObject` pointer in release pool.
/// Panics if the pointer is `null`.
/// do unchecked downcast to specific type.

pub unsafe fn from_borrowed_ptr<T>(self, ptr: *mut ffi::PyObject) -> &'p T
where
T: PyTypeInfo,
{
let p = pythonrun::register_borrowed(self, ptr);
self.unchecked_downcast(p)
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_borrowed(self, p);
self.unchecked_downcast(p)
}
None => crate::err::panic_after_error(),
}
}

/// Register borrowed `ffi::PyObject` pointer in release pool.
Expand All @@ -412,44 +414,42 @@ impl<'p> Python<'p> {
where
T: PyTypeInfo,
{
if ptr.is_null() {
crate::err::panic_after_error();
} else {
let p = pythonrun::register_borrowed(self, ptr);
self.unchecked_mut_downcast(p)
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_borrowed(self, p);
self.unchecked_mut_downcast(p)
}
None => crate::err::panic_after_error(),
}
}

/// Register borrowed `ffi::PyObject` pointer in release pool.
/// Returns `Err(PyErr)` if the pointer is `null`.
/// do unchecked downcast to specific type.

pub unsafe fn from_borrowed_ptr_or_err<T>(self, ptr: *mut ffi::PyObject) -> PyResult<&'p T>
where
T: PyTypeInfo,
{
if ptr.is_null() {
Err(PyErr::fetch(self))
} else {
let p = pythonrun::register_borrowed(self, ptr);
Ok(self.unchecked_downcast(p))
match NonNull::new(ptr) {
Some(p) => {
let p = pythonrun::register_borrowed(self, p);
Ok(self.unchecked_downcast(p))
}
None => Err(PyErr::fetch(self)),
}
}

/// Register borrowed `ffi::PyObject` pointer in release pool.
/// Returns `None` if the pointer is `null`.
/// do unchecked downcast to specific `T`.

pub unsafe fn from_borrowed_ptr_or_opt<T>(self, ptr: *mut ffi::PyObject) -> Option<&'p T>
where
T: PyTypeInfo,
{
if ptr.is_null() {
None
} else {
let p = pythonrun::register_borrowed(self, ptr);
Some(self.unchecked_downcast(p))
}
NonNull::new(ptr).map(|p| {
let p = pythonrun::register_borrowed(self, p);
self.unchecked_downcast(p)
})
}

#[doc(hidden)]
Expand Down
41 changes: 21 additions & 20 deletions src/pythonrun.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,11 @@ impl Drop for GILGuard {

/// Release pool
struct ReleasePool {
owned: Vec<*mut ffi::PyObject>,
borrowed: Vec<*mut ffi::PyObject>,
pointers: *mut Vec<*mut ffi::PyObject>,
owned: Vec<NonNull<ffi::PyObject>>,
borrowed: Vec<NonNull<ffi::PyObject>>,
pointers: *mut Vec<NonNull<ffi::PyObject>>,
obj: Vec<Box<any::Any>>,
p: spin::Mutex<*mut Vec<*mut ffi::PyObject>>,
p: spin::Mutex<*mut Vec<NonNull<ffi::PyObject>>>,
}

impl ReleasePool {
Expand All @@ -131,7 +131,7 @@ impl ReleasePool {

// vec of pointers
let ptr = *v;
let vec: &'static mut Vec<*mut ffi::PyObject> = &mut *ptr;
let vec: &'static mut Vec<_> = &mut *ptr;
if vec.is_empty() {
return;
}
Expand All @@ -143,7 +143,7 @@ impl ReleasePool {

// release py objects
for ptr in vec.iter_mut() {
ffi::Py_DECREF(*ptr);
ffi::Py_DECREF(ptr.as_ptr());
}
vec.set_len(0);
}
Expand All @@ -152,7 +152,7 @@ impl ReleasePool {
let len = self.owned.len();
if owned < len {
for ptr in &mut self.owned[owned..len] {
ffi::Py_DECREF(*ptr);
ffi::Py_DECREF(ptr.as_ptr());
}
self.owned.set_len(owned);
}
Expand Down Expand Up @@ -235,20 +235,20 @@ pub unsafe fn register_pointer(obj: NonNull<ffi::PyObject>) {
let pool: &'static mut ReleasePool = &mut *POOL;

let mut v = pool.p.lock();
let pool: &'static mut Vec<*mut ffi::PyObject> = &mut *(*v);
pool.push(obj.as_ptr());
let pool: &'static mut Vec<_> = &mut *(*v);
pool.push(obj);
}

pub unsafe fn register_owned(_py: Python, obj: *mut ffi::PyObject) -> &PyObjectRef {
pub unsafe fn register_owned(_py: Python, obj: NonNull<ffi::PyObject>) -> &PyObjectRef {
let pool: &'static mut ReleasePool = &mut *POOL;
pool.owned.push(obj);
&*(&pool.owned[pool.owned.len() - 1] as *const *mut ffi::PyObject as *const PyObjectRef)
&*(&pool.owned[pool.owned.len() - 1] as *const _ as *const PyObjectRef)
}

pub unsafe fn register_borrowed(_py: Python, obj: *mut ffi::PyObject) -> &PyObjectRef {
pub unsafe fn register_borrowed(_py: Python, obj: NonNull<ffi::PyObject>) -> &PyObjectRef {
let pool: &'static mut ReleasePool = &mut *POOL;
pool.borrowed.push(obj);
&*(&pool.borrowed[pool.borrowed.len() - 1] as *const *mut ffi::PyObject as *const PyObjectRef)
&*(&pool.borrowed[pool.borrowed.len() - 1] as *const _ as *const PyObjectRef)
}

impl GILGuard {
Expand Down Expand Up @@ -280,7 +280,7 @@ impl GILGuard {

#[cfg(test)]
mod test {
use super::{GILPool, ReleasePool, POOL};
use super::{GILPool, ReleasePool, POOL, NonNull};
use crate::conversion::ToPyObject;
use crate::object::PyObject;
use crate::python::{Python, ToPyPointer};
Expand Down Expand Up @@ -311,7 +311,7 @@ mod test {

empty = ffi::PyTuple_New(0);
cnt = ffi::Py_REFCNT(empty) - 1;
let _ = pythonrun::register_owned(py, empty);
let _ = pythonrun::register_owned(py, NonNull::new(empty).unwrap());

assert_eq!(p.owned.len(), 1);
}
Expand Down Expand Up @@ -341,14 +341,15 @@ mod test {
// empty tuple is singleton
empty = ffi::PyTuple_New(0);
cnt = ffi::Py_REFCNT(empty) - 1;
let _ = pythonrun::register_owned(py, empty);

let _ = pythonrun::register_owned(py, NonNull::new(empty).unwrap());

assert_eq!(p.owned.len(), 1);

{
let _pool = GILPool::new();
let empty = ffi::PyTuple_New(0);
let _ = pythonrun::register_owned(py, empty);
let _ = pythonrun::register_owned(py, NonNull::new(empty).unwrap());
assert_eq!(p.owned.len(), 2);
}
assert_eq!(p.owned.len(), 1);
Expand Down Expand Up @@ -376,7 +377,7 @@ mod test {
assert_eq!(p.borrowed.len(), 0);

cnt = ffi::Py_REFCNT(obj_ptr);
pythonrun::register_borrowed(py, obj_ptr);
pythonrun::register_borrowed(py, NonNull::new(obj_ptr).unwrap());

assert_eq!(p.borrowed.len(), 1);
assert_eq!(ffi::Py_REFCNT(obj_ptr), cnt);
Expand Down Expand Up @@ -405,15 +406,15 @@ mod test {
assert_eq!(p.borrowed.len(), 0);

cnt = ffi::Py_REFCNT(obj_ptr);
pythonrun::register_borrowed(py, obj_ptr);
pythonrun::register_borrowed(py, NonNull::new(obj_ptr).unwrap());

assert_eq!(p.borrowed.len(), 1);
assert_eq!(ffi::Py_REFCNT(obj_ptr), cnt);

{
let _pool = GILPool::new();
assert_eq!(p.borrowed.len(), 1);
pythonrun::register_borrowed(py, obj_ptr);
pythonrun::register_borrowed(py, NonNull::new(obj_ptr).unwrap());
assert_eq!(p.borrowed.len(), 2);
}

Expand Down