-
Notifications
You must be signed in to change notification settings - Fork 11
Sampling interval instead of sampling rate #253
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
Comments
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? |
Given that |
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. |
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. |
@andreasbovens - given that Whereby has experimented with this API, any input on naming? |
I was talking to @arskama about the current implementation, and it actually caps 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. |
At this point, we're using a However, if there's a desire to change this (and I can see why given the currently allowed interval), |
FYI, I have a CL open that changes this accordingly in the article at https://developer.chrome.com/docs/web-platform/compute-pressure. |
Uh oh!
There was an error while loading. Please reload this page.
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:
That's is about
sampleRate
: https://w3c.github.io/compute-pressure/#the-pressureobserveroptions-dictionaryIn 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:
setInterval
using millisecondssampleInterval
in https://wicg.github.io/js-self-profiling/, in millisecondsminInterval
in https://wicg.github.io/periodic-background-sync/, in millisecondshttps://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.
The text was updated successfully, but these errors were encountered: