search

fastly / Viceroy

Viceroy provides local testing for developers working with Compute.
https://fastly.dev/learning/compute/testing/#running-a-local-testing-server
Apache License 2.0
149 stars 35 forks source link

Provide a way to set the interval for samples when profiling #340

Open katef opened 10 months ago

katef commented 10 months ago

Currently the sample frequency for profiling in Viceroy is hardcoded to 50µs. For our purposes we don't need the frequency to be so low that performance is unaffected, but we do need it to be high enough to report on small things. Setting it as a CLI option seems reasonable to me.

wasmtime provides --profile=guest[,path[,interval]] which allows setting the interval between samples for profiling: https://github.com/bytecodealliance/wasmtime/blob/main/docs/examples-profiling-guest.md?plain=1#L7-L11:

To use this profiler with the Wasmtime CLI, pass the --profile=guest[,path[,interval]] flag.

  • path is where to write the profile, wasmtime-guest-profile.json by default
  • interval is the duration between samples, 10ms by default

I think it'd make sense to match this, Viceroy already has similar syntax for its --profile cli flag.

jameysharp commented 10 months ago

I'm in favor of this suggestion. There's a subtlety though, which is that the EPOCH_INTERRUPTION_PERIOD is used regardless of whether the guest profiler is enabled or not. It also has an impact on how long the guest can run before async background tasks get an opportunity to make progress. As I recall, Rainy and I had some trouble with Viceroy's test suite unless we set the interval fairly short.

It's probably worth putting some thought into what the default interval should be in both cases: 10ms for Wasmtime and 50µs for Viceroy are both pretty arbitrary choices. It makes some sense that they're different if we assume that Viceroy guests are usually short-lived compared with Wasmtime guests, but the exact durations are still not well justified.

One Viceroy user reported a cost I hadn't considered of having a short interval: There's a thread that wakes up briefly on that interval, even if no guests are running at the time, and that prevents the CPU from going into deeper power-saving states. So Viceroy users on battery might need a longer interval for power management reasons. Eventually it'd be nice to pause the epoch-interruption thread while there are no requests in flight, but that's some extra complexity.