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

Move source to options #262

Closed kenchris closed 7 months ago

kenchris commented 7 months ago

Following up on @Elchi3 's #256 where the following was suggested

await observer.observe({ sources: ["cpu"], sampleInterval: 2500 });

This is pretty consistent with PerformanceObserver, the "type" (and "entryTypes") is also part of the options.

We could do

await observer.observe({ source: "cpu", sampleInterval: 2500 });

now (pre shipping), and add sources in the future.

Or does @elchi3 believe we should just support sources and not source?

Elchi3 commented 7 months ago

I think the reason for PerformanceObserve to have both type and entryTypes is that there are rules about how this plays together with other properties of the options object:

// valid
observe({ type: "resource", buffered: true });
observe({ type: "event", durationThreshold: 160 });
observe({ entryTypes: ["mark", "measure"] });

// invalid
observe({ entryTypes: ["resource"], buffered: true });
observe({ entryTypes: ["event", "mark"], durationThreshold: 160 });
observe({ entryTypes: ["resource"], type: "event" });

So in PressureObserver, you need to decide if you want to allow sampleInterval to be set for all sources or not, for example, and what would be the deal with source-specific options:

await observer.observe({ sources: ["cpu", "gpu"], sampleInterval: 2500 }); / valid?
await observer.observe({ source: "cpu", sampleInterval: 2500 }); // valid

await observer.observe({ sources: ["cpu", "gpu"], gpuSpecificOption: "foo" }); // invalid?
await observer.observe({ source: "gpu", gpuSpecificOption: "foo" }); // valid

Or does @Elchi3 believe we should just support sources and not source?

I think I would say having both makes sense. Especially, if you believe there will be source-specific options.

kenchris commented 7 months ago

sampleInterval should be available for all sources and are a request, given the hardware might not be able to fulfill it.

I think it is a bit early to say if we get source specific options, but that could definitely happen.

I generally think that

  observer.observe({ source: "gpu" })

is more clear than

  observer.observe("gpu")
kenchris commented 7 months ago

@rakuco do you have any input?

rakuco commented 7 months ago

My understanding is that "source" (or even "sources" depending on the outcome of #258 and the above) will always be a required parameter given the semantics of this API. This is similar to element or target in MutationObserver, IntersectionObserver and ResizeObserver, so from a spec or IDL perspective it feels confusing to have it as part of PressureObserverOptions, as options is an optional parameter for observe().

You could also make it a required PressureObserverOptions member and stop making options optional, but it feels contrary to https://w3ctag.github.io/design-principles/#optional-parameters (other parameters are indeed optional) and https://w3ctag.github.io/design-principles/#prefer-dictionaries.

kenchris commented 7 months ago

That is exactly what it was modelled after @rakuco I just wanted to make sure that we really thought this through. Maybe PerformanceObserver is a bit of the odd one out here.

I think we are good and should close this.

tomayac commented 7 months ago

To be on the safe side, it may be worthwhile to loop in the @w3ctag and hear their opinion on the matter.

kenchris commented 7 months ago

True, if the @w3ctag disagree with our reasoning, please reopen