Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Support more profilers with fastly compute serve #1103

Open
jameysharp opened this issue Dec 8, 2023 · 6 comments
Open

Comments

@jameysharp
Copy link

Is your feature request related to a problem? Please describe.
The fastly compute serve --profile-guest option works on all platforms and doesn't require installing any additional software, so it's a great default choice for profiling, but it has some limitations compared to native profilers like Linux perf or Intel VTune. Viceroy supports these other profilers via its --profile option but the Fastly CLI doesn't expose those alternatives when invoking Viceroy with fastly compute serve.

Describe the solution you'd like
Add an option to fastly compute serve that causes it to pass e.g. --profile=jitdump to Viceroy. Currently Viceroy also supports perfmap and vtune variants.

It would be preferable to also wrap Viceroy in the appropriate profiler. For example, profiling with VTune means running vtune -run-pass-thru=--no-altstack -collect hotspots viceroy --profile=vtune instead of just running viceroy.

When using Linux perf in jitdump mode, after recording the profile with perf record, the user has to additionally run something like perf inject --jit --input perf.data --output perf.jit.data. It would be nice to do this for them. It would also be nice to have perf record write to a temporary file and have perf inject output to the perf.data name that perf report expects to read from.

The user may want to pass additional options to the profiler so it would be good to have an option for that.

Describe alternatives you've considered
Adding support for passing through uninterpreted arguments to Viceroy, as suggested in #1047, might work here too. However, that means the user has to wrap the profiler around the Fastly CLI and ensure that it profiles child processes as well, which not only makes the user experience more complex but will add a bunch of data they don't care about to the profile.

In the future, Wasmtime and Viceroy may gain support for more profilers. It would be nice if we didn't need to change the Fastly CLI to support new profilers, which would suggest making the profiler name be an uninterpreted string. We could probably define some way for Viceroy to tell the CLI what profilers it supports and perhaps how to invoke them, or make Viceroy re-spawn itself under the appropriate profiler.

Additional context
The Wasmtime book documents Wasmtime's support for external profilers, specifically perf and VTune. There's also documentation on using Samply with Wasmtime which relies on the same perfmap profiling mode that perf can use.

@tedmielczarek-fastly
Copy link

Could you make this work by writing a wrapper script that invokes viceroy under your profiler of choice, and passing that to the CLI via --viceroy-path?

@tedmielczarek-fastly
Copy link

For the record, I tested this and it does work:

% cat > /tmp/viceroy.sh
#!/bin/sh

exec viceroy --unknown-import-behavior zero-or-null "$@"

Then:

% fastly compute serve --viceroy-path=/tmp/viceroy.sh

@jameysharp
Copy link
Author

That's a great proof of concept, thanks! I don't think shell-script wrappers really address the UX points I was raising, but I do think this is a good way to prototype the necessary functionality. For Linux perf in "jitdump" mode as an example, where there's a required postprocessing step after the profile is collected, I think something like this should work, though I haven't tested it:

#!/bin/sh
perf record -k mono viceroy --profile jitdump "$@"
perf inject --jit --input perf.data --output perf.jit.data
echo 'now run `perf report --input perf.jit.data`'

@fgsch
Copy link
Member

fgsch commented Apr 24, 2024

Until this is implemented, this can be achieved via #1186 .

@jameysharp
Copy link
Author

#1186 is great but note that if you do something like --viceroy-args '--profile jitdump' then you have to run the profiler around the entire Fastly CLI process, rather than just profiling Viceroy itself. I'm not sure whether all the profilers Wasmtime supports work with multi-process setups like that.

@fgsch
Copy link
Member

fgsch commented Apr 24, 2024

@jameysharp good point. I wonder if we should include some scripts together with the CLI and direct people to use --viceroy-path=... instead of adding the option to specify other profilers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants