w3c / compute-pressure

A web API proposal that provides information about available compute capacity
https://www.w3.org/TR/compute-pressure/
Other
69 stars 10 forks source link

Sampling interval instead of sampling rate #253

Closed foolip closed 8 months ago

foolip commented 8 months ago

In https://groups.google.com/a/chromium.org/g/blink-dev/c/7leKysvPZWk/m/WyFKLqwRAQAJ this aggregated feedback from the Chrome origin trial was provided:

7% of the respondents mentioned that they would prefer to give the callback frequency in time units (secs, milliseconds), similar to other APIs such as setInterval in place of Hz as currently designed. A number scale along with labels would be more useful e.g. { pressure: 2, label: 'nominal' }.

That's is about sampleRate: https://w3c.github.io/compute-pressure/#the-pressureobserveroptions-dictionary

In a look for "frequency", "interval" and "samplerate" in https://github.com/w3c/webref/tree/main/ed/idl I find the following precedent in favor of using a milliseconds interval:

https://w3c.github.io/sensors/ uses a frequency in Hz for how often a sensor should update.

All existing cases of sampleRate on the web platform have to do with audio or video, most of it audio.

I hope that helps make a decision on whether to make a change or not.

kenchris commented 8 months ago

We discussed this before in the team and I don't feel strongly about any of the options. Luckily it is easy to convert from one to the other, but a patch to the spec and implementation is also quite simple.

When I looked at this last I don't think the JS self profiling existed (or I wasn't aware of it) so that balances it a bit in favor of milliseconds.

On the other hand many who use Compute Pressure currently use it with video and audio streams, but I don't think anyone runs it as the same frequency/samplerate.

@rakuco do you have input?

foolip commented 8 months ago

Given that sampleRate is only used for audio/video elsewhere, I might suggest frequency if you want Hz, or sampleInterval if you want a duration in milliseconds. (API owners hat off here.)

kenchris commented 8 months ago

I am fine with changing it to frequency (rename).

The other thing is that we don't expect people to need state changes very often, so mostly in seconds and probably never below one second, so in that sense, sampleInterval might make more sense.

foolip commented 8 months ago

What kind of sampling rates/intervals have you seen people using in demos and prototypes using this API? If it's usually no more than 10Hz, then that would suggest interval as the more convenient/familiar representation.

miketaylr commented 8 months ago

@andreasbovens - given that Whereby has experimented with this API, any input on naming?

rakuco commented 8 months ago

@rakuco do you have input?

I was talking to @arskama about the current implementation, and it actually caps sampleRate at 1Hz (not by design, the code just used to be like that in the original implementation and has not been changed yet), so allowed rates are always in the (0, 1.0) interval at the moment.

Given that there don't seem to be any complaints about this so far, I think it's fair to say that sampling rates are < 10Hz most of the time/all the time, so going for an interval instead of a frequency sounds reasonable.

andreasbovens commented 8 months ago

At this point, we're using a sampleRate of 1Hz, and talking with our engineers, they feel this naming is clear and makes sense.

However, if there's a desire to change this (and I can see why given the currently allowed interval), sampleInterval with values in milliseconds can work as well.

tomayac commented 8 months ago

FYI, I have a CL open that changes this accordingly in the article at https://developer.chrome.com/docs/web-platform/compute-pressure.