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

Weakly-counted types #6989

Open
szostid opened this issue Jan 25, 2025 · 4 comments
Open

Weakly-counted types #6989

szostid opened this issue Jan 25, 2025 · 4 comments
Labels
area: api Issues related to API surface type: enhancement New feature or request

Comments

@szostid
Copy link

szostid commented Jan 25, 2025

Since the introduction of clonable types in wgpu 24.0, I've been able to remove all my Arc<T> wrappers around types like Buffer, Texture, but there are no corresponding Weak<T> types, which is a bit of an issue.

I've decided to add weakly-counted reference types under the feature weak-references for most of the dispatch types, skipping DispatchSurface (it's wrapper Surface is not Clone) and DispatchSurfaceOutputDetail (it's wrapper contains suboptimal and presented, and those could be mutated independently of the non-weak SurfaceTexture itself), but the necessity of some of those types, like AdapterWeak is a bit dubious, and this is why I've not opened a PR for this. I'm also uncertain about the naming of the types which are reponsible for the weak <-> strong transfers, (its as_weak and upgrade for now, as_weak is also an associated functions unlike the one in Arc). Adding more docs is not an issue.

This is the fork in which I've implemented the feature:
https://github.com/szostid/wgpu

PS: its my first time making a commit like this, any suggestions are helpful

@Wumpf
Copy link
Member

Wumpf commented Jan 25, 2025

I like the idea as such, but the main issue I see is that in the long run we'd actually wanted to be able to leverage the existing tracking inside of wgpu-core more. Allowing weak referencing on top will make such future efforts a lot more complicated & constrained.

but there are no corresponding Weak types, which is a bit of an issue.

Probably worth diving more into this: can you describe use-cases for which weak references would be useful for wgpu resources?

@kpreid
Copy link
Contributor

kpreid commented Jan 25, 2025

I had a use case for weak references, which was “poll this Weak<Device> as long as it continues to exist and have unfinished work”. I managed to make it work in a different and arguably better way, but with a more complex data structure.

Another use cases I can imagine would be caching/memoizing derived resources, e.g. “given this Weak<Buffer>, look up the BindGroup that binds it and some other stuff”. In my own code, such caches are so far all single-element so they don’t need weak referencing to avoid leaks, but that isn’t a fundamental property.

@szostid
Copy link
Author

szostid commented Jan 25, 2025

I use weakly-counted types to keep weak references to lazily-loaded resources like textures.

In an async function, I would, for example, await a web request, and after it finishes I would write to a texture, but I do not want to keep a strong reference to that texture, to avoid unnecessary writes if the texture was already destroyed.

This is also why I've said that the necessity of stuff like InstanceWeak is dubious - I can't think of any use cases of Weak references to some of the types, but I've decided to implement them for consistency anyways.

@cwfitzgerald cwfitzgerald added type: enhancement New feature or request area: api Issues related to API surface labels Jan 27, 2025
@Wumpf
Copy link
Member

Wumpf commented Jan 29, 2025

We talked about this briefly in the weekly wgpu maintainer meeting. Conclusion was that we're right now a little bit too nervous of introducing weak handles since there's still too much churn on the registries inside of wgpu-core: Right now there's an extra Arc layer in wgpu when it should actually re-use reference counting done in wgpu-core.

That said, it's more likely than not that we then can just use weak handles to those Arc inside of wgpu-core, so it should work out! 🙂
But there's some uncertainty around potentially surprising behavior, for instance it could then happen that as a user you can drop all strong handles but your weak handles might still work since wgpu-core may or may not keep around the objects a little bit longer etc.

Long story short: we want this, but first the extra layer of Arc should go away

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API surface type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants