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

qcell + selfref? #37

Closed
SoniEx2 opened this issue Aug 16, 2022 · 7 comments
Closed

qcell + selfref? #37

SoniEx2 opened this issue Aug 16, 2022 · 7 comments

Comments

@SoniEx2
Copy link
Contributor

SoniEx2 commented Aug 16, 2022

would it be possible to have a cell type which is similar to LCell, but is based around the selfref crate?

(maybe with generativity? we're not familiar with that crate)

this is related to #30 but more general

specifically we want a thread-safe lock-free &mut SelfRefCellEnvironment which opens a reusable LCell-like environment.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Aug 16, 2022

#30 basically explains what we have, but the idea would be to somehow end up with:

struct MyCells<'id, 'this> {
  x: LCell<'id, Foo>,
  y: Cell<Option<&'this LCell<'id, dyn Bar>>>,
  ...
}

or something similar (yeah the Cell<Option<T>> wrapping is a bit awkward but it's fine really. the main point is that this would be fully zero-cost, unlike the current solution with QCells.)

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Aug 19, 2022

we're playing around with this idea, and it's not going well so far. :/

#![feature(generic_associated_types)]

use qcell::{LCell, LCellOwner};
use selfref::Holder;

use std::pin::Pin;

pub struct SRCellEnvironment<'k, T: selfref::Opaque + 'k> {
    inner: Holder<'k, T>,
}

impl<'k, T: selfref::Opaque> SRCellEnvironment<'k, T> {
    pub fn new_with<F: selfref::NewWith<'k, T>>(f: F) -> Self
    where
        T::Kind<'k>: Sized
    {
        Self {
            inner: Holder::new_with(f),
        }
    }
}

impl<'k, T: selfref::Opaque> SRCellEnvironment<'k, T> {
    pub fn operate_in<'i, F, R>(self: Pin<&'i mut Self>, f: F) -> R
    where
        F: OperateIn<'k, T, Out = R>
    {
        struct OperateInWrapper<F>(F);
        impl<'k, T, F> selfref::OperateIn<'k, T> for OperateInWrapper<F>
        where F: OperateIn<'k, T>, T: selfref::Opaque + 'k {
            type Out = F::Out;
            fn operate_in<'a>(self, x: Pin<&'a T::Kind<'a>>) -> Self::Out where 'k: 'a {
                let OperateInWrapper(this) = self;
                // SAFETY? we're actually unsure if this is sound!
                let guard = unsafe {
                    generativity::Guard::new(generativity::Id::new())
                };
                this.operate_in(LCellOwnerWrapper(LCellOwner::new(guard)), x)
            }
        } 
        unsafe {
            self.map_unchecked_mut(|this| &mut this.inner)
        }.into_ref().operate_in(OperateInWrapper(f))
    }
}

pub trait OperateIn<'k, T: selfref::Opaque + 'k> {
    /// The value returned by this operation.
    type Out;
    /// Do this operation.
    fn operate_in<'a>(self, owner: LCellOwnerWrapper<'a>, x: Pin<&'a T::Kind<'a>>) -> Self::Out where 'k: 'a;
}

pub struct LCellOwnerWrapper<'a>(LCellOwner<'a>);

impl<'a> Drop for LCellOwnerWrapper<'a> {
    #[inline]
    fn drop(&mut self) {
    }
}

impl<'id> LCellOwnerWrapper<'id> {
    pub fn cell<T>(&self, value: T) -> LCell<'id, T> {
        self.0.cell(value)
    }
    pub fn ro<'a, T: ?Sized>(&'a self, lc: &'a LCell<'id, T>) -> &'a T {
        self.0.ro(lc)
    }
    pub fn rw<'a, T: ?Sized>(&'a mut self, lc: &'a LCell<'id, T>) -> &'a mut T {
        self.0.rw(lc)
    }
}

#[test]
fn test() {
    use std::cell::Cell;
    use std::marker::PhantomData;
    trait Foo {
        fn update(&mut self);
    }
    trait Bar {
        fn get(&self) -> String;
    }
    struct MyThing;
    impl Foo for MyThing {
        fn update(&mut self) {
            println!("hello world!");
        }
    }
    impl Bar for MyThing {
        fn get(&self) -> String {
            return String::from("hello");
        }
    }
    struct IntrusiveLCell<'a, T: ?Sized> {
        y: Cell<Option<&'a LCell<'a, dyn Bar>>>,
        x: LCell<'a, T>,
    }
    struct IntrusiveLCellKey<T: ?Sized>(PhantomData<T>);
    // SAFETY: T cannot refer to 'a so there's no unsoundness there. also the
    // struct is a pretty basic self-ref struct with no Drop impl or glue which
    // would otherwise make this unsound.
    unsafe impl<T: ?Sized> selfref::Opaque for IntrusiveLCellKey<T> {
        type Kind<'a> = IntrusiveLCell<'a, T>;
    }

    let mut holder = Box::pin(
        SRCellEnvironment::<IntrusiveLCellKey<MyThing>>::new_with(
            selfref::new_with_closure::<IntrusiveLCellKey<MyThing>, _>(|_| {
                IntrusiveLCell {
                    x: LCell::new(MyThing),
                    y: Cell::new(None),
                }
            })
        )
    );

    struct operate_in;
    impl<'k> OperateIn<'k, IntrusiveLCellKey<MyThing>> for operate_in {
        type Out = ();
        fn operate_in<'a>(self, mut owner: LCellOwnerWrapper<'a>, x: Pin<&'a IntrusiveLCell<'a, MyThing>>) where 'k: 'a {
            x.y.set(Some(&x.get_ref().x));
            owner.rw(&x.x).update();
            println!("{}", owner.ro(x.y.get().unwrap()).get());
        }
    }

    holder.as_mut().operate_in(operate_in);

    let mut holder_dyn: Pin<Box<SRCellEnvironment<IntrusiveLCellKey<dyn Foo>>>> = holder;
}

(the Drop for LCellOwnerWrapper is probably important. who knows. maybe this whole thing is broken. ah well.)

@uazu
Copy link
Owner

uazu commented Aug 19, 2022

I still think it would be better to deconstruct the fat pointer and store a pointer and several vtable pointers and reconstruct the fat pointer on request. I included some links in issue #30. Okay it is more low-level, and perhaps needs more thought to ensure soundness. But trying to do too much in the type system is just adding complexity. I still can't quite get my head around the selfref API and implementation, although if I work on it more probably I will understand. I would go directly for what you need at a byte level (e.g. pointer to boxed data and a set of vtable pointers), and then build a safe abstraction around that. I know you're trying to build on existing abstractions, but it seems to me that it's just making things harder.

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Aug 19, 2022

it seems we hit rust-lang/rust#75899 aka rust-lang/rust#50213 so it's not our fault we ran into issues!

but yeah we basically want something fully zero-cost! QCell is not zero-cost enough! LCell on the other hand is supposed to get optimized out.

nevertheless, this is far more flexible than #30, and also far more flexible than "just going directly for the vtable pointers". tho it's certainly still suboptimal for just storing the vtable pointers, but aside from that it lets you have LCells and move them (or, well, a Pin<Box<_>> to them) around. (like, the owner is carried with the LCells, instead of having to setup a single-use scope. note that both LCellOwner::scope and generativity only support single-use scopes, while this selfref hack is meant to be reusable.)

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Sep 17, 2022

so this works with unsizing now huh. :p

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Jan 14, 2023

we've published a crate, no idea if it's sound, but feel free to play with it. https://users.rust-lang.org/t/soundness-review-request-srce-0-1/87587

@SoniEx2
Copy link
Contributor Author

SoniEx2 commented Jan 27, 2023

we've rolled this up into selfref so we guess we can close this now

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