Skip to content

Commit

Permalink
Avoid recursive snatch lock acquisitions
Browse files Browse the repository at this point in the history
  • Loading branch information
SludgePhD committed Mar 22, 2024
1 parent 00e0e72 commit 315bbf3
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 32 deletions.
19 changes: 11 additions & 8 deletions wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ use crate::{
pipeline::{PipelineFlags, RenderPipeline, VertexStep},
resource::{Buffer, Resource, ResourceInfo, ResourceType},
resource_log,
snatch::SnatchGuard,
track::RenderBundleScope,
validation::check_buffer_usage,
Label, LabelHelpers,
Expand Down Expand Up @@ -894,7 +895,11 @@ impl<A: HalApi> RenderBundle<A> {
/// Note that the function isn't expected to fail, generally.
/// All the validation has already been done by this point.
/// The only failure condition is if some of the used buffers are destroyed.
pub(super) unsafe fn execute(&self, raw: &mut A::CommandEncoder) -> Result<(), ExecutionError> {
pub(super) unsafe fn execute(
&self,
raw: &mut A::CommandEncoder,
snatch_guard: &SnatchGuard,
) -> Result<(), ExecutionError> {
let mut offsets = self.base.dynamic_offsets.as_slice();
let mut pipeline_layout = None::<Arc<PipelineLayout<A>>>;
if !self.discard_hal_labels {
Expand All @@ -903,8 +908,6 @@ impl<A: HalApi> RenderBundle<A> {
}
}

let snatch_guard = self.device.snatchable_lock.read();

use ArcRenderCommand as Cmd;
for command in self.base.commands.iter() {
match command {
Expand All @@ -914,7 +917,7 @@ impl<A: HalApi> RenderBundle<A> {
bind_group,
} => {
let raw_bg = bind_group
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or(ExecutionError::InvalidBindGroup(bind_group.info.id()))?;
unsafe {
raw.set_bind_group(
Expand All @@ -938,7 +941,7 @@ impl<A: HalApi> RenderBundle<A> {
size,
} => {
let buffer: &A::Buffer = buffer
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or(ExecutionError::DestroyedBuffer(buffer.info.id()))?;
let bb = hal::BufferBinding {
buffer,
Expand All @@ -954,7 +957,7 @@ impl<A: HalApi> RenderBundle<A> {
size,
} => {
let buffer = buffer
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or(ExecutionError::DestroyedBuffer(buffer.info.id()))?;
let bb = hal::BufferBinding {
buffer,
Expand Down Expand Up @@ -1041,7 +1044,7 @@ impl<A: HalApi> RenderBundle<A> {
indexed: false,
} => {
let buffer = buffer
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or(ExecutionError::DestroyedBuffer(buffer.info.id()))?;
unsafe { raw.draw_indirect(buffer, *offset, 1) };
}
Expand All @@ -1052,7 +1055,7 @@ impl<A: HalApi> RenderBundle<A> {
indexed: true,
} => {
let buffer = buffer
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or(ExecutionError::DestroyedBuffer(buffer.info.id()))?;
unsafe { raw.draw_indexed_indirect(buffer, *offset, 1) };
}
Expand Down
7 changes: 5 additions & 2 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
id::{BufferId, CommandEncoderId, DeviceId, TextureId},
init_tracker::{MemoryInitKind, TextureInitRange},
resource::{Resource, Texture, TextureClearMode},
snatch::SnatchGuard,
track::{TextureSelector, TextureTracker},
};

Expand Down Expand Up @@ -239,6 +240,7 @@ impl Global {
}
let (encoder, tracker) = cmd_buf_data.open_encoder_and_tracker()?;

let snatch_guard = device.snatchable_lock.read();
clear_texture(
&dst_texture,
TextureInitRange {
Expand All @@ -249,6 +251,7 @@ impl Global {
&mut tracker.textures,
&device.alignments,
device.zero_buffer.as_ref().unwrap(),
&snatch_guard,
)
}
}
Expand All @@ -260,10 +263,10 @@ pub(crate) fn clear_texture<A: HalApi>(
texture_tracker: &mut TextureTracker<A>,
alignments: &hal::Alignments,
zero_buffer: &A::Buffer,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), ClearError> {
let snatch_guard = dst_texture.device.snatchable_lock.read();
let dst_raw = dst_texture
.raw(&snatch_guard)
.raw(snatch_guard)
.ok_or_else(|| ClearError::InvalidTexture(dst_texture.as_info().id()))?;

// Issue the right barrier.
Expand Down
1 change: 1 addition & 0 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ impl Global {
transit,
&mut tracker.textures,
device,
&snatch_guard,
);
CommandBuffer::insert_barriers_from_tracker(
transit,
Expand Down
11 changes: 8 additions & 3 deletions wgpu-core/src/command/memory_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::{
hal_api::HalApi,
init_tracker::*,
resource::{Resource, Texture},
snatch::SnatchGuard,
track::{TextureTracker, Tracker},
FastHashMap,
};
Expand Down Expand Up @@ -144,6 +145,7 @@ pub(crate) fn fixup_discarded_surfaces<
encoder: &mut A::CommandEncoder,
texture_tracker: &mut TextureTracker<A>,
device: &Device<A>,
snatch_guard: &SnatchGuard<'_>,
) {
for init in inits {
clear_texture(
Expand All @@ -156,6 +158,7 @@ pub(crate) fn fixup_discarded_surfaces<
texture_tracker,
&device.alignments,
device.zero_buffer.as_ref().unwrap(),
snatch_guard,
)
.unwrap();
}
Expand All @@ -167,6 +170,7 @@ impl<A: HalApi> BakedCommands<A> {
pub(crate) fn initialize_buffer_memory(
&mut self,
device_tracker: &mut Tracker<A>,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), DestroyedBufferError> {
// Gather init ranges for each buffer so we can collapse them.
// It is not possible to do this at an earlier point since previously
Expand Down Expand Up @@ -225,16 +229,15 @@ impl<A: HalApi> BakedCommands<A> {
.unwrap()
.1;

let snatch_guard = buffer.device.snatchable_lock.read();
let raw_buf = buffer
.raw
.get(&snatch_guard)
.get(snatch_guard)
.ok_or(DestroyedBufferError(buffer_id))?;

unsafe {
self.encoder.transition_buffers(
transition
.map(|pending| pending.into_hal(&buffer, &snatch_guard))
.map(|pending| pending.into_hal(&buffer, snatch_guard))
.into_iter(),
);
}
Expand Down Expand Up @@ -271,6 +274,7 @@ impl<A: HalApi> BakedCommands<A> {
&mut self,
device_tracker: &mut Tracker<A>,
device: &Device<A>,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), DestroyedTextureError> {
let mut ranges: Vec<TextureInitRange> = Vec::new();
for texture_use in self.texture_memory_actions.drain_init_actions() {
Expand Down Expand Up @@ -310,6 +314,7 @@ impl<A: HalApi> BakedCommands<A> {
&mut device_tracker.textures,
&device.alignments,
device.zero_buffer.as_ref().unwrap(),
snatch_guard,
);

// A Texture can be destroyed between the command recording
Expand Down
3 changes: 2 additions & 1 deletion wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2374,7 +2374,7 @@ impl Global {
.extend(texture_memory_actions.register_init_action(action));
}

unsafe { bundle.execute(raw) }
unsafe { bundle.execute(raw, &snatch_guard) }
.map_err(|e| match e {
ExecutionError::DestroyedBuffer(id) => {
RenderCommandError::DestroyedBuffer(id)
Expand Down Expand Up @@ -2427,6 +2427,7 @@ impl Global {
transit,
&mut tracker.textures,
&cmd_buf.device,
&snatch_guard,
);

cmd_buf_data
Expand Down
19 changes: 15 additions & 4 deletions wgpu-core/src/command/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::{
TextureInitTrackerAction,
},
resource::{Resource, Texture, TextureErrorDimension},
snatch::SnatchGuard,
track::{TextureSelector, Tracker},
};

Expand Down Expand Up @@ -452,6 +453,7 @@ fn handle_texture_init<A: HalApi>(
copy_texture: &ImageCopyTexture,
copy_size: &Extent3d,
texture: &Arc<Texture<A>>,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), ClearError> {
let init_action = TextureInitTrackerAction {
texture: texture.clone(),
Expand Down Expand Up @@ -480,6 +482,7 @@ fn handle_texture_init<A: HalApi>(
&mut trackers.textures,
&device.alignments,
device.zero_buffer.as_ref().unwrap(),
snatch_guard,
)?;
}
}
Expand All @@ -499,6 +502,7 @@ fn handle_src_texture_init<A: HalApi>(
source: &ImageCopyTexture,
copy_size: &Extent3d,
texture: &Arc<Texture<A>>,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), TransferError> {
handle_texture_init(
MemoryInitKind::NeedsInitializedMemory,
Expand All @@ -509,6 +513,7 @@ fn handle_src_texture_init<A: HalApi>(
source,
copy_size,
texture,
snatch_guard,
)?;
Ok(())
}
Expand All @@ -525,6 +530,7 @@ fn handle_dst_texture_init<A: HalApi>(
destination: &ImageCopyTexture,
copy_size: &Extent3d,
texture: &Arc<Texture<A>>,
snatch_guard: &SnatchGuard<'_>,
) -> Result<(), TransferError> {
// Attention: If we don't write full texture subresources, we need to a full
// clear first since we don't track subrects. This means that in rare cases
Expand All @@ -549,6 +555,7 @@ fn handle_dst_texture_init<A: HalApi>(
destination,
copy_size,
texture,
snatch_guard,
)?;
Ok(())
}
Expand Down Expand Up @@ -779,6 +786,8 @@ impl Global {

let (dst_range, dst_base) = extract_texture_selector(destination, copy_size, &dst_texture)?;

let snatch_guard = device.snatchable_lock.read();

// Handle texture init *before* dealing with barrier transitions so we
// have an easier time inserting "immediate-inits" that may be required
// by prior discards in rare cases.
Expand All @@ -790,10 +799,9 @@ impl Global {
destination,
copy_size,
&dst_texture,
&snatch_guard,
)?;

let snatch_guard = device.snatchable_lock.read();

let (src_buffer, src_pending) = {
let buffer_guard = hub.buffers.read();
let src_buffer = buffer_guard
Expand Down Expand Up @@ -935,6 +943,8 @@ impl Global {

let (src_range, src_base) = extract_texture_selector(source, copy_size, &src_texture)?;

let snatch_guard = device.snatchable_lock.read();

// Handle texture init *before* dealing with barrier transitions so we
// have an easier time inserting "immediate-inits" that may be required
// by prior discards in rare cases.
Expand All @@ -946,10 +956,9 @@ impl Global {
source,
copy_size,
&src_texture,
&snatch_guard,
)?;

let snatch_guard = device.snatchable_lock.read();

let src_pending = tracker
.textures
.set_single(&src_texture, src_range, hal::TextureUses::COPY_SRC)
Expand Down Expand Up @@ -1152,6 +1161,7 @@ impl Global {
source,
copy_size,
&src_texture,
&snatch_guard,
)?;
handle_dst_texture_init(
encoder,
Expand All @@ -1161,6 +1171,7 @@ impl Global {
destination,
copy_size,
&dst_texture,
&snatch_guard,
)?;

let src_pending = cmd_buf_data
Expand Down
16 changes: 13 additions & 3 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,19 @@ impl Global {
hal::BufferUses::empty()
} else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
// buffer is mappable, so we are just doing that at start
let snatch_guard = device.snatchable_lock.read();
let map_size = buffer.size;
let ptr = if map_size == 0 {
std::ptr::NonNull::dangling()
} else {
match map_buffer(device.raw(), &buffer, 0, map_size, HostMap::Write) {
match map_buffer(
device.raw(),
&buffer,
0,
map_size,
HostMap::Write,
&snatch_guard,
) {
Ok(ptr) => ptr,
Err(e) => {
to_destroy.push(buffer);
Expand Down Expand Up @@ -2008,9 +2016,10 @@ impl Global {
}

// Wait for all work to finish before configuring the surface.
let snatch_guard = device.snatchable_lock.read();
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
match device.maintain(fence, wgt::Maintain::Wait) {
match device.maintain(fence, wgt::Maintain::Wait, &snatch_guard) {
Ok((closures, _)) => {
user_callbacks = closures;
}
Expand Down Expand Up @@ -2120,9 +2129,10 @@ impl Global {
device: &crate::device::Device<A>,
maintain: wgt::Maintain<queue::WrappedSubmissionIndex>,
) -> Result<DevicePoll, WaitIdleError> {
let snatch_guard = device.snatchable_lock.read();
let fence = device.fence.read();
let fence = fence.as_ref().unwrap();
let (closures, queue_empty) = device.maintain(fence, maintain)?;
let (closures, queue_empty) = device.maintain(fence, maintain, &snatch_guard)?;

// Some deferred destroys are scheduled in maintain so run this right after
// to avoid holding on to them until the next device poll.
Expand Down
11 changes: 10 additions & 1 deletion wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
self, Buffer, DestroyedBuffer, DestroyedTexture, QuerySet, Resource, Sampler,
StagingBuffer, Texture, TextureView,
},
snatch::SnatchGuard,
track::{ResourceTracker, Tracker, TrackerIndex},
FastHashMap, SubmissionIndex,
};
Expand Down Expand Up @@ -780,6 +781,7 @@ impl<A: HalApi> LifetimeTracker<A> {
&mut self,
raw: &A::Device,
trackers: &Mutex<Tracker<A>>,
snatch_guard: &SnatchGuard,
) -> Vec<super::BufferMapPendingClosure> {
if self.ready_to_map.is_empty() {
return Vec::new();
Expand Down Expand Up @@ -816,7 +818,14 @@ impl<A: HalApi> LifetimeTracker<A> {
log::debug!("Buffer {tracker_index:?} map state -> Active");
let host = mapping.op.host;
let size = mapping.range.end - mapping.range.start;
match super::map_buffer(raw, &buffer, mapping.range.start, size, host) {
match super::map_buffer(
raw,
&buffer,
mapping.range.start,
size,
host,
snatch_guard,
) {
Ok(ptr) => {
*buffer.map_state.lock() = resource::BufferMapState::Active {
ptr,
Expand Down
Loading

0 comments on commit 315bbf3

Please sign in to comment.