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

Properly Deal with Timeouts Inside device::maintain #5012

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

cwfitzgerald
Copy link
Member

@cwfitzgerald cwfitzgerald commented Jan 8, 2024

Connections

Closes #4589
Closes #3601

Description

We were not properly dealing with polls which timed out. This exposed the concept of timeouts to the user so that they can properly deal with polls which time out.

This doesn't fix anything with timeouts inside surface.configure.

Testing

Added timeout test.

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.

@cwfitzgerald cwfitzgerald requested a review from a team January 8, 2024 03:18
@cwfitzgerald cwfitzgerald requested a review from a team as a code owner January 8, 2024 03:18
@cwfitzgerald cwfitzgerald force-pushed the device-timeouts branch 4 times, most recently from 2589ad5 to a497bf1 Compare January 8, 2024 06:01
@cwfitzgerald cwfitzgerald marked this pull request as draft January 8, 2024 07:43
@cwfitzgerald
Copy link
Member Author

cwfitzgerald commented Jan 8, 2024

Pulled this into draft as it breaks all of our WebGL2 tests.

tl;dr: the whole reason device.poll(Wait) worked on WebGL2 is because of a the very logic bug that this fixes. WebGL2 is always treated as a timeout, so it always treats the wait as finished. Which is completely UB on other backends, but on WebGL2 it doesn't matter as there's no way to observe the lack of it actually finishing.

Going to need to:

  • Add a feature to make poll(Wait) fail validation.
  • Add an async helper function for a cross platform poll(Wait)
  • Switch all existing polls to that function.

@cwfitzgerald
Copy link
Member Author

I still want to deal with this, it was just a much larger task than I initially thought.

@teoxoy
Copy link
Member

teoxoy commented Jul 26, 2024

This should also include the removal of the expected failure on MoltenVK (f38bdd8).

@thalisvilela
Copy link

My first time working with WebGPU today, trying to implement heavy slow compute shader workload, as i set up and tested everything, as soon i started throwing real work at it, stumbled upon this issue immediately... Not my lucky day...

@Wumpf
Copy link
Member

Wumpf commented Aug 2, 2024

@thalisvilela fwiw this issue does not affect WebGPU but all native backends + WebGL, but that's probably what you meant :)

@thalisvilela
Copy link

@Wumpf thank you very much for the tip! I would have probably give a shot at these other methods, you just saved me from a lot of frustration! It's the first time i'm working directly with "shaders", i've mostly used CUDA before. I'm glad this issue surfaced immediately to be honest.

@torokati44

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants