implements #8119 : Metal backend's wgpu_hal::Device::wait implementat…

[39ali]

…ion polls instead of waiting

Connections
#8119

Description
this uses a CondVar and lets the thread sleep instead of polling every 1ms

Testing
ran poll tests

Squash or Rebase?
either

Checklist

• Run cargo fmt.
• Run taplo format.
• Run cargo clippy --tests. If applicable, add:
  • --target wasm32-unknown-unknown
• Run cargo xtask test to run tests.
• If this contains user-facing changes, add a CHANGELOG.md entry.

[39ali]

If timeout is provided, the function will block indefinitely or until , is this a typo ?, should be If timeout is NOT provided

[39ali]

@jimblandy

[inner-daemons]

Looks good overall. 2 questions. Also going to CC @cwfitzgerald because this will almost certainly have to be reassigned

[inner-daemons]

I'm skeptical about why this is Mutex<()> instead of just keeping track of the last completed submission index directly like Mutex<u64>.

[andyleiserson]

There is also pub type AtomicFenceValue = core::sync::atomic::AtomicU64

[inner-daemons]

@andyleiserson I get that but I feel like that could be replaced at this point, combined into the mutex

[inner-daemons]

Should the sync really be shared between devices in the same adapter?

[39ali]

made couple of changes that should address both comments :

• Value inside the mutex (correctness / clarity)
  Previously, completed_value was an AtomicU64 that lived outside the mutex, kinda an anti-pattern, now Mutex<FenceValue> directly guards the value, which is the canonical condvar usage.

• Sync is now per-fence (reduced spurious wakeups)
  Previously sync lived on AdapterShared, shared across every fence submitted to any device from that adapter. When any command buffer completed, it called notify_all() on that shared condvar, waking every thread waiting on any fence from that adapter, even completely unrelated ones. They'd all re-check their predicate and go back to sleep. Now each Fence owns its own condvar, so a completion only wakes threads actually waiting on that fence.

[andyleiserson]

It pains me a little to replace an Atomic with a Mutex, but this is the right way of using the Condvar, and I don't think it's worth speculatively adding a shadow copy in an atomic.

[andyleiserson]

I think it would be better to unwrap here on overflow. Or maybe use wait_while_for so we don't have to do timeout accounting ourselves (unless our build configuration doesn't support wait_for?).

[andyleiserson]

I noted in #9532 (comment) that this check isn't correct -- it should be looking for MTLCommandBufferStatus::Error as well. Although, I'm also not sure why it's necessary to scan for additional completed command buffers here, maybe we could remove this entirely.