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

Reintroduce buffer snatching Part 1 #4867

Merged
merged 4 commits into from
Dec 14, 2023
Merged

Reintroduce buffer snatching Part 1 #4867

merged 4 commits into from
Dec 14, 2023

Conversation

nical
Copy link
Contributor

@nical nical commented Dec 12, 2023

Connections

This is the first step towards addressing #4787.

Description

We need destroy() methods to be able to deallocate gpu resources while the associated handle is still alive. This is difficult to do safely because most of wegpu's API can be used from any thread.One of the steps towards our goal is to re-introduce "buffer snatching". The idea is that when, say, buffer.destroy() is called, we snatch the raw buffer out of wgpu_core's buffer struct, and enqueue it to be deallocated after the latest submission the buffer has been used in is complete.
Snatching the raw handle out requires some form of locking. @cwfitzgerald's proposed solution is to protect all raw buffer and texture reads behind the read guard of a device-global RwLock, and only ever thake a write lock during snatching. The hope is that the overhead of taking a single read guard in most places will be small enough and that destroy will be called concurrently with other wgpu work rarely enough that it won't cause very bad contention.

This PR implements the very first step of Connor's idea:

  • Device holds snatchable_lock.
  • a Snatchable<T> type is introduced. Its content cannot be accessed without an appropriate lock guard.
  • Buffer.raw is now a ``Snatchable<A::Buffer>` and all associated reads need the guard.

The snatching is not actually performed in this PR (yet). It will come in a followup.

Checklist

  • Run cargo fmt.
  • Run cargo clippy.
  • Run cargo xtask test to run tests.

@nical nical requested a review from a team as a code owner December 12, 2023 11:45
@ErichDonGubler ErichDonGubler marked this pull request as draft December 12, 2023 14:51
@nical nical marked this pull request as ready for review December 12, 2023 15:45
@nical
Copy link
Contributor Author

nical commented Dec 12, 2023

I don't want this marked as draft, it should get reviewed and hopefully landed. The "WIP" in the title was referring to the fact that it's a part of a larger ongoing refactoring, I'll remove it since it is more confusing than helpful.

@nical nical changed the title (WIP) Reintroduce buffer snatching Part 1 Reintroduce buffer snatching Part 1 Dec 12, 2023
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Looks good! Some organizational notes.

wgpu-core/src/device/resource.rs Outdated Show resolved Hide resolved
wgpu-core/src/device/resource.rs Outdated Show resolved Hide resolved
wgpu-core/src/device/resource.rs Outdated Show resolved Hide resolved
@ErichDonGubler
Copy link
Member

I don't want this marked as draft, it should get reviewed and hopefully landed. The "WIP" in the title was referring to the fact that it's a part of a larger ongoing refactoring, I'll remove it since it is more confusing than helpful.

Ah, sorry!

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Nice!

wgpu-core/src/snatch.rs Outdated Show resolved Hide resolved
wgpu-core/src/snatch.rs Outdated Show resolved Hide resolved
@nical nical merged commit c5e6122 into gfx-rs:trunk Dec 14, 2023
27 checks passed
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jan 3, 2024
…225e1744fd7. r=webgpu-reviewers,supply-chain-reviewers,teoxoy

Changelog:

 * #4865 fix present mode for wgl
   By xiaopengli89 in gfx-rs/wgpu#4865
 * #4873 Bump ctor from 0.2.5 to 0.2.6
   By dependabot[bot] in gfx-rs/wgpu#4873
 * #4874 Bump syn from 2.0.40 to 2.0.41
   By dependabot[bot] in gfx-rs/wgpu#4874
 * #4869 [naga wgsl-out] Include the `f` suffix on `f32` literals.
   By jimblandy in gfx-rs/wgpu#4869
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4870 [naga wgsl] Let unary operators accept and produce abstract types.
   By jimblandy in gfx-rs/wgpu#4870
 * #4867 Reintroduce buffer snatching Part 1
   By nical in gfx-rs/wgpu#4867
 * #4882 Bump zerocopy from 0.7.26 to 0.7.31
   By dependabot[bot] in gfx-rs/wgpu#4882
 * #4878 Buffer snatching part 2 - Refactor create_buffer
   By nical in gfx-rs/wgpu#4878
 * #4815 Add `wgpu` crate features for backends
   By daxpedda in gfx-rs/wgpu#4815
 * #4887 Allow clippy::pattern_type_mismatch
   By nical in gfx-rs/wgpu#4887
 * #4886 Document wgpu & wgpu-core features
   By Wumpf in gfx-rs/wgpu#4886
 * #4826 validation: More detailed on incompatible BGL
   By scoopr in gfx-rs/wgpu#4826
 * #4888 Web: add support for more `RawWindowHandle` variants
   By daxpedda in gfx-rs/wgpu#4888
 * #4890 Bump thiserror from 1.0.50 to 1.0.51
   By dependabot[bot] in gfx-rs/wgpu#4890
 * #4880 Simplify `ResourceMaps`
   By nical in gfx-rs/wgpu#4880
 * #4891 Make the naga version in trunk as high as the latest published one
   By nical in gfx-rs/wgpu#4891
 * #4893 Avoid allocating memory every time we might log a label
   By nical in gfx-rs/wgpu#4893
 * #4894 Remove some locks in BindGroup
   By nical in gfx-rs/wgpu#4894
 * #4862 Ensure that DeviceLostCallbackC is always called exactly once
   By bradwerth in gfx-rs/wgpu#4862
 * #4900 Support Device fence sharing with dx12 on Windows
   By sotaroikeda in gfx-rs/wgpu#4900
 * #4903 Bump tokio from 1.35.0 to 1.35.1
   By dependabot[bot] in gfx-rs/wgpu#4903
 * #4895 Check that raw buffers and raw bind groups are valid
   By nical in gfx-rs/wgpu#4895
 * #4901 fix: docs
   By miaobuao in gfx-rs/wgpu#4901
 * #4892 Simplify some code around buffer unmapping
   By nical in gfx-rs/wgpu#4892
 * #4896 Buffer snatching
   By nical in gfx-rs/wgpu#4896
 * #4851 Eagerly release GPU resources when we lose the device.
   By bradwerth in gfx-rs/wgpu#4851
 * #4906 Use nightly for docs
   By cwfitzgerald in gfx-rs/wgpu#4906
 * #4908 Bump syn from 2.0.41 to 2.0.42
   By dependabot[bot] in gfx-rs/wgpu#4908
 * #4909 Bump profiling from 1.0.12 to 1.0.13
   By dependabot[bot] in gfx-rs/wgpu#4909
 * #4910 Bump anyhow from 1.0.75 to 1.0.76
   By dependabot[bot] in gfx-rs/wgpu#4910
 * #4913 Remove id32 Feature
   By cwfitzgerald in gfx-rs/wgpu#4913
 * #4914 Add BGL Deduplication Index Test
   By cwfitzgerald in gfx-rs/wgpu#4914
 * #4921 Fix typo "layout pipeline layout" -> "pipeline layout"
   By HactarCE in gfx-rs/wgpu#4921
 * #4922 Bump winit from 0.29.4 to 0.29.5
   By dependabot[bot] in gfx-rs/wgpu#4922
 * #4924 Inline `document-features` usage, remove dep.
   By ErichDonGubler in gfx-rs/wgpu#4924
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4930 Bump winit from 0.29.5 to 0.29.6
   By dependabot[bot] in gfx-rs/wgpu#4930
 * #4929 Bump web-time from 0.2.3 to 0.2.4
   By dependabot[bot] in gfx-rs/wgpu#4929
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4940 Align `wgpu_types::CompositeAlphaMode` serde serializations to spec
   By littledivy in gfx-rs/wgpu#4940
 * #4936 Bump anyhow from 1.0.76 to 1.0.77
   By dependabot[bot] in gfx-rs/wgpu#4936
 * #4933 Bump thiserror from 1.0.51 to 1.0.52
   By dependabot[bot] in gfx-rs/wgpu#4933
 * #4932 Bump syn from 2.0.42 to 2.0.43
   By dependabot[bot] in gfx-rs/wgpu#4932

Differential Revision: https://phabricator.services.mozilla.com/D197519
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Jan 5, 2024
…225e1744fd7. r=webgpu-reviewers,supply-chain-reviewers,teoxoy

Changelog:

 * #4865 fix present mode for wgl
   By xiaopengli89 in gfx-rs/wgpu#4865
 * #4873 Bump ctor from 0.2.5 to 0.2.6
   By dependabot[bot] in gfx-rs/wgpu#4873
 * #4874 Bump syn from 2.0.40 to 2.0.41
   By dependabot[bot] in gfx-rs/wgpu#4874
 * #4869 [naga wgsl-out] Include the `f` suffix on `f32` literals.
   By jimblandy in gfx-rs/wgpu#4869
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4850 [naga wgsl-in] Support abstract operands to binary operators.
   By jimblandy in gfx-rs/wgpu#4850
 * #4870 [naga wgsl] Let unary operators accept and produce abstract types.
   By jimblandy in gfx-rs/wgpu#4870
 * #4867 Reintroduce buffer snatching Part 1
   By nical in gfx-rs/wgpu#4867
 * #4882 Bump zerocopy from 0.7.26 to 0.7.31
   By dependabot[bot] in gfx-rs/wgpu#4882
 * #4878 Buffer snatching part 2 - Refactor create_buffer
   By nical in gfx-rs/wgpu#4878
 * #4815 Add `wgpu` crate features for backends
   By daxpedda in gfx-rs/wgpu#4815
 * #4887 Allow clippy::pattern_type_mismatch
   By nical in gfx-rs/wgpu#4887
 * #4886 Document wgpu & wgpu-core features
   By Wumpf in gfx-rs/wgpu#4886
 * #4826 validation: More detailed on incompatible BGL
   By scoopr in gfx-rs/wgpu#4826
 * #4888 Web: add support for more `RawWindowHandle` variants
   By daxpedda in gfx-rs/wgpu#4888
 * #4890 Bump thiserror from 1.0.50 to 1.0.51
   By dependabot[bot] in gfx-rs/wgpu#4890
 * #4880 Simplify `ResourceMaps`
   By nical in gfx-rs/wgpu#4880
 * #4891 Make the naga version in trunk as high as the latest published one
   By nical in gfx-rs/wgpu#4891
 * #4893 Avoid allocating memory every time we might log a label
   By nical in gfx-rs/wgpu#4893
 * #4894 Remove some locks in BindGroup
   By nical in gfx-rs/wgpu#4894
 * #4862 Ensure that DeviceLostCallbackC is always called exactly once
   By bradwerth in gfx-rs/wgpu#4862
 * #4900 Support Device fence sharing with dx12 on Windows
   By sotaroikeda in gfx-rs/wgpu#4900
 * #4903 Bump tokio from 1.35.0 to 1.35.1
   By dependabot[bot] in gfx-rs/wgpu#4903
 * #4895 Check that raw buffers and raw bind groups are valid
   By nical in gfx-rs/wgpu#4895
 * #4901 fix: docs
   By miaobuao in gfx-rs/wgpu#4901
 * #4892 Simplify some code around buffer unmapping
   By nical in gfx-rs/wgpu#4892
 * #4896 Buffer snatching
   By nical in gfx-rs/wgpu#4896
 * #4851 Eagerly release GPU resources when we lose the device.
   By bradwerth in gfx-rs/wgpu#4851
 * #4906 Use nightly for docs
   By cwfitzgerald in gfx-rs/wgpu#4906
 * #4908 Bump syn from 2.0.41 to 2.0.42
   By dependabot[bot] in gfx-rs/wgpu#4908
 * #4909 Bump profiling from 1.0.12 to 1.0.13
   By dependabot[bot] in gfx-rs/wgpu#4909
 * #4910 Bump anyhow from 1.0.75 to 1.0.76
   By dependabot[bot] in gfx-rs/wgpu#4910
 * #4913 Remove id32 Feature
   By cwfitzgerald in gfx-rs/wgpu#4913
 * #4914 Add BGL Deduplication Index Test
   By cwfitzgerald in gfx-rs/wgpu#4914
 * #4921 Fix typo "layout pipeline layout" -> "pipeline layout"
   By HactarCE in gfx-rs/wgpu#4921
 * #4922 Bump winit from 0.29.4 to 0.29.5
   By dependabot[bot] in gfx-rs/wgpu#4922
 * #4924 Inline `document-features` usage, remove dep.
   By ErichDonGubler in gfx-rs/wgpu#4924
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4871 Speed up Naga's `cargo xtask validate wgsl` from 12s to 0.8s
   By jimblandy in gfx-rs/wgpu#4871
 * #4930 Bump winit from 0.29.5 to 0.29.6
   By dependabot[bot] in gfx-rs/wgpu#4930
 * #4929 Bump web-time from 0.2.3 to 0.2.4
   By dependabot[bot] in gfx-rs/wgpu#4929
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4902 [naga xtask] Run validation jobs in parallel, using jobserver.
   By jimblandy in gfx-rs/wgpu#4902
 * #4940 Align `wgpu_types::CompositeAlphaMode` serde serializations to spec
   By littledivy in gfx-rs/wgpu#4940
 * #4936 Bump anyhow from 1.0.76 to 1.0.77
   By dependabot[bot] in gfx-rs/wgpu#4936
 * #4933 Bump thiserror from 1.0.51 to 1.0.52
   By dependabot[bot] in gfx-rs/wgpu#4933
 * #4932 Bump syn from 2.0.42 to 2.0.43
   By dependabot[bot] in gfx-rs/wgpu#4932

Differential Revision: https://phabricator.services.mozilla.com/D197519
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.

3 participants