wasm: introduce custom stack allocator for wasmtime #14161

rockwotj · 2023-10-13T16:05:06Z

Introduce a custom stack allocator that we plugin to wasmtime using the APIs added in bytecodealliance/wasmtime#7209.

See the individual commits on the overall design and implementation details, as well as the tradeoffs of other options.

Backports Required

Release Notes

none

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

Now that we've activated wasmtime's async functionality, a couple of crates have changed. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

rockwotj · 2023-10-13T17:43:18Z

@travisdowns commit eae2763 has the implementation of what we talked about in slack if you're interested in taking a peek (no worries if not)

vbotbuildovich · 2023-10-14T06:01:32Z

ducktape was retried in job https://buildkite.com/redpanda/redpanda/builds/38963#018b2c89-7442-4b9b-af40-0649d207c732

cmake/dependencies.cmake

src/v/wasm/allocator.cc

src/v/wasm/allocator.h

src/v/wasm/wasmtime.cc

dotnwat · 2023-10-18T05:52:46Z

src/v/wasm/wasmtime.cc

+        // stack space left.
+        std::ptrdiff_t stack_left = (&dummy_stack_var) - bounds->bottom;
+        void* stack_ptr = ::alloca(stack_left - max_host_function_stack_usage);
+        // Prevent the alloca from being optimized away by logging the result.


why is this a concern? is there some sort of data dependency analysis that the compiler can't see?

I am not sure? At first I was just calling ::alloca(...) and discarding the result and the stack wasn't overflowing, so I guess clang was optimizing that away or something? Ooh I should also probably make sure there is no tail call optimization or anything too.

By default wasmtime allocates stacks using `mmap`, but there is now an API to override the stack allocator. This is the custom allocator that we will plug into wasmtime. We allocate stacks on demand since they will fit within our 128KB allocation limit that we recommend. These stacks will also have a guard page at the bottom of the stack to protect against stack overflow. We cache stacks (indefinitely) because the call to `mprotect` will break up any transparent huge pages (THP) that have been allocated by the seastar allocator, in an effort to not breakup all these pages all over memory we reuse them aggressively. Another option is to preallocate a 2MB THP on startup and chunk that up for wasmtime stacks, but then that imposes a limit on the number of VMs we can run on a single core, to attempt to prevent that limitation we will allocate them dynamically, as the performance impact should be small anyways for breaking up those pages. There is more as comments in the allocator header as well. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

This plugins a sharded stack allocator into a custom wasmtime stack allocator. The setup and API here is very similar to how the custom wasmtime (linear) data memory is plugged in. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

We want to ensure that our host functions don't blow the stack when executing if a guest uses up a bunch of stack space within the Wasm VM. We do this in our tests by allocating a variable amount on the stack so that it looks to every host function that the guest has used the maximum amount of the stack. Additionally, we only run in this "strict stack mode" in release tests. We don't want to do this in production builds, and in debug builds ASAN throws a fit when doing this. Presumably ASAN complains because it doesn't know we've switched the stack as that happens in Rust land which isn't instrumented with ASAN checks. Honestly this is fine as stack usage in debug mode is wildly different than release mode and we're realistically only using debug mode internally in non production usage. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

rockwotj · 2023-10-18T15:05:51Z

Force push: Address #14161 (comment)

dotnwat

lgtm.

i think it is worth understanding the bit about alloca being optimized away, but other than that ship it.

rockwotj · 2023-10-18T18:38:00Z

i think it is worth understanding the bit about alloca being optimized away

Looks like clang "optimizes" it away: https://godbolt.org/z/eEexvrj4b

Which may or may not be compiler bug... happy to take suggestions on better ways to prevent it from being optimized away (we do want this to work in release mode).

dotnwat · 2023-10-18T18:49:57Z

Which may or may not be compiler bug... happy to take suggestions on better ways to prevent it from being optimized away (we do want this to work in release mode).

Oh i see on closer inspection you want the extra stack space but you have no use for the returned stack_ptr, and that's probably why it is optimized away without the logging.

I do wonder though if it being optimized away is a mirage? For example. If I write ::alloca(100) and ignore the return value, then wouldn't it be a legitimate optimization for the compiler to behave as if alloca never existed and just allocate the extra space?

rockwotj · 2023-10-18T18:54:44Z

I do wonder though if it being optimized away is a mirage? For example. If I write ::alloca(100) and ignore the return value, then wouldn't it be a legitimate optimization for the compiler to behave as if alloca never existed and just allocate the extra space?

I'm not sure I follow - it's probably fine to ignore the alloca if we ignore the return result, but it does change the semantics of the resulting code.

behave as if alloca never existed and just allocate the extra space

huh these seem at odds?

travisdowns · 2023-10-19T16:25:57Z

@rockwotj - it's a bit hard to be precise because alloca is not part of the C++ standard and so it sort of lives in this grey area where the semantics aren't entirely clear since we can't refer solely to the types of side effects that the standard guarantees will occur.

However, it is not surprising to me that the alloca is optimized away and if it were defined in the standard I would expect it be in a way that allows it be optimized away. For example, in the same way that malloc, free, new and delete can be optimized away. The primary side effect of alloca is to allocate space which you can use, with certain restrictions (e.g., freed implicitly when the function returns). Actual manipulation of the stack pointer (whatever that even means) is very far down the list of interesting side effects yet has a large cost to enforce so this is definitely the type of thing that ends up in the "not a guaranteed side effect bucket".

The usual way to work around this stuff is to "escape" the relative value. E.g., escape the alloca pointer, so the compiler no longer knows what went on it with and has to assume it gets used in all the supported ways. We have perf::do_not_optimize(x) which "escapes" a value, it works in this case:

https://godbolt.org/z/W7nK4jGsd

rockwotj requested review from BenPope, a team, emaxerrno and dswang as code owners October 13, 2023 16:05

rockwotj requested review from andrewhsu and removed request for a team October 13, 2023 16:05

github-actions bot added area/build area/redpanda labels Oct 13, 2023

rockwotj force-pushed the stacks branch from f8902c8 to c3d5994 Compare October 13, 2023 16:05

rockwotj added 2 commits October 13, 2023 11:59

cmake: update wasmtime

2ccc1c1

Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

third-party: update rust crates

2f2b992

Now that we've activated wasmtime's async functionality, a couple of crates have changed. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

rockwotj force-pushed the stacks branch 3 times, most recently from 16941cd to c027feb Compare October 13, 2023 17:14

rockwotj requested review from dotnwat, oleiman, mmaslankaprv and michael-redpanda October 13, 2023 17:14

rockwotj force-pushed the stacks branch 3 times, most recently from 144d63b to 6e10e25 Compare October 14, 2023 03:53

mmaslankaprv reviewed Oct 16, 2023

View reviewed changes

cmake/dependencies.cmake Show resolved Hide resolved

rockwotj requested a review from mmaslankaprv October 18, 2023 03:49

dotnwat reviewed Oct 18, 2023

View reviewed changes

rockwotj added 3 commits October 18, 2023 10:05

wasm: plugin custom stack allocator to wasmtime

c50d585

This plugins a sharded stack allocator into a custom wasmtime stack allocator. The setup and API here is very similar to how the custom wasmtime (linear) data memory is plugged in. Signed-off-by: Tyler Rockwood <rockwood@redpanda.com>

rockwotj force-pushed the stacks branch from 6e10e25 to 52299a4 Compare October 18, 2023 15:05

rockwotj requested a review from dotnwat October 18, 2023 15:06

dotnwat approved these changes Oct 18, 2023

View reviewed changes

rockwotj merged commit 0860203 into redpanda-data:dev Oct 18, 2023
9 checks passed

rockwotj deleted the stacks branch October 18, 2023 18:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wasm: introduce custom stack allocator for wasmtime #14161

wasm: introduce custom stack allocator for wasmtime #14161

rockwotj commented Oct 13, 2023

rockwotj commented Oct 13, 2023 •

edited

Loading

vbotbuildovich commented Oct 14, 2023

dotnwat Oct 18, 2023

rockwotj Oct 18, 2023

rockwotj commented Oct 18, 2023

dotnwat left a comment

rockwotj commented Oct 18, 2023 •

edited

Loading

dotnwat commented Oct 18, 2023

rockwotj commented Oct 18, 2023

travisdowns commented Oct 19, 2023

wasm: introduce custom stack allocator for wasmtime #14161

wasm: introduce custom stack allocator for wasmtime #14161

Conversation

rockwotj commented Oct 13, 2023

Backports Required

Release Notes

rockwotj commented Oct 13, 2023 • edited Loading

vbotbuildovich commented Oct 14, 2023

dotnwat Oct 18, 2023

Choose a reason for hiding this comment

rockwotj Oct 18, 2023

Choose a reason for hiding this comment

rockwotj commented Oct 18, 2023

dotnwat left a comment

Choose a reason for hiding this comment

rockwotj commented Oct 18, 2023 • edited Loading

dotnwat commented Oct 18, 2023

rockwotj commented Oct 18, 2023

travisdowns commented Oct 19, 2023

rockwotj commented Oct 13, 2023 •

edited

Loading

rockwotj commented Oct 18, 2023 •

edited

Loading