Skip to content

Commit

Permalink
Merge #2235
Browse files Browse the repository at this point in the history 
2235: Metal pools improved r=grovesNL a=kvark

Addresses part of  #2229 by ~~removing the descriptor pool lock entirely - it's meant to be externally synchronized by Vulkan anyway~~ moving the ranges out of the descriptor lock.
Fixes the correctness of a pool reset that's not created with the individual reset flag.

PR checklist:
- [ ] `make` succeeds (on *nix)
- [x] `make reftests` succeeds
- [x] tested examples with the following backends: metal
- [ ] `rustfmt` run on changed code


Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
  • Loading branch information
@bors @kvark
bors[bot] and kvark committed Jul 16, 2018
2 parents 45a72f6 + 674de26 commit 3129d5c
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 109 deletions.
53 changes: 21 additions & 32 deletions src/backend/metal/src/command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ type CommandBufferInnerPtr = Arc<RefCell<CommandBufferInner>>;

pub struct CommandPool {
pub(crate) shared: Arc<Shared>,
pub(crate) managed: Option<Vec<CommandBufferInnerPtr>>,
pub(crate) allocated: Vec<CommandBufferInnerPtr>,
}

unsafe impl Send for CommandPool {}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1446,12 +1443,10 @@ impl RawCommandQueue<Backend> for CommandQueue {

impl pool::RawCommandPool<Backend> 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);
}
}

Expand Down Expand Up @@ -1506,9 +1501,7 @@ impl pool::RawCommandPool<Backend> 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
}

Expand All @@ -1518,14 +1511,10 @@ impl pool::RawCommandPool<Backend> 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!")
Expand Down Expand Up @@ -2659,7 +2648,7 @@ impl com::RawCommandBuffer<Backend> 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;
Expand All @@ -2673,7 +2662,7 @@ impl com::RawCommandBuffer<Backend> 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()
Expand Down Expand Up @@ -2711,7 +2700,7 @@ impl com::RawCommandBuffer<Backend> 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 });
},
Expand All @@ -2721,15 +2710,15 @@ impl com::RawCommandBuffer<Backend> 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 });
},
);
}
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() {
Expand Down Expand Up @@ -2835,7 +2824,7 @@ impl com::RawCommandBuffer<Backend> 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;
Expand All @@ -2849,7 +2838,7 @@ impl com::RawCommandBuffer<Backend> 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()
Expand All @@ -2864,7 +2853,7 @@ impl com::RawCommandBuffer<Backend> 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 });
},
Expand All @@ -2874,15 +2863,15 @@ impl com::RawCommandBuffer<Backend> 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 });
},
);
}
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() {
Expand Down
24 changes: 7 additions & 17 deletions src/backend/metal/src/device.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -586,26 +586,17 @@ impl Device {

impl hal::Device<Backend> 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>(
Expand Down Expand Up @@ -1329,8 +1320,7 @@ impl hal::Device<Backend> 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)
}
}

Expand Down
Loading

0 comments on commit 3129d5c

Please sign in to comment.