From 4c1a81789223f0af3632b4dacab84c92931c4ce7 Mon Sep 17 00:00:00 2001 From: konstin Date: Sun, 25 Nov 2018 23:59:44 +0100 Subject: [PATCH] Backport #281 --- CHANGELOG.md | 23 +++++-- Cargo.toml | 4 +- benches/bench_dict.rs | 21 ++++++ pyo3-derive-backend/Cargo.toml | 2 +- pyo3cls/Cargo.toml | 4 +- src/instance.rs | 2 +- src/object.rs | 2 +- src/pythonrun.rs | 114 +++++++++++++++++++++++---------- tests/test_dict_iter.rs | 17 +++++ 9 files changed, 141 insertions(+), 48 deletions(-) create mode 100644 benches/bench_dict.rs create mode 100644 tests/test_dict_iter.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index c57f0906d0a..fdb6dbc922f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). -## Unreleased +## [0.5.2] - 2018-11-26 + +### Fixed + + * Fix undeterministic segfaults when creating many objects by kngwyu in [#281](https://github.com/PyO3/pyo3/pull/281) + +## 0.5.1 - 2018-11-24 + +Yanked + +## [0.5.0] - 2018-11-11 ### Added @@ -20,6 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Slowly removing specialization uses * `PyString`, `PyUnicode`, and `PyBytes` no longer have a `data()` method (replaced by `as_bytes()`) and `PyStringData` has been removed. + * The pyobject_extract macro ### Changed * Removes the types from the root module and the prelude. They now live in `pyo3::types` instead. @@ -31,7 +42,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Updated to syn 0.15 * Splitted `PyTypeObject` into `PyTypeObject` without the create method and `PyTypeCreate` with requires `PyObjectAlloc + PyTypeInfo + Sized`. * Ran `cargo edition --fix` which prefixed path with `crate::` for rust 2018 - * Renamed `async` to `pyasync` as async will be a keyword in the 2018 edition. + * Renamed `async` to `pyasync` as async will be a keyword in the 2018 edition. + * Starting to use `NonNull<*mut PyObject>` for Py and PyObject by ijl [#260](https://github.com/PyO3/pyo3/pull/260) ### Fixed @@ -39,10 +51,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Lots of clippy errors * Fix segfault on calling an unknown method on a PyObject * Work around a [bug](https://github.com/rust-lang/rust/issues/55380) in the rust compiler by kngwyu [#252](https://github.com/PyO3/pyo3/pull/252) - -### Removed - - * The pyobject_extract macro + * Fixed a segfault with subclassing pyo3 create classes and using `__class__` by kngwyu [#263](https://github.com/PyO3/pyo3/pull/263) ## [0.4.1] - 2018-08-20 @@ -187,6 +196,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Initial release +[0.5.2]: https://github.com/pyo3/pyo3/compare/v0.5.0...v0.5.2 +[0.5.0]: https://github.com/pyo3/pyo3/compare/v0.4.1...v0.5.0 [0.4.1]: https://github.com/pyo3/pyo3/compare/v0.4.0...v0.4.1 [0.4.0]: https://github.com/pyo3/pyo3/compare/v0.3.2...v0.4.0 [0.3.2]: https://github.com/pyo3/pyo3/compare/v0.3.1...v0.3.2 diff --git a/Cargo.toml b/Cargo.toml index f50d0c34819..ba9fbdb78e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3" -version = "0.5.0-alpha.3" +version = "0.5.2" description = "Bindings to Python interpreter" authors = ["PyO3 Project and Contributors "] readme = "README.md" @@ -22,7 +22,7 @@ codecov = { repository = "PyO3/pyo3", branch = "master", service = "github" } libc = "0.2.43" spin = "0.4.9" num-traits = "0.2.6" -pyo3cls = { path = "pyo3cls", version = "=0.5.0-alpha.3" } +pyo3cls = { path = "pyo3cls", version = "=0.5.2" } mashup = "0.1.9" num-complex = { version = "0.2.1", optional = true } diff --git a/benches/bench_dict.rs b/benches/bench_dict.rs new file mode 100644 index 00000000000..114f38eb0f2 --- /dev/null +++ b/benches/bench_dict.rs @@ -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_00; + 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; + } + }); +} diff --git a/pyo3-derive-backend/Cargo.toml b/pyo3-derive-backend/Cargo.toml index ae4e9763ebb..492b1c984fd 100644 --- a/pyo3-derive-backend/Cargo.toml +++ b/pyo3-derive-backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3-derive-backend" -version = "0.5.0-alpha.3" +version = "0.5.2" description = "Code generation for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] diff --git a/pyo3cls/Cargo.toml b/pyo3cls/Cargo.toml index 1130a1cc4ef..558c82b26dd 100644 --- a/pyo3cls/Cargo.toml +++ b/pyo3cls/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pyo3cls" -version = "0.5.0-alpha.3" +version = "0.5.2" description = "Proc macros for PyO3 package" authors = ["PyO3 Project and Contributors "] keywords = ["pyo3", "python", "cpython", "ffi"] @@ -16,4 +16,4 @@ proc-macro = true quote= "0.6.9" proc-macro2 = "0.4.20" syn = { version = "0.15.15", features = ["full", "extra-traits"] } -pyo3-derive-backend = { path = "../pyo3-derive-backend", version = "=0.5.0-alpha.3" } +pyo3-derive-backend = { path = "../pyo3-derive-backend", version = "=0.5.2" } diff --git a/src/instance.rs b/src/instance.rs index 0da4fd8ab07..ac05a997674 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -298,7 +298,7 @@ impl PartialEq for Py { impl Drop for Py { fn drop(&mut self) { unsafe { - pythonrun::register_pointer(self.0); + pythonrun::register_pointer(self.0.as_ptr()); } } } diff --git a/src/object.rs b/src/object.rs index 694a7f93461..8c71820a3a7 100644 --- a/src/object.rs +++ b/src/object.rs @@ -316,7 +316,7 @@ impl<'a> FromPyObject<'a> for PyObject { impl Drop for PyObject { fn drop(&mut self) { unsafe { - pythonrun::register_pointer(self.0); + pythonrun::register_pointer(self.0.as_ptr()); } } } diff --git a/src/pythonrun.rs b/src/pythonrun.rs index 0fcd00d762f..498f02827e1 100644 --- a/src/pythonrun.rs +++ b/src/pythonrun.rs @@ -1,6 +1,6 @@ // Copyright (c) 2017-present PyO3 Project and Contributors use crate::ffi; -use crate::python::{NonNullPyObject, Python}; +use crate::python::Python; use crate::types::PyObjectRef; use spin; use std::{any, marker, rc, sync}; @@ -107,8 +107,8 @@ impl Drop for GILGuard { /// Release pool struct ReleasePool { - owned: Vec<*mut ffi::PyObject>, - borrowed: Vec<*mut ffi::PyObject>, + owned: ArrayList<*mut ffi::PyObject>, + borrowed: ArrayList<*mut ffi::PyObject>, pointers: *mut Vec<*mut ffi::PyObject>, obj: Vec>, p: spin::Mutex<*mut Vec<*mut ffi::PyObject>>, @@ -117,8 +117,8 @@ struct ReleasePool { impl ReleasePool { fn new() -> ReleasePool { ReleasePool { - owned: Vec::with_capacity(256), - borrowed: Vec::with_capacity(256), + owned: ArrayList::new(), + borrowed: ArrayList::new(), pointers: Box::into_raw(Box::new(Vec::with_capacity(256))), obj: Vec::with_capacity(8), p: spin::Mutex::new(Box::into_raw(Box::new(Vec::with_capacity(256)))), @@ -127,20 +127,16 @@ impl ReleasePool { unsafe fn release_pointers(&mut self) { let mut v = self.p.lock(); - - // vec of pointers - let ptr = *v; - let vec: &'static mut Vec<*mut ffi::PyObject> = &mut *ptr; + let vec = &mut **v; if vec.is_empty() { return; } // switch vectors - *v = self.pointers; - self.pointers = ptr; + std::mem::swap(&mut self.pointers, &mut *v); drop(v); - // release py objects + // release PyObjects for ptr in vec.iter_mut() { ffi::Py_DECREF(*ptr); } @@ -148,18 +144,13 @@ impl ReleasePool { } pub unsafe fn drain(&mut self, owned: usize, borrowed: usize, pointers: bool) { - let len = self.owned.len(); - if owned < len { - for ptr in &mut self.owned[owned..len] { - ffi::Py_DECREF(*ptr); - } - self.owned.set_len(owned); - } - - let len = self.borrowed.len(); - if borrowed < len { - self.borrowed.set_len(borrowed); + // Release owned objects(call decref) + while owned < self.owned.len() { + let last = self.owned.pop_back().unwrap(); + ffi::Py_DECREF(last); } + // Release borrowed objects(don't call decref) + self.borrowed.truncate(borrowed); if pointers { self.release_pointers(); @@ -230,24 +221,19 @@ pub unsafe fn register_any<'p, T: 'static>(obj: T) -> &'p T { .unwrap() } -pub unsafe fn register_pointer(obj: NonNullPyObject) { - 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()); +pub unsafe fn register_pointer(obj: *mut ffi::PyObject) { + let pool = &mut *POOL; + (**pool.p.lock()).push(obj); } pub unsafe fn register_owned(_py: Python, obj: *mut 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) + let pool = &mut *POOL; + &*(pool.owned.push_back(obj) as *const _ as *const PyObjectRef) } pub unsafe fn register_borrowed(_py: Python, obj: *mut 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) + let pool = &mut *POOL; + &*(pool.borrowed.push_back(obj) as *const _ as *const PyObjectRef) } impl GILGuard { @@ -277,6 +263,64 @@ impl GILGuard { } } +use self::array_list::ArrayList; + +mod array_list { + use std::collections::LinkedList; + use std::mem; + + const BLOCK_SIZE: usize = 256; + + /// A container type for Release Pool + /// See #271 for why this is crated + pub(super) struct ArrayList { + inner: LinkedList<[T; BLOCK_SIZE]>, + length: usize, + } + + impl ArrayList { + pub fn new() -> Self { + ArrayList { + inner: LinkedList::new(), + length: 0, + } + } + pub fn push_back(&mut self, item: T) -> &T { + let next_idx = self.next_idx(); + if next_idx == 0 { + self.inner.push_back(unsafe { mem::uninitialized() }); + } + self.inner.back_mut().unwrap()[next_idx] = item; + self.length += 1; + &self.inner.back().unwrap()[next_idx] + } + pub fn pop_back(&mut self) -> Option { + self.length -= 1; + let current_idx = self.next_idx(); + if self.length >= BLOCK_SIZE && current_idx == 0 { + let last_list = self.inner.pop_back()?; + return Some(last_list[0].clone()); + } + self.inner.back().map(|arr| arr[current_idx].clone()) + } + pub fn len(&self) -> usize { + self.length + } + pub fn truncate(&mut self, new_len: usize) { + if self.length <= new_len { + return; + } + while self.inner.len() > (new_len + BLOCK_SIZE - 1) / BLOCK_SIZE { + self.inner.pop_back(); + } + self.length = new_len; + } + fn next_idx(&self) -> usize { + self.length % BLOCK_SIZE + } + } +} + #[cfg(test)] mod test { use super::{GILPool, ReleasePool, POOL}; diff --git a/tests/test_dict_iter.rs b/tests/test_dict_iter.rs new file mode 100644 index 00000000000..f4e36017b02 --- /dev/null +++ b/tests/test_dict_iter.rs @@ -0,0 +1,17 @@ +extern crate pyo3; + +use pyo3::{prelude::*, types::IntoPyDict}; + +#[test] +fn iter_dict_nosegv() { + let gil = Python::acquire_gil(); + let py = gil.python(); + const LEN: usize = 10_000_000; + let dict = (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict(py); + let mut sum = 0; + for (k, _v) in dict.iter() { + let i: u64 = k.extract().unwrap(); + sum += i; + } + assert_eq!(sum, 49999995000000); +}