Skip to content
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

Consider removing MLBufferView in its current form #560

Closed
a-sully opened this issue Feb 8, 2024 · 0 comments · Fixed by #569
Closed

Consider removing MLBufferView in its current form #560

a-sully opened this issue Feb 8, 2024 · 0 comments · Fixed by #569

Comments

@a-sully
Copy link
Contributor

a-sully commented Feb 8, 2024

Building on #546 and #553

In the Chromium implementation, the Model Loader API spec, and - soon, once #553 merges - the WebNN spec, MLBufferView is specified as:

typedef ArrayBufferView MLBufferView;

This was intended to be forward-looking (let's be prepared for when we start using hardware accelerators?!) to provide a "view over some device-agnostic buffer", but at this point:

  1. We're ready to use hardware accelerators; MLBuffer is our proposed solution for a device-agnostic buffer (see Support for device-based tensor storage objects #482)
  2. It's unclear whether a typedef is sufficient to provide the "view over an MLBuffer" that MLBufferView was initially intending to provide, and
  3. This typedef is not adding any value otherwise

As @bbernhar pointed out in #553 (comment), we'll still likely want the ability to take a view over an MLBuffer. However, I (personally) think it's more likely this ends up looking more like a GpuBufferBinding than a typedef

Given all this, I propose we remove MLBufferView in its current form for now, and then we can discuss the specifics of how to take a view over an MLBuffer at a later time

a-sully added a commit to a-sully/webnn that referenced this issue Feb 8, 2024
a-sully added a commit to a-sully/webnn that referenced this issue Feb 15, 2024
a-sully added a commit to a-sully/model-loader that referenced this issue Feb 15, 2024
a-sully added a commit to a-sully/model-loader that referenced this issue Feb 15, 2024
anssiko pushed a commit to webmachinelearning/model-loader that referenced this issue Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant