search

bytecodealliance / wasmtime

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

wasmtime: Take guest-profiler samples at hostcalls #8802

Closed jameysharp closed 2 weeks ago

jameysharp commented 3 weeks ago

The output of the guest profiler can be misleading around hostcalls. Whatever happened to be the last sample before the hostcall appears to run for the entire time of the hostcall. This change ensures that we can see the actual call stack at the time of the hostcall, and get a visual indication of which periods are not spent executing guest code.

jameysharp commented 2 weeks ago

@alexcrichton, this PR means that the guest profiler will require the new call-hook feature. How do you think we should set the defaults for these features?

I could imagine for example that profiling and call-hook would both be off-by-default in the wasmtime crate so that embedders don't take the performance hit unless they ask for it, but enabled by default in wasmtime-cli so that CLI users have the full profiler available.

Alternatively, the profiler's use of call-hooks could be made conditional on whether call-hook is enabled, but 1) I think that's going to be a bit ugly and 2) the new functionality makes the profiles quite a bit easier to interpret correctly so I kind of want it always available.

alexcrichton commented 2 weeks ago

Is the use case you're primarily interested in the CLI itself? If so could we thread that needle by making the hooks conditional but unconditionally enabling them in the CLI?

jameysharp commented 2 weeks ago

I do want this available in the CLI but I also want it in Fastly's CLI embedding of Wasmtime (Viceroy). So as long as all embedders have the option to turn this on then I'm happy.

I'll push a commit for that in a bit.

fitzgen commented 2 weeks ago

Can we make it so that the sample-at-hostcall functionality only happens when the call hook feature is enabled and then embeddings can enable or disable the feature to get more functionality (at the cost of some host-wasm call overhead) as they choose?

jameysharp commented 2 weeks ago

I just realized that the CallHook type (the enum indicating which kind of transition happened) is not guarded by the call-hook feature, which makes this simpler than I expected. That means the part of the guest profiler that's in the wasmtime crate does not actually require the call-hook feature; it can unconditionally provide a call_hook method for an embedder to use if they want to.

The embedder is responsible for arranging for that function to be called from an actual call-hook, and so it's their responsibility to enable the call-hook feature if they want to use that part of the guest profiler. In principle I think an embedder could also use other means to pass the same information to the profiler if they wanted to.

Hopefully that makes everyone happy?

alexcrichton commented 2 weeks ago

Oh nice! That sounds good to me 👍