Defer command buffer recycling until allocate#429
Defer command buffer recycling until allocate#429bors[bot] merged 2 commits intogfx-rs:masterfrom LaylBongers:master
Conversation
|
Please rebase instead of introducing the merge commit |
wgpu-core/src/device.rs
Outdated
| ); | ||
| { | ||
| let mut last_done_guard = self.last_done.lock(); | ||
| *last_done_guard = last_done_guard.max(last_done); |
There was a problem hiding this comment.
I don't think we need a Mutex here. After this function is called in submit, the device is re-locked for writing, so we can just assign the value in there directly.
There was a problem hiding this comment.
device.pending is locked but the device itself isn't given a write a lock. I could introduce a write lock between the maintain call and further uses of the device, if that works better.
There was a problem hiding this comment.
Ok, I think I know what we can do to avoid this field. At any point, we can consider active[0].index to be the first undone submission index. So we can take that and pass it to the pool.maintain() as a different semantics (first undone), and in side the maintain() implementation just change the <= last_done comparison to < first_undone. How does that sound?
There was a problem hiding this comment.
Sounds good to me, seems straightforward to implement as well.
wgpu-core/src/device.rs
Outdated
| ); | ||
| { | ||
| let mut last_done_guard = self.last_done.lock(); | ||
| *last_done_guard = last_done_guard.max(last_done); |
There was a problem hiding this comment.
Ok, I think I know what we can do to avoid this field. At any point, we can consider active[0].index to be the first undone submission index. So we can take that and pass it to the pool.maintain() as a different semantics (first undone), and in side the maintain() implementation just change the <= last_done comparison to < first_undone. How does that sound?
| // The first entry in the active list should have the lowest index | ||
| let lowest_active_index = device.pending.lock() | ||
| .active.get(0) | ||
| .map(|active| active.index) |
429: Defer command buffer recycling until allocate r=kvark a=LaylConway As discussed on Matrix, this change defers command buffer recycling until allocation. This lets a rendering thread clean up its own command buffers, making sure only the owning thread accesses anything from its command pool. I have also added a TODO comment about cleaning up thread pools, which currently isn't happening as far as I can tell. This previously was an issue as well but now is a slightly bigger one as the command buffer wouldn't get recycled either. I figured that, for now, fixing that issue is out of the scope of this PR. This PR focuses only on resolving the race condition causing the validation error. This fixes #423 Co-authored-by: Layl <2385329-layl@users.noreply.gitlab.com>
Build succeededAnd happy new year from bors! 🎉 |
As discussed on Matrix, this change defers command buffer recycling until allocation.
This lets a rendering thread clean up its own command buffers, making sure only the owning thread accesses anything from its command pool.
I have also added a TODO comment about cleaning up thread pools, which currently isn't happening as far as I can tell. This previously was an issue as well but now is a slightly bigger one as the command buffer wouldn't get recycled either.
I figured that, for now, fixing that issue is out of the scope of this PR. This PR focuses only on resolving the race condition causing the validation error.
This fixes #423