@reillyeon

but I think that any argument for why compute inputs and outputs are transferred should also apply to constants.

Agreed.

I'll note that transferring ArrayBuffer parameters makes an API harder to call from WebAssembly

This is true.

Should we consider copying the content of input (and constant) ArrayBuffer instead? Notice that the Chromium implementation actually copies the input and constant ArrayBuffers for XNNPACK backend and DirectML (mojo) backend. It means for each input ArrayBuffer, WebAssembly path may need two copies, one is done by JS that copies from Wasm heap to an ArrayBuffer that can be transferred, another one is done by Chromium implementation that copies from ArrayBuffer to internal storage.

Given the constraints on implementations based on the underlying platform APIs I think it's reasonable to define constant and input handling such that MLGraphBuilder and compute() copy any provided ArrayBuffers. If developers want to avoid a copy they can use MLBuffers instead.

In theory Chromium's XNNPACK backend could avoid copies but I'm also planning to delete it in favor of a more flexible TFLite backend. The zero-copy option only works for CPU backends anyways (without MLBuffer).

Renamed this issue given my current position on how to resolve the inconsistency between handling of constant and input ArrayBuffers.

#700 naively addresses this issue by copying (rather than transferring) inputs. This is a bit awkward, though:

1. MLComputeResult contains the copied inputs. This is completely superfluous
2. It's a bit odd that the inputs - but not the outputs - are transferred
3. Should we bother passing outputs to compute() in the first place? As @reillyeon mentioned above, if the developer wants zero/minimal-copy, they should use MLBuffer. Passing outputs doesn't actually reduce copies in the Chromium implementation anyways

Thoughts on these changes?

dictionary MLComputeResult {
-  MLNamedArrayBufferViews inputs;
  MLNamedArrayBufferViews outputs;
};

-Promise<MLComputeResult> compute(MLGraph graph, MLNamedArrayBufferViews inputs, MLNamedArrayBufferViews outputs);
+Promise<MLComputeResult> compute(MLGraph graph, MLNamedArrayBufferViews inputs);

Note that we'd like to deprecate compute() for MLBuffer + dispatch() at some point so perhaps this is not worth much discussion... Though MLBuffer isn't specified at all yet, so that may be a ways off :)

(moved the comment here from the PR)
For me copying is fine, but also asking @inexorabletash: in general for the web platform, should we express more concisely the use case (policy) behind this, i.e. the expected immutability of inputs (transfer of ownership) and that copying is just a way (mechanism) to ensure that... we'd rather want something like "lock input" steps (choose a better name) - in most cases implemented (even spec'd) as copying the input. I understand in this case it will always end up being a copy, but (coming from RTOS times) something hurts inside :).

The closest I've found (thanks to Webdex) are these annotated invocations:

• https://encoding.spec.whatwg.org/#dom-textdecoder-decode
• https://fetch.spec.whatwg.org/#incrementally-read-loop
• https://gpuweb.github.io/gpuweb/#dom-gpuqueue-writetexture

The one in Encoding is probably the closest to what you're looking for:

• Normative spec text just says copy, i.e. implementations can do anything, but it must be indistinguishable from a copy
• Informative note encouraging implementations to optimize if possible
• Warning about security hazards of the data mutating

I think the equivalent here would be to add an informative note to the call sites (in #700, "Let copiedInputs be the result of copying MLNamedArrayBufferViews inputs ...") describing why a copy is made. I think this is helpful for reviewers, future editors, and implementers to understand why it is specified this way. This could be written as just NOTE: A copy is performed because... Implementations are encouraged to... or perhaps <details class=note><summary>Why a copy?</summary>A copy is performed because...</details>

Thanks for the input, @inexorabletash. Adding a NOTE to #700 SGTM

Thoughts on these changes?

dictionary MLComputeResult {
-  MLNamedArrayBufferViews inputs;
  MLNamedArrayBufferViews outputs;
};

-Promise<MLComputeResult> compute(MLGraph graph, MLNamedArrayBufferViews inputs, MLNamedArrayBufferViews outputs);
+Promise<MLComputeResult> compute(MLGraph graph, MLNamedArrayBufferViews inputs);

Any objections to expanding the scope of this issue to the above diff? i.e. Don't transfer input ArrayBuffers, and return output ArrayBuffers rather than passing them to compute()

Accepting a typed array to write the result into (or "BYOB") is a valuable pattern for reducing memory allocation overhead (and thus garbage collection overhead).

I don't think it's worth changing the interface here but we should consider providing a BYOB version of readBuffer() which accepts a typed array to copy the contents of the MLBuffer into. By specifying that the typed array is written to on the ML task source I think we can avoid the need to transfer the buffer since script won't have concurrent access to its contents (anything written to the buffer before the copy is performed will be overwritten).