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.

Sign up for GitHub

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

Jump to bottom

[js/webgpu] reuse buffer for GpuDataManager #16746

Merged
merged 3 commits into from
Jul 21, 2023
Merged

[js/webgpu] reuse buffer for GpuDataManager #16746

merged 3 commits into from
Jul 21, 2023

Conversation

qjia7
Copy link
Contributor

@qjia7 qjia7 commented Jul 18, 2023

Description

Allocating new GPUBuffer in every session.run is not efficient. We should make it only happen in the first run. In the following runs, we should try to reuse those buffers.

Motivation and Context

  • This PR is for performance.
    See mobilenetv2 becomes 9.58 ms from 12.9 ms.

@qjia7
[js/webgpu] reuse buffer for GpuDataManager
8cdfb10 
This PR is for performance.
See mobilenetv2 becomes 9.58 ms from 12.9 ms.
@qjia7 qjia7 marked this pull request as ready for review July 18, 2023 08:32
@qjia7
Copy link
Contributor Author

qjia7 commented Jul 18, 2023

@fs-eire @guschmue Please take a look, thanks!

@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@guschmue
Copy link
Contributor

guschmue commented Jul 19, 2023 •
edited
Loading

CI pipeline is nagging: run npm run format from js/ should make it happy.
https://dev.azure.com/onnxruntime/onnxruntime/_build/results?buildId=1067304&view=logs&jobId=96bb6f8b-177f-5c87-f240-68050a28686f&j=96bb6f8b-177f-5c87-f240-68050a28686f&t=473361f3-9d46-5fb7-3d33-5a64e2c16152

@qjia7 qjia7 requested a review from fs-eire July 20, 2023 05:00
@qjia7
Copy link
Contributor Author

qjia7 commented Jul 20, 2023

CI pipeline is nagging: run npm run format from js/ should make it happy. https://dev.azure.com/onnxruntime/onnxruntime/_build/results?buildId=1067304&view=logs&jobId=96bb6f8b-177f-5c87-f240-68050a28686f&j=96bb6f8b-177f-5c87-f240-68050a28686f&t=473361f3-9d46-5fb7-3d33-5a64e2c16152

Done. Please take another look. Thanks.

@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@fs-eire
Copy link
Contributor

fs-eire commented Jul 20, 2023

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline, Windows GPU TensorRT CI Pipeline, Windows ARM64 QNN CI Pipeline, orttraining-linux-ci-pipeline, orttraining-linux-gpu-ci-pipeline, orttraining-ortmodule-distributed, ONNX Runtime React Native CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@fs-eire
Copy link
Contributor

fs-eire commented Jul 20, 2023

/azp run Linux CPU CI Pipeline, Linux CPU Minimal Build E2E CI Pipeline, Linux GPU CI Pipeline, Linux GPU TensorRT CI Pipeline, Linux OpenVINO CI Pipeline, MacOS CI Pipeline, ONNX Runtime Web CI Pipeline, onnxruntime-binary-size-checks-ci-pipeline, Linux QNN CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@guschmue
Copy link
Contributor

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@guschmue
Copy link
Contributor

/azp run Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 16746 in repo microsoft/onnxruntime

@guschmue
Copy link
Contributor

/azp run Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@guschmue guschmue merged commit 193415a into microsoft:main Jul 21, 2023
@qjia7 qjia7 deleted the reuse-buffer branch July 24, 2023 01:04
jchen351 pushed a commit that referenced this pull request Aug 12, 2023
@qjia7 @jchen351
[js/webgpu] reuse buffer for GpuDataManager (#16746)
42aecbc 
### Description
<!-- Describe your changes. -->
Allocating new GPUBuffer in every session.run is not efficient. We
should make it only happen in the first run. In the following runs, we
should try to reuse those buffers.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
- This PR is for performance.
See mobilenetv2 becomes 9.58 ms from 12.9 ms.
siweic0 pushed a commit to siweic0/onnxruntime-web that referenced this pull request May 9, 2024
@qjia7
[js/webgpu] reuse buffer for GpuDataManager (microsoft#16746)
c0e5dd1 
### Description
<!-- Describe your changes. -->
Allocating new GPUBuffer in every session.run is not efficient. We
should make it only happen in the first run. In the following runs, we
should try to reuse those buffers.

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
- This PR is for performance.
See mobilenetv2 becomes 9.58 ms from 12.9 ms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gyagp gyagp gyagp left review comments

@guschmue guschmue guschmue approved these changes

@fs-eire fs-eire fs-eire approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@qjia7 @guschmue @fs-eire @gyagp