From 604d120ec23a767824001ac92ac1ba76fbdeacab Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Mon, 16 Jul 2018 08:39:00 -0400 Subject: [PATCH 1/2] [mtl] reset command pool properly --- src/backend/metal/src/command.rs | 33 +++++++++++--------------------- src/backend/metal/src/device.rs | 19 +++++------------- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index d36912ffb4a..17330d0e202 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -115,7 +115,7 @@ type CommandBufferInnerPtr = Arc>; pub struct CommandPool { pub(crate) shared: Arc, - pub(crate) managed: Option>, + pub(crate) allocated: Vec, } unsafe impl Send for CommandPool {} @@ -889,12 +889,9 @@ impl Drop for CommandBufferInner { impl CommandBufferInner { pub(crate) fn reset(&mut self, shared: &Shared) { - match self.sink.take() { - Some(CommandSink::Immediate { token, mut encoder_state, .. }) => { - encoder_state.end(); - shared.queue.lock().unwrap().release(token); - } - _ => {} + if let Some(CommandSink::Immediate { token, mut encoder_state, .. }) = self.sink.take() { + encoder_state.end(); + shared.queue.lock().unwrap().release(token); } self.retained_buffers.clear(); self.retained_textures.clear(); @@ -1446,12 +1443,10 @@ impl RawCommandQueue for CommandQueue { impl pool::RawCommandPool for CommandPool { fn reset(&mut self) { - if let Some(ref mut managed) = self.managed { - for cmd_buffer in managed { - cmd_buffer - .borrow_mut() - .reset(&self.shared); - } + for cmd_buffer in &self.allocated { + cmd_buffer + .borrow_mut() + .reset(&self.shared); } } @@ -1506,9 +1501,7 @@ impl pool::RawCommandPool for CommandPool { }, }).collect(); - if let Some(ref mut managed) = self.managed { - managed.extend(buffers.iter().map(|buf| buf.inner.clone())); - } + self.allocated.extend(buffers.iter().map(|buf| buf.inner.clone())); buffers } @@ -1518,14 +1511,10 @@ impl pool::RawCommandPool for CommandPool { for buf in &mut buffers { buf.reset(true); } - let managed = match self.managed { - Some(ref mut vec) => vec, - None => return, - }; for cmd_buf in buffers { - match managed.iter_mut().position(|b| Arc::ptr_eq(b, &cmd_buf.inner)) { + match self.allocated.iter_mut().position(|b| Arc::ptr_eq(b, &cmd_buf.inner)) { Some(index) => { - managed.swap_remove(index); + self.allocated.swap_remove(index); } None => { error!("Unable to free a command buffer!") diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 779fbd3629d..3b2fd5a83f8 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -583,26 +583,17 @@ impl Device { impl hal::Device for Device { fn create_command_pool( - &self, _family: QueueFamilyId, flags: CommandPoolCreateFlags + &self, _family: QueueFamilyId, _flags: CommandPoolCreateFlags ) -> command::CommandPool { command::CommandPool { shared: self.shared.clone(), - managed: if flags.contains(CommandPoolCreateFlags::RESET_INDIVIDUAL) { - None - } else { - Some(Vec::new()) - }, + allocated: Vec::new(), } } - fn destroy_command_pool(&self, pool: command::CommandPool) { - if let Some(vec) = pool.managed { - for cmd_buf in vec { - cmd_buf - .borrow_mut() - .reset(&self.shared); - } - } + fn destroy_command_pool(&self, mut pool: command::CommandPool) { + use hal::pool::RawCommandPool; + pool.reset(); } fn create_render_pass<'a, IA, IS, ID>( From 674de26b8cc1f5327d35446368ccfb4e21bce96b Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Mon, 16 Jul 2018 10:30:44 -0400 Subject: [PATCH 2/2] [mtl] lighter locks on descriptor allocation --- src/backend/metal/src/command.rs | 20 ++--- src/backend/metal/src/device.rs | 5 +- src/backend/metal/src/native.rs | 133 +++++++++++++++++-------------- 3 files changed, 85 insertions(+), 73 deletions(-) diff --git a/src/backend/metal/src/command.rs b/src/backend/metal/src/command.rs index 17330d0e202..28a37f73905 100644 --- a/src/backend/metal/src/command.rs +++ b/src/backend/metal/src/command.rs @@ -2648,7 +2648,7 @@ impl com::RawCommandBuffer for CommandBuffer { for (set_index, desc_set) in sets.into_iter().enumerate() { match *desc_set.borrow() { native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { - let pool = pool.read().unwrap(); + let data = pool.read().unwrap(); let mut sampler_base = sampler_range.start as usize; let mut texture_base = texture_range.start as usize; let mut buffer_base = buffer_range.start as usize; @@ -2662,7 +2662,7 @@ impl com::RawCommandBuffer for CommandBuffer { if buffer_base != bf_range.start { dynamic_offsets.clear(); - for bref in &pool.buffers[bf_range.clone()] { + for bref in &data.buffers[bf_range.clone()] { if bref.base.is_some() && bref.dynamic { dynamic_offsets.push(*offset_iter .next() @@ -2700,7 +2700,7 @@ impl com::RawCommandBuffer for CommandBuffer { debug_assert_eq!(sampler_base, sm_range.end); resources.set_samplers( pipe_layout.res_overrides[&loc].sampler_id as usize, - &pool.samplers[sm_range.clone()], + &data.samplers[sm_range.clone()], |index, sampler| { pre.issue(soft::RenderCommand::BindSampler { stage, index, sampler }); }, @@ -2710,7 +2710,7 @@ impl com::RawCommandBuffer for CommandBuffer { debug_assert_eq!(texture_base, tx_range.end); resources.set_textures( pipe_layout.res_overrides[&loc].texture_id as usize, - &pool.textures[tx_range.clone()], + &data.textures[tx_range.clone()], |index, texture| { pre.issue(soft::RenderCommand::BindTexture { stage, index, texture }); }, @@ -2718,7 +2718,7 @@ impl com::RawCommandBuffer for CommandBuffer { } if buffer_base != bf_range.start { debug_assert_eq!(buffer_base, bf_range.end); - let buffers = &pool.buffers[bf_range.clone()]; + let buffers = &data.buffers[bf_range.clone()]; let start = pipe_layout.res_overrides[&loc].buffer_id as usize; let mut dynamic_index = 0; for (i, bref) in buffers.iter().enumerate() { @@ -2824,7 +2824,7 @@ impl com::RawCommandBuffer for CommandBuffer { }]; match *desc_set.borrow() { native::DescriptorSet::Emulated { ref pool, ref layouts, ref sampler_range, ref texture_range, ref buffer_range } => { - let pool = pool.read().unwrap(); + let data = pool.read().unwrap(); let mut sampler_base = sampler_range.start as usize; let mut texture_base = texture_range.start as usize; let mut buffer_base = buffer_range.start as usize; @@ -2838,7 +2838,7 @@ impl com::RawCommandBuffer for CommandBuffer { if buffer_base != bf_range.start { dynamic_offsets.clear(); - for bref in &pool.buffers[bf_range.clone()] { + for bref in &data.buffers[bf_range.clone()] { if bref.base.is_some() && bref.dynamic { dynamic_offsets.push(*offset_iter .next() @@ -2853,7 +2853,7 @@ impl com::RawCommandBuffer for CommandBuffer { debug_assert_eq!(sampler_base, sm_range.end); resources.set_samplers( res_override.sampler_id as usize, - &pool.samplers[sm_range], + &data.samplers[sm_range], |index, sampler| { pre.issue(soft::ComputeCommand::BindSampler { index, sampler }); }, @@ -2863,7 +2863,7 @@ impl com::RawCommandBuffer for CommandBuffer { debug_assert_eq!(texture_base, tx_range.end); resources.set_textures( res_override.texture_id as usize, - &pool.textures[tx_range], + &data.textures[tx_range], |index, texture| { pre.issue(soft::ComputeCommand::BindTexture { index, texture }); }, @@ -2871,7 +2871,7 @@ impl com::RawCommandBuffer for CommandBuffer { } if buffer_base != bf_range.start { debug_assert_eq!(buffer_base, bf_range.end); - let buffers = &pool.buffers[bf_range]; + let buffers = &data.buffers[bf_range]; let start = res_override.buffer_id as usize; let mut dynamic_index = 0; for (i, bref) in buffers.iter().enumerate() { diff --git a/src/backend/metal/src/device.rs b/src/backend/metal/src/device.rs index 3b2fd5a83f8..438154b1f31 100644 --- a/src/backend/metal/src/device.rs +++ b/src/backend/metal/src/device.rs @@ -9,7 +9,7 @@ use std::borrow::Borrow; use std::collections::hash_map::Entry; use std::ops::Range; use std::path::Path; -use std::sync::{Arc, Condvar, Mutex, RwLock}; +use std::sync::{Arc, Condvar, Mutex}; use std::{cmp, mem, slice, time}; use hal::{self, error, image, pass, format, mapping, memory, buffer, pso, query, window}; @@ -1317,8 +1317,7 @@ impl hal::Device for Device { n::DescriptorPool::count_bindings(desc.ty, desc.count, &mut num_samplers, &mut num_textures, &mut num_buffers); } - let inner = n::DescriptorPoolInner::new(num_samplers, num_textures, num_buffers); - n::DescriptorPool::Emulated(Arc::new(RwLock::new(inner))) + n::DescriptorPool::new_emulated(num_samplers, num_textures, num_buffers) } } diff --git a/src/backend/metal/src/native.rs b/src/backend/metal/src/native.rs index 7796bd5cc25..d1066825935 100644 --- a/src/backend/metal/src/native.rs +++ b/src/backend/metal/src/native.rs @@ -224,11 +224,16 @@ unsafe impl Sync for Buffer {} #[derive(Debug)] pub enum DescriptorPool { - Emulated(Arc>), + Emulated { + inner: Arc>, + sampler_alloc: RangeAllocator, + texture_alloc: RangeAllocator, + buffer_alloc: RangeAllocator, + }, ArgumentBuffer { raw: metal::Buffer, range_allocator: RangeAllocator, - } + }, } //TODO: re-evaluate Send/Sync here unsafe impl Send for DescriptorPool {} @@ -243,35 +248,38 @@ pub struct BufferBinding { #[derive(Debug)] pub struct DescriptorPoolInner { pub samplers: Vec>, - sampler_alloc: RangeAllocator, pub textures: Vec>, - texture_alloc: RangeAllocator, pub buffers: Vec, - buffer_alloc: RangeAllocator, } -impl DescriptorPoolInner { - pub fn new(num_samplers: usize, num_textures: usize, num_buffers: usize) -> Self { - DescriptorPoolInner { +impl DescriptorPool { + pub(crate) fn new_emulated(num_samplers: usize, num_textures: usize, num_buffers: usize) -> Self { + let inner = DescriptorPoolInner { samplers: vec![None; num_samplers], - sampler_alloc: RangeAllocator::new(0 .. num_samplers as pso::DescriptorBinding), textures: vec![None; num_textures], - texture_alloc: RangeAllocator::new(0 .. num_textures as pso::DescriptorBinding), buffers: vec![BufferBinding { base: None, dynamic: false }; num_buffers], + }; + DescriptorPool::Emulated { + inner: Arc::new(RwLock::new(inner)), + sampler_alloc: RangeAllocator::new(0 .. num_samplers as pso::DescriptorBinding), + texture_alloc: RangeAllocator::new(0 .. num_textures as pso::DescriptorBinding), buffer_alloc: RangeAllocator::new(0 .. num_buffers as pso::DescriptorBinding), } } fn report_available(&self) { - trace!("\tavailable {} samplers, {} textures, and {} buffers", - self.sampler_alloc.total_available(), - self.texture_alloc.total_available(), - self.buffer_alloc.total_available(), - ); + match *self { + DescriptorPool::Emulated { ref sampler_alloc, ref texture_alloc, ref buffer_alloc, .. } => { + trace!("\tavailable {} samplers, {} textures, and {} buffers", + sampler_alloc.total_available(), + texture_alloc.total_available(), + buffer_alloc.total_available(), + ); + } + DescriptorPool::ArgumentBuffer { .. } => {} + } } -} -impl DescriptorPool { pub(crate) fn count_bindings( desc_type: pso::DescriptorType, desc_count: usize, @@ -306,8 +314,9 @@ impl DescriptorPool { impl hal::DescriptorPool for DescriptorPool { fn allocate_set(&mut self, set_layout: &DescriptorSetLayout) -> Result { + self.report_available(); match *self { - DescriptorPool::Emulated(ref pool_inner) => { + DescriptorPool::Emulated { ref inner, ref mut sampler_alloc, ref mut texture_alloc, ref mut buffer_alloc } => { debug!("pool: allocate_set"); let (layout_bindings, immutable_samplers) = match set_layout { &DescriptorSetLayout::Emulated(ref bindings, ref samplers) => (bindings, samplers), @@ -318,7 +327,9 @@ impl hal::DescriptorPool for DescriptorPool { let mut total_samplers = 0; let mut total_textures = 0; let mut total_buffers = 0; + let mut has_immutable_samplers = false; for layout in layout_bindings.iter() { + has_immutable_samplers |= layout.immutable_samplers; Self::count_bindings(layout.ty, layout.count, &mut total_samplers, &mut total_textures, &mut total_buffers); } @@ -326,11 +337,8 @@ impl hal::DescriptorPool for DescriptorPool { total_samplers, total_textures, total_buffers); // step[2]: try to allocate the ranges from the pool - let mut inner = pool_inner.write().unwrap(); - inner.report_available(); - let sampler_range = if total_samplers != 0 { - match inner.sampler_alloc.allocate_range(total_samplers as _) { + match sampler_alloc.allocate_range(total_samplers as _) { Ok(range) => range, Err(e) => { return Err(if e.fragmented_free_length >= total_samplers as u32 { @@ -344,11 +352,11 @@ impl hal::DescriptorPool for DescriptorPool { 0 .. 0 }; let texture_range = if total_textures != 0 { - match inner.texture_alloc.allocate_range(total_textures as _) { + match texture_alloc.allocate_range(total_textures as _) { Ok(range) => range, Err(e) => { if sampler_range.end != 0 { - inner.sampler_alloc.free_range(sampler_range); + sampler_alloc.free_range(sampler_range); } return Err(if e.fragmented_free_length >= total_samplers as u32 { pso::AllocationError::FragmentedPool @@ -361,14 +369,14 @@ impl hal::DescriptorPool for DescriptorPool { 0 .. 0 }; let buffer_range = if total_buffers != 0 { - match inner.buffer_alloc.allocate_range(total_buffers as _) { + match buffer_alloc.allocate_range(total_buffers as _) { Ok(range) => range, Err(e) => { if sampler_range.end != 0 { - inner.sampler_alloc.free_range(sampler_range); + sampler_alloc.free_range(sampler_range); } if texture_range.end != 0 { - inner.texture_alloc.free_range(texture_range); + texture_alloc.free_range(texture_range); } return Err(if e.fragmented_free_length >= total_samplers as u32 { pso::AllocationError::FragmentedPool @@ -382,27 +390,32 @@ impl hal::DescriptorPool for DescriptorPool { }; // step[3]: fill out immutable samplers - let mut immutable_sampler_offset = 0; - let mut sampler_offset = sampler_range.start as usize; - for layout in layout_bindings.iter() { - if layout.immutable_samplers { - for (sampler, immutable) in inner - .samplers[sampler_offset .. sampler_offset + layout.count] - .iter_mut() - .zip(&immutable_samplers[immutable_sampler_offset..]) - { - *sampler = Some(SamplerPtr(immutable.as_ptr())) + if has_immutable_samplers { + let mut data = inner.write().unwrap(); + let mut immutable_sampler_offset = 0; + let mut sampler_offset = sampler_range.start as usize; + let (mut tx_temp, mut bf_temp) = (0, 0); + + for layout in layout_bindings.iter() { + if layout.immutable_samplers { + for (sampler, immutable) in data + .samplers[sampler_offset .. sampler_offset + layout.count] + .iter_mut() + .zip(&immutable_samplers[immutable_sampler_offset..]) + { + *sampler = Some(SamplerPtr(immutable.as_ptr())) + } + immutable_sampler_offset += layout.count; } - immutable_sampler_offset += layout.count; + Self::count_bindings(layout.ty, layout.count, &mut sampler_offset, &mut tx_temp, &mut bf_temp); } - let (mut tx_temp, mut bf_temp) = (0, 0); - Self::count_bindings(layout.ty, layout.count, &mut sampler_offset, &mut tx_temp, &mut bf_temp); + + assert_eq!(immutable_sampler_offset, immutable_samplers.len()); + debug!("\tassigning {} immutable_samplers", immutable_samplers.len()); } - assert_eq!(immutable_sampler_offset, immutable_samplers.len()); - debug!("\tassigning {} immutable_samplers", immutable_samplers.len()); Ok(DescriptorSet::Emulated { - pool: Arc::clone(pool_inner), + pool: Arc::clone(inner), layouts: Arc::clone(layout_bindings), sampler_range, texture_range, @@ -432,33 +445,32 @@ impl hal::DescriptorPool for DescriptorPool { I: IntoIterator { match self { - DescriptorPool::Emulated(pool_inner) => { + DescriptorPool::Emulated { ref inner, ref mut sampler_alloc, ref mut texture_alloc, ref mut buffer_alloc } => { debug!("pool: free_sets"); - let mut inner = pool_inner.write().unwrap(); + let mut data = inner.write().unwrap(); for descriptor_set in descriptor_sets { match descriptor_set { DescriptorSet::Emulated { sampler_range, texture_range, buffer_range, .. } => { debug!("\t{:?} samplers, {:?} textures, and {:?} buffers", sampler_range, texture_range, buffer_range); - for sampler in &mut inner.samplers[sampler_range.start as usize .. sampler_range.end as usize] { + for sampler in &mut data.samplers[sampler_range.start as usize .. sampler_range.end as usize] { *sampler = None; } if sampler_range.start != sampler_range.end { - inner.sampler_alloc.free_range(sampler_range); + sampler_alloc.free_range(sampler_range); } - for image in &mut inner.textures[texture_range.start as usize .. texture_range.end as usize] { + for image in &mut data.textures[texture_range.start as usize .. texture_range.end as usize] { *image = None; } if texture_range.start != texture_range.end { - inner.texture_alloc.free_range(texture_range); + texture_alloc.free_range(texture_range); } - for buffer in &mut inner.buffers[buffer_range.start as usize .. buffer_range.end as usize] { + for buffer in &mut data.buffers[buffer_range.start as usize .. buffer_range.end as usize] { buffer.base = None; } if buffer_range.start != buffer_range.end { - inner.buffer_alloc.free_range(buffer_range); + buffer_alloc.free_range(buffer_range); } - inner.report_available(); } DescriptorSet::ArgumentBuffer{..} => { panic!("Tried to free a DescriptorSet not given out by this DescriptorPool!") @@ -480,25 +492,26 @@ impl hal::DescriptorPool for DescriptorPool { } } } + self.report_available(); } fn reset(&mut self) { match *self { - DescriptorPool::Emulated(ref pool_inner) => { + DescriptorPool::Emulated { ref inner, ref mut sampler_alloc, ref mut texture_alloc, ref mut buffer_alloc } => { debug!("pool: reset"); - let mut inner = pool_inner.write().unwrap(); + let mut data = inner.write().unwrap(); - inner.sampler_alloc.reset(); - inner.texture_alloc.reset(); - inner.buffer_alloc.reset(); + sampler_alloc.reset(); + texture_alloc.reset(); + buffer_alloc.reset(); - for sampler in &mut inner.samplers { + for sampler in &mut data.samplers { *sampler = None; } - for texture in &mut inner.textures { + for texture in &mut data.textures { *texture = None; } - for buffer in &mut inner.buffers { + for buffer in &mut data.buffers { buffer.base = None; } }