-
Notifications
You must be signed in to change notification settings - Fork 272
Rust: Provide a trait method to (optionally) control resource allocation #1625
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| include!(env!("BINDINGS")); | ||
|
|
||
| use crate::test::arena_allocated_resources::to_test::Thing; | ||
|
|
||
| struct Component; | ||
|
|
||
| export!(Component); | ||
|
|
||
| impl Guest for Component { | ||
| fn run() { | ||
| let thing1 = Thing::new(3); | ||
| let thing2 = Thing::new(5); | ||
| assert_eq!(3, thing1.get()); | ||
| assert_eq!(5, thing2.get()); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| include!(env!("BINDINGS")); | ||
|
|
||
| use crate::exports::test::arena_allocated_resources::to_test::{Guest, GuestThing}; | ||
|
|
||
| export!(Component); | ||
|
|
||
| struct Component; | ||
|
|
||
| impl Guest for Component { | ||
| type Thing = MyThing; | ||
| } | ||
|
|
||
| mod arena { | ||
|
|
||
| use core::{cell::UnsafeCell, mem::MaybeUninit}; | ||
|
|
||
| /// A simple no_std arena allocator for fixed-size allocations. | ||
| /// | ||
| /// The arena allocates items of type T sequentially from a pre-allocated buffer | ||
| /// and does not support individual deallocation. Memory is reclaimed | ||
| /// only when the entire arena is reset. | ||
| pub struct Arena<T, const SIZE: usize> { | ||
| buffer: [MaybeUninit<T>; SIZE], | ||
| offset: usize, | ||
| } | ||
|
|
||
| impl<T, const SIZE: usize> Arena<T, SIZE> { | ||
| /// Allocates space for a single item of type T. | ||
| /// Returns a mutable reference to the allocated memory, or None if there's insufficient space. | ||
| pub fn alloc_one(&mut self) -> Option<&mut T> { | ||
| if self.offset < SIZE { | ||
| let ptr = self.buffer[self.offset].as_mut_ptr(); | ||
| self.offset += 1; | ||
| Some(unsafe { &mut *ptr }) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// A static-safe wrapper for Arena that uses interior mutability. | ||
| /// | ||
| /// This allows an Arena to be stored in a static variable and accessed safely | ||
| /// in single-threaded contexts without requiring std or alloc. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// This type is safe to use in single-threaded environments. In multi-threaded | ||
| /// contexts, external synchronization is required. | ||
| pub struct StaticArena<T, const SIZE: usize> { | ||
| arena: UnsafeCell<Arena<T, SIZE>>, | ||
| } | ||
|
|
||
| // SAFETY: StaticArena is Sync because we enforce single-threaded access through | ||
| // the API. It can be safely shared across threads as long as only one thread | ||
| // accesses it at a time (which is the responsibility of the user). | ||
| unsafe impl<T, const SIZE: usize> Sync for StaticArena<T, SIZE> where T: Sync {} | ||
|
|
||
| // SAFETY: StaticArena is Send because the underlying Arena can be moved between | ||
| // threads, and T itself must be Send. | ||
| unsafe impl<T, const SIZE: usize> Send for StaticArena<T, SIZE> where T: Send {} | ||
|
|
||
| impl<T, const SIZE: usize> StaticArena<T, SIZE> { | ||
| /// Creates a new static arena. | ||
| pub const fn new() -> Self { | ||
| StaticArena { | ||
| arena: UnsafeCell::new(Arena { | ||
| buffer: [const { MaybeUninit::uninit() }; SIZE], | ||
| offset: 0, | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| /// Gets mutable access to the arena. | ||
| /// | ||
| /// # Safety | ||
| /// | ||
| /// This is safe in single-threaded contexts. In multi-threaded contexts, | ||
| /// the caller must ensure exclusive access. | ||
| #[inline] | ||
| pub fn get_mut(&self) -> &mut Arena<T, SIZE> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW this is fundamentally unsafe and more-or-less UB. This'll want to be protected by some sort of lock or similar. For this test though I think it's ok to avoid being fancy and just manually call
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Turning I didn't look that closely into this third re-implementation of the arena, my goal was to create a minimalist wasm component without any reference to malloc - which involves avoiding panic and libc, but can be achieved by wat-patching. I guess I should look a bit more closely into how to make the arena throwaway example code opsem safe and not an evil example buried in a high-quality code base.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Such a thing (&self to &mut T) exists ... https://doc.rust-lang.org/core/cell/struct.UnsafeCell.html#method.as_mut_unchecked but I agree that the unsafety should become better capsuled. |
||
| unsafe { &mut *self.arena.get() } | ||
| } | ||
|
|
||
| /// Allocates a single item. | ||
| pub fn alloc_one(&self) -> Option<&mut T> { | ||
| self.get_mut().alloc_one() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| use arena::StaticArena; | ||
|
|
||
| #[derive(Clone)] | ||
| struct MyThing { | ||
| contents: u32, | ||
| } | ||
|
|
||
| static ARENA: StaticArena<Option<MyThing>, 4> = StaticArena::new(); | ||
|
|
||
| impl GuestThing for MyThing { | ||
| fn new(v: u32) -> MyThing { | ||
| MyThing { contents: v } | ||
| } | ||
| fn get(&self) -> u32 { | ||
| self.contents | ||
| } | ||
| fn _resource_into_raw(val: Option<Self>) -> *mut Option<Self> | ||
| where | ||
| Self: Sized, | ||
| { | ||
| val.and_then(|v| { | ||
| ARENA.alloc_one().map(|x| { | ||
| *x = Some(v); | ||
| x as *mut _ | ||
| }) | ||
| }) | ||
| .unwrap_or(core::ptr::null_mut()) | ||
| } | ||
| unsafe fn _resource_from_raw(handle: *mut Option<Self>) -> Option<Self> | ||
| where | ||
| Self: Sized, | ||
| { | ||
| let res = unsafe { &mut *handle }.take(); | ||
| res | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package test:arena-allocated-resources; | ||
|
|
||
| interface to-test { | ||
| resource thing { | ||
| constructor(v: u32); | ||
| get: func() -> u32; | ||
| } | ||
| } | ||
|
|
||
| world test { | ||
| export to-test; | ||
| } | ||
|
|
||
| world runner { | ||
| import to-test; | ||
|
|
||
| export run: func(); | ||
| } |
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.
There's a few aspects of this I'd like to bikeshed if that's ok with you as well. One is that I'd rather not open-code the
Option<Self>implementation if possible. Given how everything's moving to atrait-based definition, do you think it'd be possible to have, for example, a default associated type or something like that which represents the container here? Something liketype RawContainer: SomeOtherTrait = Option<Self>;or something like that, whereOption<T>by default implementsSomeOtherTrait. That wouldn't be directly related to the allocation here, and would involve abstracting the preexisting.as_mut().unwrap()operations in a few places, but it'd be in the same trend as this.Orthogonally I'd bikeshed these exact shapes/docs a bit. The
_-prefixed methods are probably overkill at this point. It might make sense to rename them to maybe a_-suffix rather than a_prefix (which typically means "yes I know this is unused ignore that" which isn't the case here). The implementations would stay the same, however. The documentation, though, I think will want to be expanded to explain a bit more about the allocation lifecycle and what's expected of the pointer (e.g. it lives for the entire duration of the resource). I thinkresource_into_rawwill need to beunsafeas well because one of the contractual invariants ofresource_from_rawwill be that it only consumes values previously produced byresource_into_raw. Finally, thewhere Self: SizedI think is fine to move to the trait definition (makeSizeda supertrait) and avoid the syntactic overhead here.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.
I agree with hiding the
Option<T>implementation detail by an associated type or similar (I think there exists anFooImptype or similar already for this). I am not sure about creating another trait here (feels overkill to me for now).I also agree on the
_suffix and the better documentation and Sized in a different place.But .. I consider the argumentation about
into_rawbetter beunsafenot compelling. There is no invariant oninto_raw, onlyfrom_rawhas invariants (only called once and only on a result frominto_raw). Similarly only one of these member functions is unsafe on Box as well.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.
That sounds reasonable to me, yeah, and good points!