bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.21k stars 1.28k forks source link

Add a compile-time feature for call hooks #8795

Closed alexcrichton closed 3 months ago

alexcrichton commented 3 months ago

This commit moves the Store::call_hook API behind a Cargo feature named call-hook. This helps speed up the path from wasm into the host by avoiding branches at the start and the end of the execution. In a thread on Zulip this is locally leading to significant performance gains in this particular microbenchmark so having an option to disable it at the crate layer seems like a reasonable way to thread this needle for now. This definitely has a downside in that it requires a crate feature at all, but I'm not sure of a better solution as LLVM can't dynamically detect that Store::call_hook is never invoked and therefore the branch can be optimized away.

pchickey commented 3 months ago

I think this is the correct choice - call hooks fill a pretty particular need, and I'd even be fine if this feature was not enabled by default. Have we surveyed if anyone besides Fastly's embedding even makes use of it?

alexcrichton commented 3 months ago

I haven't surveyed myself but carrying a Cargo feature for it isn't too bad. Most of our Cargo features have been relatively low overhead to maintain so far. The main question to me is that this might be an appropriate feature to be off-by-default instead of on-by-default as it can affect benchmarks, but even then it's pretty rare anyone benchmarks host-to-wasm calls or vice versa, most benchmarking is just of the wasm itself.

tschneidereit commented 3 months ago

I think regardless of who might encounter this in benchmarks, as long it's not essentially always used, it makes sense to disable by default: we should keep the call overhead low wherever we can, after all, and requiring people to disable very specific features for best performance doesn't seem great to me.

alexcrichton commented 3 months ago

I can try to investigate this a bit. I believe the gc feature has a much larger impact than call-hooks in terms of performance. I haven't bottomed that out but disabling gc-by-default I think would be much more significant than disabling call-hook by default.

alexcrichton commented 3 months ago

Measuring locally from the benchmark from Zulip, which is very heavy in wasm->host calls and the host call itself does nothing, enabling the gc feature of Wasmtime yields a slowdown of 3.5x and enabling the call-hook feature slows down only 30%.

Put another way I don't think it's worth over-rotating too much on call-hook, gc is the main feature that's causing a slowdown.

alexcrichton commented 3 months ago

With https://github.com/bytecodealliance/wasmtime/pull/8807 the performance is on-par now with the gc feature enabled, so I'll follow that up by disabling the call-hook feature by default.