pchickey edited PR #3393 from pch/async_limiting
to main
.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey edited PR #3393 from pch/async_limiting
to main
:
This PR:
- Moves
ResourceLimiter
's definition fromwasmtime_runtime
towasmtime
- Adds an additional
ResourceLimiterAsync
, which is just like aResourceLimiter
except thememory_growing
andtable_growing
methods areasync
(viaasync_trait
). This is so that async embeddings of wasmtime to yield execution while they await for a resource to be available.- In order for the
ResourceLimiterAsync
methods to await inside memory & table grows, we need to add async variants ofMemory::new
,Memory::grow
,Table::new
, andTable::grow
. These variants are not required for all asyncStore
s, only for stores which are given aStore::limiter_async
. So, this won't break any existing code.I had to restructure a fair bit of the interface between runtime and wasmtime for this PR, including extending the
wasmtime_runtime::Store
trait, and changing the error type passed between them toanyhow::Error
instead ofBox<dyn std::error::Error + Send + Sync>
.Additionally, both
ResourceLimiter
s are now safer: wasmtime_runtime now catches any panics that occur from the user's impl of the traits.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey has marked PR #3393 as ready for review.
pchickey edited PR #3393 from pch/async_limiting
to main
:
This PR:
- Moves
ResourceLimiter
's definition fromwasmtime_runtime
towasmtime
- Adds an additional
ResourceLimiterAsync
, which is just like aResourceLimiter
except thememory_growing
andtable_growing
methods areasync
(viaasync_trait
). This is so that async embeddings of wasmtime to yield execution while they await for a resource to be available.- In order for the
ResourceLimiterAsync
methods to await inside memory & table grows, we need to add async variants ofMemory::new
,Memory::grow
,Table::new
, andTable::grow
. These variants are not required for all asyncStore
s, only for stores which are given aStore::limiter_async
. So, this won't break any existing code.- Both
ResourceLimiter
traits now gettable_grow_failed
, which corresponds tomemory_grow_failed
. Better to just get this over with now than have to add it in later.- Test suite covers table limiting and async and panics now.
I had to restructure a fair bit of the interface between runtime and wasmtime for this PR, including extending the
wasmtime_runtime::Store
trait, and changing the error type passed between them toanyhow::Error
instead ofBox<dyn std::error::Error + Send + Sync>
.Additionally, both
ResourceLimiter
s are now safer: wasmtime_runtime now catches any panics that occur from the user's impl of the traits.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey edited PR #3393 from pch/async_limiting
to main
:
This PR:
- Moves
ResourceLimiter
's definition fromwasmtime_runtime
towasmtime
- Adds an additional
ResourceLimiterAsync
, which is just like aResourceLimiter
except thememory_growing
andtable_growing
methods areasync
(via#[async_trait]
). This is so that async embeddings of wasmtime to yield execution while they await for a resource to be available.- In order for the
ResourceLimiterAsync
methods to await inside memory & table grows, we need to add async variants ofMemory::new
,Memory::grow
,Table::new
, andTable::grow
. These variants are not required for all asyncStore
s, only for stores which are given aStore::limiter_async
. So, this won't break any existing code.- Both
ResourceLimiter
traits now gettable_grow_failed
, which corresponds tomemory_grow_failed
. Better to just get this over with now than have to add it in later.- Test suite covers table limiting and async and panics now.
I had to restructure a fair bit of the interface between runtime and wasmtime for this PR, including extending the
wasmtime_runtime::Store
trait, and changing the error type passed between them toanyhow::Error
instead ofBox<dyn std::error::Error + Send + Sync>
.Additionally, both
ResourceLimiter
s are now safer: wasmtime_runtime now catches any panics that occur from the user's impl of the traits.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
pchickey edited PR #3393 from pch/async_limiting
to main
:
This PR:
- Moves
ResourceLimiter
's definition fromwasmtime_runtime
towasmtime
- Adds an additional
ResourceLimiterAsync
, which is just like aResourceLimiter
except thememory_growing
andtable_growing
methods areasync
(via#[async_trait]
). This is so that async embeddings of wasmtime can handle memory or table growing byawait
ing for a resource to be available.- In order for the
ResourceLimiterAsync
methods to await inside memory & table grows, we need to add async variants ofMemory::new
,Memory::grow
,Table::new
, andTable::grow
. These variants are not required for all asyncStore
s, only for stores which are given aStore::limiter_async
. So, this won't break any existing code.- Both
ResourceLimiter
traits now gettable_grow_failed
, which corresponds tomemory_grow_failed
. Better to just get this over with now than have to add it in later.- Test suite covers table limiting and async and panics now.
I had to restructure a fair bit of the interface between runtime and wasmtime for this PR, including extending the
wasmtime_runtime::Store
trait, and changing the error type passed between them toanyhow::Error
instead ofBox<dyn std::error::Error + Send + Sync>
.Additionally, both
ResourceLimiter
s are now safer: wasmtime_runtime now catches any panics that occur from the user's impl of the traits.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I'd be a little uneasy actually using setjmp to sail past a
catch_unwind
frame, so could this be transmitted through the return value and outside of this closure theraise_user_trap
happens?
alexcrichton submitted PR review.
alexcrichton created PR review comment:
I think the indentation here may be a bit off?
alexcrichton created PR review comment:
Could this and the panic test below be updated to grow through wasm? I think that's the more interesting path to test b/c otherwise there's already only Rust code on the stack which didn't necessitate adding a
catch_unwind
or anything like that.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey submitted PR review.
pchickey created PR review comment:
I didn't think of that, thanks! Easy fix
pchickey submitted PR review.
pchickey created PR review comment:
Ah yeah rustfmt gave up because its in a macro.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey requested alexcrichton for a review on PR #3393.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey updated PR #3393 from pch/async_limiting
to main
.
alexcrichton submitted PR review.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey updated PR #3393 from pch/async_limiting
to main
.
pchickey updated PR #3393 from pch/async_limiting
to main
.
alexcrichton submitted PR review.
pchickey merged PR #3393.
Last updated: Dec 23 2024 at 12:05 UTC