eust-dfinity opened PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
Could you elaborate a bit on your use case for this? I would sort of expect that to override everything about allocation you'd have imported memories, but it sounds like your use case is also covering situations where the host wants to instantiate modules that define their own memory, but the host still wants to configure how those memories are allocated.
alexcrichton created PR Review Comment:
FWIW this is making the
wasmtime_runtime
crate a public dependency of thewasmtime
crate. We've previously avoided doing this because thewasmtime_*
family of crate are intended to be far more lower level than thewasmtime
crate.That being said we haven't quite had a vector of customization like this before where completely dynamic behavior is needed. I suspect we're going to have these grow over time, but I figured it's worth pointing out that this has a few consequences for the
wasmtime
crate:
This primarily means that if
wasmtime_runtime
has a major version bump, we have to bump the major version ofwasmtime
as well. It's expected thatwasmtime_runtime
will have lots of major version bumps because it's just a bunch of internals. Forwasmtime
, however, I think the idea is that eventually we reach a steady state where we just release new patch/minor versions.This is also relatively difficult to use since you'll have to match up, manually, with the version of
wasmtime_runtime
thatwasmtime
is using.All that is to say that if we can avoid exposing this degree of a public dependency I think we should. One way, for example, would be to define traits in
wasmtime
as well as inwasmtime_runtime
. Thewasmtime
crate would then provide its own implementation of the traits inwasmtime_runtime
for provided types. This is a bit wonky, however, from our perspective and may not be the most trivial to manage.
alexcrichton created PR Review Comment:
The term "Allocator" feels a bit general here, would it be possible to tweak the naming here to indicate that this isn't like a malloc/free allocator but rather an allocator of linear memories?
alexcrichton created PR Review Comment:
In terms of safety something here needs to be
unsafe
due to the trickiness to get this all right (and segfaults which happen if there's a bug). I think all traits introduced here (bothAllocator
andLinearMemory
) will need to beunsafe trait
declarations since they have a number of safety guarantees to uphold in their implementations.
eust-dfinity submitted PR Review.
eust-dfinity created PR Review Comment:
Our use case is in fact to control all memories used in modules, not just imported ones. Basically we monitor memory usage and track memory changes. Possible reallocations which may happen in wasmtime right now make it more difficult.
eust-dfinity submitted PR Review.
eust-dfinity created PR Review Comment:
Thank you for the info. I was meaning to ask about it as well.
I'll try to move things around, but first I'd rather come to a consensus on the general picture of how the whole thing should work internally.There is also a question of MemoryPlan which resides in environ now.
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
alexcrichton submitted PR Review.
alexcrichton submitted PR Review.
alexcrichton created PR Review Comment:
How come this is wrapped in a
RefCell
?
alexcrichton created PR Review Comment:
I think it's safe to remove this and have
as_ptr
return*mut u8
since all linear memories need to support interior mutability anyway.
alexcrichton created PR Review Comment:
It might be good to leave a statement here too saying that this is a pretty new and experimental feature and it's recommended to be familiar with the code before using it. I'm not confident personally at least that this is an exhaustive set of what you need to guarantee for usage of this to be safe.
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
eust-dfinity submitted PR Review.
eust-dfinity created PR Review Comment:
This is complementary to:
https://github.com/bytecodealliance/wasmtime/blob/a325b62ade47540b4fe528d0c98e3c7dc1bfc042/crates/runtime/src/memory.rs#L147
Basically it's here because RuntimeLinearMemory::vmmemory() takes &self and not &mut self, but needs to return mut ptr.
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
eust-dfinity updated PR #1400 from host_memory
to master
:
As per discussion on host managed memory on zulip started by @lostman here is a simple implementation.
To make things least invasive I ended up with two traits - one for allocator and one for actual memory. This way LinearMemory is not forced to know anything about the allocator and consequently does not need to inherit restrictions placed on the allocator (e.g. Sync).
Additionally DefaultAllocator is made public, so it's possible to create a wrapper around the existing allocator/memory (insert some hooks) and supply it as a custom one.
There are no specific test cases for custom allocators. Can be added if required.
Implementation itself is tested within existing tests with DefaultAllocator.@alexcrichton @tschneidereit @peterhuene please have a look
alexcrichton merged PR #1400.
Last updated: Dec 23 2024 at 12:05 UTC