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

wgpu-hal dynamic dispatch, towards fewer generics in wgpu-core #6002

Closed
wants to merge 63 commits into from

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Jul 20, 2024

Connections

Description

Still heavy work in progress!


This latest attempt to degenerification of wgpu core tries to be as straight forward as possible:

  • introduce dynamic base types that cast to concrete types in HAL (DynDevice, DynCommandEncoder etc.)
  • wherever wgpu-core passes around &A::CommandEncoder (and similar) instead pass around & dyn DynCommandEncoder
  • start using a lot of Box<&dyn DynBuffer> (and similar)
  • drop <A: hal::API> from all the wgpu-core types
  • profit!

Previous more complex attempts tried to avoid boxing as much as possible, I'm deliberately doing here the opposite, well aware that this may cost us! But nobody actually measured how much yet and neither have I, so the jury is still out on this.

The hope is that this approach is simple enough that we can pull it off and cheap enough that we can worry about optimizing things later (it's not too hard to come up with ideas on how to tackle ensuing costs, we just have to know what to optimize first!).


I'm keeping this in a fairly landable state at every step (minus Metal & web things which I'm not testing all the time) and try to keep steps granular, BUT....

  • there's still quite a bit of experimentation going on where I have to go back and change some things
    • I may or may not clean up the commit history to eliminate a few things where I went in circles.
  • this overall approach has the risk of degrading performance more than we're willing to pay (we are willing to pay a certain amount, since status quo is not great for all the reasons described in Make wgpu-core less generic (brainstorm) #5124), but it's hard to tell if it's bad or not without a full vertical slice
    • I may end up getting this almost to completion and then re-open a series of smaller, more reviewable PRs to land it

Testing
Hopefully covered by existing tests ;-)

Checklist

  • Run cargo fmt.
  • Run cargo clippy. If applicable, add:
    • --target wasm32-unknown-unknown
    • --target wasm32-unknown-emscripten
  • Run cargo xtask test to run tests.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@Wumpf Wumpf changed the title HAL dynamic dispatch towards fewer generics in wgpu-core wgpu-hal dynamic dispatch towards fewer generics in wgpu-core Jul 20, 2024
@Wumpf Wumpf changed the title wgpu-hal dynamic dispatch towards fewer generics in wgpu-core wgpu-hal dynamic dispatch, towards fewer generics in wgpu-core Jul 20, 2024
@Wumpf Wumpf force-pushed the dynamic-hal-dispatch branch 3 times, most recently from e73fb15 to 8a9cbee Compare July 24, 2024 07:45
@Wumpf Wumpf force-pushed the dynamic-hal-dispatch branch 5 times, most recently from 739adb9 to a498175 Compare August 3, 2024 10:29
@Wumpf Wumpf force-pushed the dynamic-hal-dispatch branch 9 times, most recently from 19ba1f1 to a2bb962 Compare August 10, 2024 10:10
Wumpf added 27 commits August 10, 2024 13:06
…ffects from there leading to boxing of almost all hal resources
gfx_select macros are empty husks now that are waiting to be removed
@Wumpf Wumpf force-pushed the dynamic-hal-dispatch branch from a2bb962 to bef23ee Compare August 10, 2024 12:17
@Wumpf
Copy link
Member Author

Wumpf commented Aug 10, 2024

closing in favor of 3 part pr series coming up soon!

@Wumpf Wumpf closed this Aug 10, 2024
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.

1 participant