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

Add Support for Dynamically-Sized-Types #121

Open
cbrownsey opened this issue Feb 4, 2025 · 1 comment
Open

Add Support for Dynamically-Sized-Types #121

cbrownsey opened this issue Feb 4, 2025 · 1 comment

Comments

@cbrownsey
Copy link

cbrownsey commented Feb 4, 2025

I've been engaged over the past few months in an exercise in Yak-shaving, wherein I re-implemented this crate in a couple of different ways, before finally deciding to just contribute to the already existing and much higher quality solution. The main feature that I need is the allocation of proper DSTs, which this crate doesn't support.
Although in the end my solution to this problem was to use nightly to get access to the ptr_metadata feature, one of the interceding versions simply polyfilled that feature in order to be able to allocate a subset of unsized types in a Gc.

The whole point of this is to store a function which computes a layout in the CollectVtable instead of storing a static layout, so as to allow truly dynamically sized Gc pointers.

Proposed API

The additional API surface, and changes to existing structs, is very very roughly as follows. And although I'm not hugely familiar with this code base, I don't see anywhere obvious which is antithetical to this approach.

trait MetaSized {
    type Metadata: Sized + Copy;

    fn layout_from_meta(meta: Self::Metadata) -> Result<Layout, LayoutError>;

    fn into_parts(this: *const Self) -> (*const (), Self::Metadata);
    fn from_parts(ptr: *const (), meta: Self::Metadata) -> *const Self;
    fn into_parts_mut(this: *mut Self) -> (*mut (), Self::Metadata);
    fn from_parts_mut(ptr: *mut (), meta: Self::Metadata) -> *mut Self;
}

impl<T> MetaSized for T { ... }
impl<T> MetaSized for [T] { ... }
impl MetaSized for str { ... }

// The additional type parameter here is to enable the pattern of reading a sized `GcBoxInner<(), T::Metadata>`, 
// extracting the metadata field, and then using that metadata to read a proper unsized `GcBoxInner<T>`.
struct GcBoxInner<T: ?Sized + MetaSized, M = T::Metadata> {
    pub(crate) header: GcBoxHeader,
    //+ Added field
    /// The metadata for the value stored in this `GcBox`.
    pub(crate) metadata: M,
    /// The typed value stored in this `GcBox`.
    pub(crate) value: mem::ManuallyDrop<T>,
}

impl<T: ?Sized + MetaSized> GcBoxInner<T> {
    // `GcBoxInner` cannot implement MetaSized due to overlapping impls.
    fn layout_from_meta(meta: T::Metadata) -> Result<Layout, LayoutError> { ... }

    fn into_parts(this: *const Self) -> (*const (), T::Metadata) { ... }
    unsafe fn from_parts(ptr: *const (), meta: T::Metadata) -> *const Self { ... }
    fn into_parts_mut(this: *mut Self) -> (*mut (), T::Metadata) { ... }
    unsafe fn from_parts_mut(ptr: *mut (), meta: T::Metadata) -> *mut Self { ... }
}

#[repr(align(16))]
struct CollectVtable {
    //- Removed field
    // box_layout: Layout,
    /// Computes the layout of the `GcBox` the GC'd value is stored in.
    layout_fn: unsafe fn(GcBox) -> Layout,
    /// Drops the value stored in the given `GcBox` (without deallocating the box).
    drop_value: unsafe fn(GcBox),
    /// Traces the value stored in the given `GcBox`.
    trace_value: unsafe fn(GcBox, &mut Context),
}

Downsides

There are of course downsides; it adds indirection where there previously was none; it is a fair amount of complexity to add; it's not truly general for all unsized types, just those which implement a crate-private trait; and I'm sure there are more. On the other hand, for all Sized types, this actually frees up 8 bytes in the CollectVtable without adding any data to the GcBoxInner, the other upsides I believe far outweigh the costs, and I'm happy to implement it myself if this seems up to snuff.

Playground with enough code to convince me that this is doable on stable, and frankly probably more than is necessary.

@cbrownsey
Copy link
Author

After tinkering around in the codebase for a little while, this proposal is a non-starter as written, as it infects every unsized bound with the MetaSized trait, and thus excludes dyn objects.
This can be reworked, by defining GcBoxInner as follows

struct GcBoxInner<T: ?Sized, M = ()> {
    pub(crate) metadata: M,
    pub(crate) header: GcBoxHeader,
    pub(crate) value: mem::ManuallyDrop<T>,
}

and then having GcBox point to the second field of a valid GcBoxInner<T, T::Metadata>. Since GcBoxInner is #[repr(C)], this means it is valid to read a GcBox as pointing to the first field of a GcBoxInner<T, ()>. The offset from this pointer to the metadata field can be statically calculated, with no information about the layout of T needed.

I also believe that this change also makes it possible to define a ThinGc pointer type, which could be a worthwhile addition.

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

1 participant