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

webgpu: Remove bufSize from GPUData #7637

Merged
merged 2 commits into from
May 2, 2023

Conversation

qjia7
Copy link
Contributor

@qjia7 qjia7 commented Apr 28, 2023

The buffer size and usage are the attributes of GPUBuffer. So bufSize doesn't need to be provided extra in GPUData.

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

The buffer size and usage are the attributes of GPUBuffer. So bufSize
doesn't need to be provided extra in GPUData.
@qjia7 qjia7 requested review from gyagp, Linchenn and xhcao April 28, 2023 05:52
qjia7 added a commit to qjia7/tfjs-examples that referenced this pull request Apr 28, 2023
Copy link
Contributor

@xhcao xhcao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@gyagp gyagp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Linchenn
Copy link
Collaborator

Is bufSize still needed in

export interface GPUData {
  tensorRef: Tensor;
  texture?: WebGLTexture;
  buffer?: GPUBuffer;
  texShape?: [number, number];
  bufSize?: number;
}

@Linchenn
Copy link
Collaborator

Could you help update this doc, https://github.com/tensorflow/tfjs/blob/master/docs/OPTIMIZATION_PURE_GPU_PIPELINE.md? Or, it could be in a separate pr.

@gyagp
Copy link

gyagp commented Apr 28, 2023

Is bufSize still needed in

export interface GPUData {
  tensorRef: Tensor;
  texture?: WebGLTexture;
  buffer?: GPUBuffer;
  texShape?: [number, number];
  bufSize?: number;
}

Jiajia's PR only changes the API, which may impact the upcoming release. We will do the related refactoring after holidays.

Copy link
Collaborator

@Linchenn Linchenn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM

@mattsoulanille mattsoulanille merged commit f3b00b9 into tensorflow:master May 2, 2023
@qjia7
Copy link
Contributor Author

qjia7 commented May 4, 2023

Could you help update this doc, https://github.com/tensorflow/tfjs/blob/master/docs/OPTIMIZATION_PURE_GPU_PIPELINE.md? Or, it could be in a separate pr.

PR #7651 fixes it. Thanks.

@Linchenn
Copy link
Collaborator

Linchenn commented May 4, 2023

Is bufSize still needed in

export interface GPUData {
  tensorRef: Tensor;
  texture?: WebGLTexture;
  buffer?: GPUBuffer;
  texShape?: [number, number];
  bufSize?: number;
}

@qjia7

@qjia7
Copy link
Contributor Author

qjia7 commented May 5, 2023

Is bufSize still needed in

export interface GPUData {
  tensorRef: Tensor;
  texture?: WebGLTexture;
  buffer?: GPUBuffer;
  texShape?: [number, number];
  bufSize?: number;
}

@qjia7

bufSize is not needed anymore in GPUData. You can get the size from buffer.size. See here https://www.w3.org/TR/webgpu/#gpubuffer. size is one of the properties of GPUBuffer.

@Linchenn
Copy link
Collaborator

Linchenn commented May 5, 2023

Thanks!

qjia7 added a commit to tensorflow/tfjs-examples that referenced this pull request May 23, 2023
* Use data.buffer.size instead of data.bufSize

data.bufSize will be removed in tensorflow/tfjs#7637.

* remove buffer.size
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 this pull request may close these issues.

None yet

5 participants