rockwotj opened PR #7209 from rockwotj:custom-stack-memory
to bytecodealliance:main
:
Introduces an API to allow embedders to allocate stacks for fibers using custom memory instead of mmap.
rockwotj requested wasmtime-core-reviewers for a review on PR #7209.
rockwotj requested wasmtime-default-reviewers for a review on PR #7209.
rockwotj requested alexcrichton for a review on PR #7209.
rockwotj updated PR #7209.
rockwotj updated PR #7209.
rockwotj updated PR #7209.
rockwotj updated PR #7209.
rockwotj submitted PR review.
rockwotj submitted PR review.
rockwotj created PR review comment:
This is a much safer API, because we don't need to assume they will sent
env
andfinalizer
to nullptr if they don't need them.
rockwotj updated PR #7209.
rockwotj updated PR #7209.
fitzgen submitted PR review:
(Going to leave the C API bits to Alex to review)
fitzgen submitted PR review:
(Going to leave the C API bits to Alex to review)
fitzgen created PR review comment:
I think I like the
mmap: bool
flag better than repeating everything for bothManual
andMmap
.
fitzgen created PR review comment:
/// This trait is unsafe, as memory safety depends on a proper implementation
fitzgen created PR review comment:
Can remove a level of indentation via
#![...]
to put the attribute on the wholestack_creator
module, rather than defining a submodule inside it:#![cfg(all(not(target_os = "windows"), not(miri)))] use anyhow::{bail, Context}; use std::{ alloc::{GlobalAlloc, Layout, System}, ops::Range, ptr::NonNull, sync::Arc, }; use wasmtime::*;
fitzgen created PR review comment:
Instead of changing the constructor of the
OnDemandInstanceAllocator
depending on whether#[cfg(feature = "async")]
, how about we always leave out the stack creator, but then have a method for configuring it after construction:impl OnDemandInstanceAllocator { #[cfg(feature = "async")] pub fn set_stack_creator(&mut self, stack_creator: Option<Arc<dyn StackCreator>>) { self.stack_creator = stack_creator; } }
This should make construction sites a little bit simpler.
fitzgen created PR review comment:
Can we assert page alignment here?
fitzgen created PR review comment:
And here for both start and end parts of the range
rockwotj submitted PR review.
rockwotj submitted PR review.
rockwotj created PR review comment:
Done thanks
rockwotj created PR review comment:
Reverted that back.
rockwotj created PR review comment:
Sure!
rockwotj created PR review comment:
Done.
rockwotj created PR review comment:
Done.
rockwotj created PR review comment:
Thanks done!
rockwotj updated PR #7209.
rockwotj requested fitzgen for a review on PR #7209.
rockwotj updated PR #7209.
fitzgen submitted PR review:
Nice! I looked through the C API changes in more detail as well, and they look good as well.
Thanks @rockwotj!
rockwotj updated PR #7209.
rockwotj updated PR #7209.
fitzgen submitted PR review.
fitzgen merged PR #7209.
Last updated: Dec 23 2024 at 12:05 UTC