Skip to content

[WebGPU] webgpu.github.io/webgpu-samples/?sample=renderBundles uses excessive memory#35809

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
mwyrzykowski:eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory
Oct 31, 2024
Merged

[WebGPU] webgpu.github.io/webgpu-samples/?sample=renderBundles uses excessive memory#35809
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
mwyrzykowski:eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory

Conversation

@mwyrzykowski
Copy link
Copy Markdown
Contributor

@mwyrzykowski mwyrzykowski commented Oct 28, 2024

31d96d6

[WebGPU] webgpu.github.io/webgpu-samples/?sample=renderBundles uses excessive memory
https://bugs.webkit.org/show_bug.cgi?id=282153
rdar://138726891

Reviewed by Dan Glastonbury.

Command encoders were stored in a WeakHashSet, but
we only need to store the valid ones and not invalid ones.

Remove invalid encoders to avoid a retain cycle.

* Source/WebGPU/WebGPU/Buffer.mm:
(WebGPU::Buffer::setCommandEncoder const):
* Source/WebGPU/WebGPU/CommandEncoder.h:
* Source/WebGPU/WebGPU/CommandEncoder.mm:
(WebGPU::CommandEncoder::removeInvalidEncoders):
* Source/WebGPU/WebGPU/ExternalTexture.mm:
(WebGPU::ExternalTexture::setCommandEncoder const):
* Source/WebGPU/WebGPU/QuerySet.mm:
(WebGPU::QuerySet::setCommandEncoder const):
* Source/WebGPU/WebGPU/Texture.mm:
(WebGPU::Texture::setCommandEncoder const):
* Source/WebGPU/WebGPU/TextureView.mm:
(WebGPU::TextureView::setCommandEncoder const):

Canonical link: https://commits.webkit.org/285968@main

abd58ed

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2
✅ 🛠 tv 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@mwyrzykowski mwyrzykowski self-assigned this Oct 28, 2024
@mwyrzykowski mwyrzykowski added the WebGPU For bugs in WebGPU label Oct 28, 2024
@mwyrzykowski mwyrzykowski requested a review from djg October 28, 2024 18:57
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 28, 2024
@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Oct 28, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2024
@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from b3617ec to e87c9bc Compare October 29, 2024 19:15
Copy link
Copy Markdown
Contributor

@djg djg left a comment

Choose a reason for hiding this comment

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

:shipit:

Comment thread Source/WebGPU/WebGPU/ExternalTexture.mm Outdated
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from e87c9bc to 6e4093f Compare October 30, 2024 22:03
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from 6e4093f to 3af59b2 Compare October 30, 2024 23:01
@mwyrzykowski mwyrzykowski requested a review from djg October 30, 2024 23:02
@mwyrzykowski
Copy link
Copy Markdown
Contributor Author

@djg I had to make a few more additions to get this actually working. Still ok to merge?

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 31, 2024
@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Oct 31, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from 3af59b2 to 8dac693 Compare October 31, 2024 04:52
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #4301

❌ Found 5 new failures. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from 8dac693 to 84013ac Compare October 31, 2024 05:16
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 31, 2024
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safer C++ Build #4346

❌ Found 1 new failure. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
(cc @rniwa)

@mwyrzykowski mwyrzykowski removed the merging-blocked Applied to prevent a change from being merged label Oct 31, 2024
@mwyrzykowski mwyrzykowski force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from 84013ac to abd58ed Compare October 31, 2024 15:10
@mwyrzykowski mwyrzykowski added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Oct 31, 2024
@webkit-ews-buildbot webkit-ews-buildbot added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Oct 31, 2024
@webkit-ews-buildbot
Copy link
Copy Markdown
Collaborator

Safe-Merge-Queue: Build #37114.

…xcessive memory

https://bugs.webkit.org/show_bug.cgi?id=282153
rdar://138726891

Reviewed by Dan Glastonbury.

Command encoders were stored in a WeakHashSet, but
we only need to store the valid ones and not invalid ones.

Remove invalid encoders to avoid a retain cycle.

* Source/WebGPU/WebGPU/Buffer.mm:
(WebGPU::Buffer::setCommandEncoder const):
* Source/WebGPU/WebGPU/CommandEncoder.h:
* Source/WebGPU/WebGPU/CommandEncoder.mm:
(WebGPU::CommandEncoder::removeInvalidEncoders):
* Source/WebGPU/WebGPU/ExternalTexture.mm:
(WebGPU::ExternalTexture::setCommandEncoder const):
* Source/WebGPU/WebGPU/QuerySet.mm:
(WebGPU::QuerySet::setCommandEncoder const):
* Source/WebGPU/WebGPU/Texture.mm:
(WebGPU::Texture::setCommandEncoder const):
* Source/WebGPU/WebGPU/TextureView.mm:
(WebGPU::TextureView::setCommandEncoder const):

Canonical link: https://commits.webkit.org/285968@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/WebGPU-webgpu-github-iowebgpu-samplessamplerenderBundles-uses-excessive-memory branch from abd58ed to 31d96d6 Compare October 31, 2024 18:55
@webkit-commit-queue
Copy link
Copy Markdown
Collaborator

Committed 285968@main (31d96d6): https://commits.webkit.org/285968@main

Reviewed commits have been landed. Closing PR #35809 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 31d96d6 into WebKit:main Oct 31, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebGPU For bugs in WebGPU

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants