GeezerVison is showing github.com — (READY FOR REVIEW)Garbage collect: A stop-the-world cycle collector by discord9 · Pull Request #4180 · RustPython/RustPython
(READY FOR REVIEW)Garbage collect: A stop-the-world cycle collector
#4180
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.py
(3)
.rs
(45)
.toml
(4)
.yaml
(1)
All 4 file types selected
Only manifest files
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -15,7 +15,7 @@ concurrency:
cancel-in-progress: true
env:
CARGO_ARGS: --no-default-features --features stdlib,zlib,importlib,encodings,ssl,jit
CARGO_ARGS: --no-default-features --features stdlib,zlib,importlib,encodings,ssl,jit,gc_bacon
NON_WASM_PACKAGES: >-
-p rustpython-common
-p rustpython-compiler-core
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -57,7 +57,8 @@ unicode_names2 = { version = "0.6.0", git = "https://github.com/youknowone/unico
widestring = "0.5.1"
[features]
default = ["threading", "stdlib", "zlib", "importlib", "encodings", "rustpython-parser/lalrpop"]
default = ["threading", "stdlib", "zlib", "importlib", "encodings", "rustpython-parser/lalrpop", "gc_bacon"]
gc_bacon = ["rustpython-vm/gc_bacon", "rustpython-stdlib/gc"]
importlib = ["rustpython-vm/importlib"]
encodings = ["rustpython-vm/encodings"]
stdlib = ["rustpython-stdlib", "rustpython-pylib"]
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -656,8 +656,6 @@ def test_dict_update(self):
dict.update(od, [('spam', 1)])
self.assertNotIn('NULL', repr(od))
# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_reference_loop(self):
# Issue 25935
OrderedDict = self.OrderedDict
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -69,7 +69,13 @@ def trace_call(self, frame):
def trace_return(self, frame):
self.add_event('return', frame)
self.stack.pop()
# TODO: RUSTPYTHON
# it seems pop from empty list is also related to those failed tests
# if those tests(all the tests in `ProfileHookTestCase``) can pass in RustPython,
# then we can remove this `if`
# and just use `self.stack.pop()` here
if len(self.stack)!=0:
Comment thread
discord9 marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
self.stack.pop()
def trace_exception(self, frame):
self.testcase.fail(
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -75,9 +75,10 @@ def setUp(self):
def callback(self, ref):
self.cbcalled += 1
# TODO: RUSTPYTHON, cpython's period is 0.0001, but at least now using such a small gc period is too slow
# so change to 0.001 for now
@contextlib.contextmanager
def collect_in_thread(period=0.0001 ):
def collect_in_thread(period=0.001 ):
"""
Ensure GC collections happen in a different thread, at a high frequency.
"""
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -18,6 +18,7 @@ mod pyclass;
mod pymodule;
mod pypayload;
mod pystructseq;
mod pytrace;
use error::{extract_spans, Diagnostic};
use proc_macro2::TokenStream;
Expand Down
Expand Up
@@ -77,3 +78,7 @@ pub fn py_freeze(input: TokenStream, compiler: &dyn Compiler) -> TokenStream {
pub fn pypayload(input: DeriveInput) -> TokenStream {
result_to_tokens(pypayload::impl_pypayload(input))
}
pub fn pytrace(attr: AttributeArgs, item: DeriveInput) -> TokenStream {
result_to_tokens(pytrace::impl_pytrace(attr, item))
}
Original file line number
Diff line number
Diff line change
@@ -1,6 +1,6 @@
use super::Diagnostic;
use crate::util::{
format_doc, pyclass_ident_and_attrs, text_signature, ClassItemMeta, ContentItem,
format_doc, path_eq, pyclass_ident_and_attrs, text_signature, ClassItemMeta, ContentItem,
ContentItemInner, ErrorVec, ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta,
ALL_ALLOWED_NAMES,
};
Expand Down
Expand Up
@@ -413,8 +413,33 @@ pub(crate) fn impl_pyclass(attr: AttributeArgs, item: Item) -> Result<TokenStrea
attrs,
)?;
// try to know if it have a `#[pyclass(trace)]` or a `#[pytrace] on this struct
let is_trace =
{ class_meta.is_trace()? || attrs.iter().any(|attr| path_eq(&attr.path, "pytrace")) };
let maybe_trace = {
if is_trace {
quote! {
#[cfg(feature = "gc_bacon")]
impl ::rustpython_vm::object::gc::MaybeTrace for #ident {
const IS_TRACE: bool = true;
fn try_trace(&self, tracer_fn: &mut ::rustpython_vm::object::gc::TracerFn) {
::rustpython_vm::object::gc::Trace::trace(self, tracer_fn);
}
}
}
} else {
// a dummy impl, which do nothing
// #attrs
quote! {
#[cfg(feature = "gc_bacon")]
impl ::rustpython_vm::object::gc::MaybeTrace for #ident { }
}
}
};
let ret = quote! {
#item
#maybe_trace
#class_def
};
Ok(ret)
Expand Down
Original file line number
Diff line number
Diff line change
@@ -0,0 +1,70 @@
use proc_macro2::TokenStream;
use quote::quote;
use syn::{AttributeArgs, DeriveInput, Result};
/// also remove `#[notrace]` attr, and not trace corresponding field
fn gen_trace_code(item: &mut DeriveInput) -> Result<TokenStream> {
match &mut item.data {
syn::Data::Struct(s) => {
let fields = &mut s.fields;
if let syn::Fields::Named(ref mut fields) = fields {
let res: TokenStream = fields
.named
.iter_mut()
.map(|f| {
let name = f
.ident
.as_ref()
.expect("Field should have a name in non-tuple struct");
let mut do_trace = true;
f.attrs.retain(|attr| {
// remove #[notrace] and not trace this specifed field
if attr.path.segments.last().unwrap().ident == "notrace" {
do_trace = false;
false
} else {
true
}
});
if do_trace {
quote!(
::rustpython_vm::object::gc::Trace::trace(&self.#name, tracer_fn);
)
} else {
quote!()
}
})
.collect();
Ok(res)
} else {
panic!("Expect only Named fields")
}
}
syn::Data::Enum(_) => todo!(),
syn::Data::Union(_) => todo!(),
}
}
pub(crate) fn impl_pytrace(attr: AttributeArgs, mut item: DeriveInput) -> Result<TokenStream> {
if !attr.is_empty() {
panic!(
"pytrace macro expect no attr(s), found {} attr(s)",
attr.len()
);
}
let trace_code = gen_trace_code(&mut item)?;
let ty = &item.ident;
let ret = quote! {
#item
#[cfg(feature = "gc_bacon")]
unsafe impl ::rustpython_vm::object::gc::Trace for #ty {
fn trace(&self, tracer_fn: &mut ::rustpython_vm::object::gc::TracerFn) {
#trace_code
}
}
};
Ok(ret)
}
Original file line number
Diff line number
Diff line change
Expand Up
@@ -3,7 +3,8 @@ use proc_macro2::{Span, TokenStream};
use quote::{quote, ToTokens};
use std::collections::{HashMap, HashSet};
use syn::{
spanned::Spanned, Attribute, Ident, Meta, MetaList, NestedMeta, Result, Signature, UseTree,
spanned::Spanned, Attribute, Ident, Meta, MetaList, NestedMeta, Path, Result, Signature,
UseTree,
};
use syn_ext::{
ext::{AttributeExt as SynAttributeExt, *},
Expand All
@@ -25,6 +26,10 @@ pub(crate) const ALL_ALLOWED_NAMES: &[&str] = &[
"pymember",
];
pub(crate) fn path_eq(path: &Path, s: &str) -> bool {
path.get_ident().map_or(false, |id| id == s)
}
#[derive(Clone)]
struct NurseryItem {
attr_name: Ident,
Expand Down
Expand Up
@@ -178,6 +183,20 @@ impl ItemMetaInner {
Ok(value)
}
pub fn _exist_path(&self, key: &str) -> Result<bool> {
if let Some((_, meta)) = self.meta_map.get(key) {
match meta {
Meta::Path(p) => Ok(path_eq(p, key)),
other => Err(syn::Error::new_spanned(
other,
format!("#[{}({})] is expected", self.meta_name(), key),
)),
}
} else {
Ok(false)
}
}
pub fn _bool(&self, key: &str) -> Result<bool> {
let value = if let Some((_, meta)) = self.meta_map.get(key) {
match meta {
Expand Down
Expand Up
@@ -264,7 +283,7 @@ pub(crate) struct ClassItemMeta(ItemMetaInner);
impl ItemMeta for ClassItemMeta {
const ALLOWED_NAMES: &'static [&'static str] =
&["module", "name", "base", "metaclass", "unhashable"];
&["module", "name", "base", "metaclass", "unhashable", "trace" ];
fn from_inner(inner: ItemMetaInner) -> Self {
Self(inner)
Expand Down
Expand Up
@@ -308,6 +327,10 @@ impl ClassItemMeta {
self.inner()._optional_str("metaclass")
}
pub fn is_trace(&self) -> Result<bool> {
self.inner()._exist_path("trace")
}
pub fn module(&self) -> Result<Option<String>> {
const KEY: &str = "module";
let inner = self.inner();
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -91,3 +91,20 @@ pub fn pypayload(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input);
derive_impl::pypayload(input).into()
}
/// use on struct with named fields like `struct A{x:i32, y:i32}` to impl `Trace` for datatype
///
/// use `#[notrace]` on fields you wish not to trace
///
/// add `trace` attr to `#[pyclass]` to make it
/// traceable(Even from type-erased PyObject)(i.e. write `#[pyclass(trace)]`)
/// better to place after `#[pyclass]` so pyclass know `pytrace`'s existance and impl a MaybeTrace calling Trace
#[proc_macro_attribute]
Comment thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more .
This could probably be a normal derive() macro, and then you wouldn't have to deal with removing the notrace attr
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more .
This could probably be a normal derive() macro, and then you wouldn't have to deal with removing the notrace attr
Yes, #[notrace] is only used four time in th following, still considering whether or not to remove it and hand write Trace:
#[pyclass(module = false, name = "enumerate")]
#[derive(Debug)]
#[pytrace]
pub struct PyEnumerate {
#[notrace]
counter: PyRwLock<BigInt>,
iterator: PyIter,
}
#[pyclass(module = false, name = "iterator")]
#[derive(Debug)]
#[pytrace]
pub struct PySequenceIterator {
// cached sequence methods
#[notrace]
seq_methods: &'static PySequenceMethods,
internal: PyMutex<PositionIterInternal<PyObjectRef>>,
}
#[pyclass(module = false, name = "zip")]
#[derive(Debug)]
#[pytrace]
pub struct PyZip {
iterators: Vec<PyIter>,
#[notrace]
strict: PyAtomic<bool>,
}
#[derive(Debug, Clone)]
#[pytrace]
pub struct ArgMapping {
obj: PyObjectRef,
#[notrace]
methods: &'static PyMappingMethods,
}
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
pub fn pytrace(
attr: proc_macro::TokenStream,
item: proc_macro::TokenStream,
) -> proc_macro::TokenStream {
let attr = parse_macro_input!(attr);
let item = parse_macro_input!(item);
derive_impl::pytrace(attr, item).into()
}
Original file line number
Diff line number
Diff line change
Expand Up
@@ -7,6 +7,8 @@ edition = "2021"
[features]
threading = ["rustpython-common/threading", "rustpython-vm/threading"]
gc = ["gc_bacon"]
gc_bacon = []
zlib = ["libz-sys", "flate2/zlib"]
bz2 = ["bzip2"]
ssl = ["openssl", "openssl-sys", "foreign-types-shared"]
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -6,22 +6,52 @@ mod gc {
#[pyfunction]
fn collect(_args: FuncArgs, _vm: &VirtualMachine) -> i32 {
0
#[cfg(feature = "gc_bacon")]
{
usize::from(rustpython_vm::object::gc::collect()) as i32
}
#[cfg(not(feature = "gc_bacon"))]
{
0
}
}
#[pyfunction]
fn isenabled(_args: FuncArgs, _vm: &VirtualMachine) -> bool {
false
#[cfg(feature = "gc_bacon")]
{
rustpython_vm::object::gc::isenabled()
}
#[cfg(not(feature = "gc_bacon"))]
{
false
}
}
#[pyfunction]
fn enable(_args: FuncArgs, vm: &VirtualMachine) -> PyResult {
Err(vm.new_not_implemented_error("".to_owned()))
#[cfg(feature = "gc_bacon")]
{
rustpython_vm::object::gc::enable();
Ok(vm.new_pyobj(true))
}
#[cfg(not(feature = "gc_bacon"))]
{
Err(vm.new_not_implemented_error("".to_owned()))
}
}
#[pyfunction]
fn disable(_args: FuncArgs, vm: &VirtualMachine) -> PyResult {
Err(vm.new_not_implemented_error("".to_owned()))
#[cfg(feature = "gc_bacon")]
{
rustpython_vm::object::gc::disable();
Ok(vm.new_pyobj(true))
}
#[cfg(not(feature = "gc_bacon"))]
{
Err(vm.new_not_implemented_error("".to_owned()))
}
}
#[pyfunction]
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -10,6 +10,7 @@ include = ["src/**/*.rs", "Cargo.toml", "build.rs", "Lib/**/*.py"]
[features]
default = ["compiler"]
gc_bacon = []
importlib = []
encodings = ["importlib"]
vm-tracing-logging = []
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -34,6 +34,7 @@ pub type DictContentType = dictdatatype::Dict;
#[pyclass(module = false, name = "dict", unhashable = true)]
#[derive(Default)]
#[pytrace]
pub struct PyDict {
entries: DictContentType,
}
Expand Down
Original file line number
Diff line number
Diff line change
@@ -1,7 +1,7 @@
use super::{
IterStatus, PositionIterInternal, PyGenericAlias, PyIntRef, PyTupleRef, PyType, PyTypeRef,
};
use crate::common::lock::{PyMutex, PyRwLock} ;
use crate::common::lock::PyRwLock;
use crate::{
class::PyClassImpl,
convert::ToPyObject,
Expand All
@@ -12,10 +12,13 @@ use crate::{
};
use num_bigint::BigInt;
use num_traits::Zero;
use rustpython_common::lock::PyMutex;
#[pyclass(module = false, name = "enumerate")]
#[derive(Debug)]
#[pytrace]
pub struct PyEnumerate {
#[notrace]
counter: PyRwLock<BigInt>,
iterator: PyIter,
}
Expand Down
Expand Up
@@ -86,6 +89,7 @@ impl IterNext for PyEnumerate {
#[pyclass(module = false, name = "reversed")]
#[derive(Debug)]
#[pytrace]
pub struct PyReverseSequenceIterator {
internal: PyMutex<PositionIterInternal<PyObjectRef>>,
}
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -8,6 +8,7 @@ use crate::{
#[pyclass(module = false, name = "filter")]
#[derive(Debug)]
#[pytrace]
pub struct PyFilter {
predicate: PyObjectRef,
iterator: PyIter,
Expand Down
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.