Stream: git-wasmtime

Topic: wasmtime / PR #2741 Split out fiber stacks from fibers.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 01:46):

peterhuene opened PR #2741 from refactor-fiber-stacks to main:

This PR splits out a FiberStack from Fiber, allowing the instance
allocator trait to return FiberStack rather than raw stack pointers. This
keeps the stack creation mostly in wasmtime_fiber, but now the on-demand
instance allocator can make use of it.

The instance allocators no longer have to return a "not supported" error to
indicate that the store should allocate its own fiber stack.

This includes a bunch of cleanup in the instance allocator to scope stacks to
the new "async" feature in the runtime.

Closes #2708.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 01:49):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 02:06):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 02:32):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 02:45):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 03:21):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 06:23):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 06:57):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton created PR Review Comment:

Could this have an assert that the stack is not owned? (e.g. that the stack was created by allocate above)

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton created PR Review Comment:

Not to say this is too much #[cfg] that we should remove this, just pointing out the original motivation in case you felt like this is too much #[cfg].

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton created PR Review Comment:

FWIW I've seen a lot of #[cfg] annotations being added here like this, but the main purpose of an optional feature in wasmtime was to provide the ability to avoid compiling the wasmtime-fiber crate since it has custom assembly stubs that don't work everywhere. For a crate like wasmtime-runtime, though, including functions which are never used is only a marginal hit to compile time so I don't think it's necessarily required that we propagate the async feature everywhere.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton created PR Review Comment:

Aha I now see though that this is because wasmtime-fiber was picked up as a dependency.

In that case I think it may be worthwhile to basically just avoid the conditional parameters of stack sizes, and otherwise leaving these annotations as-is by avoiding compiling in a StackPool which references the wasmtime-fiber types.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 15:33):

alexcrichton created PR Review Comment:

FWIW I think it's fine to have an extra field like this that isn't conditional, it does add a fair amount of boilerplate to use #[cfg] and we're not necessarily getting a whole lot out of it.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 18:33):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 18:33):

peterhuene created PR Review Comment:

I can add it back in as I might have gotten carried away with cutting out anything having to do with "execution stacks" from the runtime without async. It definitely added a lot of cfg to get right and it clutters up the code.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 19:51):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 20:56):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 19 2021 at 21:48):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 20 2021 at 07:06):

peterhuene updated PR #2741 from refactor-fiber-stacks to main.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 20 2021 at 07:07):

peterhuene requested alexcrichton for a review on PR #2741.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 22 2021 at 14:09):

alexcrichton submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 22 2021 at 18:05):

peterhuene merged PR #2741.

Last updated: Dec 23 2024 at 12:05 UTC