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.

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

Weak ref support #11

Open
coolreader18 opened this issue Jan 13, 2021 · 2 comments
Open

Weak ref support #11

coolreader18 opened this issue Jan 13, 2021 · 2 comments

Comments

@coolreader18
Copy link

coolreader18 commented Jan 13, 2021

I'm looking at how feasible this would be to use in RustPython in order to get cyclic gc support. Overall I'm really impressed with this implementation, though one thing I'm wondering about is whether it would be possible to add weakref support to the Cc/ThreadedCc types. What I was thinking of as a user-facing API is:

// Safety: weaklist() should always return the same weaklist for a given object
// `unsafe`ness may not be necessary depending on internal implementation
unsafe trait WeakReferenceable {
    type Error; // so that it can be Infallible if Self can always be referenced
    fn weaklist(&self) -> Result<&gcmodule::WeakList<Self>, Self::Error>; // like tp_weaklistoffset
    type Weak: WeakRef;
}
unsafe trait WeakRef {
    // maybe this should be like `Linked`?
}
struct WeakList<T: WeakReferenceable> { head: T::Weak } // maybe?

Ideally, there would be an easy way to have the actual WeakCc type be a specific kind of Cc as python does it, but I'm not sure of whether that might be unnecessary overhead for other use cases or what that API would look like. Maybe just WeakRef: From<Cc<WeakReferenceable>>?

This is just from a day or so of mulling over how best to implement weaklist in RustPython based on looking at CPython code, so maybe you have a better idea of how to do. Again, I'm not sure how feasible it is/how willing you would be to actually implement the machinery behind this.

Edit: also, looking more at gcmodule's internals, it would be amazing if AbstractObjectSpace could be publically exposed so that RustPython can e.g. directly embed [Threaded]RefCount into PyObject, and generally use the gc machinery at a lower level.

@quark-zju
Copy link
Owner

quark-zju commented Jan 13, 2021

Hi, thanks for your interest. I'm definitely interested in supporting the RustPython usecase.

For weakref, it is a useful feature so it should be in this library. For its implementation, I prefer simpler and not having unsafe if possible. I think the Rust std::rc::Rc does a good job. Its API surface is simple: Rc::downgrade(&self) -> Weak and Weak::upgrade(&self) -> Option<Rc>. There is no unsafe and the space overhead is usize instead of a list. Since it's not hard to implement and verify, I pushed changes adding it. Let me know if it satisfies the weakref usecase. If so, I can publish a new version.

For AbstractObjectSpace, if I understand correctly, you want something like RawCcBox(T) where T embeds RefCount instead of RawCcBox(RefCount, T). In that case, I don't think it's as simple as utilizing AbstractObjectSpace internals. One important reason that T is separate from RefCount is because we need to be able to drop T while keeping the metadata (RefCount) alive. If you want to have read access to the ref counts (EDIT: Just added APIs for them), APIs can be added for that but moving RefCount to T in RawCcBox seems quite complicated and error-prone.

@coolreader18
Copy link
Author

I'll check it out, thanks! I might fork it and experiment with a method on Cc to decrement refcount with a custom drop function, since we're running into UB with our object layout when dropping objects (RustPython/RustPython#2381) and it's tricky to fix that without a performance hit or having a function like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants