Stream: git-wasmtime

Topic: wasmtime / PR #8795 Add a compile-time feature for call h...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:38):

alexcrichton opened PR #8795 from alexcrichton:gate-call-hook to bytecodealliance:main:

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.

[Zulip]: https://bytecodealliance.zulipchat.com/#narrow/stream/217126-wasmtime/topic/Performance.20regression.20since.20rust.201.2E65/near/444505571

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:38):

alexcrichton requested fitzgen for a review on PR #8795.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:38):

alexcrichton requested wasmtime-core-reviewers for a review on PR #8795.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:41):

fitzgen submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:44):

alexcrichton updated PR #8795.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 19:50):

alexcrichton has enabled auto merge for PR #8795.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 20:06):

alexcrichton merged PR #8795.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 21:39):

pchickey commented on PR #8795:

I think this is the correct choice - needing call hooks at all is 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?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 21:39):

pchickey edited a comment on PR #8795:

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?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 13 2024 at 21:42):

alexcrichton commented on PR #8795:

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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 14 2024 at 09:42):

tschneidereit commented on PR #8795:

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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 14 2024 at 15:06):

alexcrichton commented on PR #8795:

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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 14 2024 at 15:25):

alexcrichton commented on PR #8795:

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.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 14 2024 at 18:07):

alexcrichton commented on PR #8795:

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.


Last updated: Dec 23 2024 at 13:07 UTC