From 3cc4fa51bc9f9d4209071ff59b1643e7c3f6a533 Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Tue, 14 Apr 2020 00:56:50 -0400 Subject: [PATCH] Track pipeline layout lifetime --- wgpu-core/src/binding_model.rs | 1 + wgpu-core/src/command/compute.rs | 6 +-- wgpu-core/src/command/render.rs | 6 +-- wgpu-core/src/device/life.rs | 26 +++++++++++ wgpu-core/src/device/mod.rs | 80 +++++++++++++++++++++----------- wgpu-core/src/pipeline.rs | 4 +- 6 files changed, 88 insertions(+), 35 deletions(-) diff --git a/wgpu-core/src/binding_model.rs b/wgpu-core/src/binding_model.rs index ba2f8d7f69..e8c20477e3 100644 --- a/wgpu-core/src/binding_model.rs +++ b/wgpu-core/src/binding_model.rs @@ -80,6 +80,7 @@ pub struct PipelineLayoutDescriptor { pub struct PipelineLayout { pub(crate) raw: B::PipelineLayout, pub(crate) device_id: Stored, + pub(crate) life_guard: LifeGuard, pub(crate) bind_group_layout_ids: ArrayVec<[BindGroupLayoutId; wgt::MAX_BIND_GROUPS]>, } diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 5e57fe3d64..1a10985682 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -176,9 +176,9 @@ impl Global { } // Rebind resources - if binder.pipeline_layout_id != Some(pipeline.layout_id) { - let pipeline_layout = &pipeline_layout_guard[pipeline.layout_id]; - binder.pipeline_layout_id = Some(pipeline.layout_id); + if binder.pipeline_layout_id != Some(pipeline.layout_id.value) { + let pipeline_layout = &pipeline_layout_guard[pipeline.layout_id.value]; + binder.pipeline_layout_id = Some(pipeline.layout_id.value); binder.reset_expectations(pipeline_layout.bind_group_layout_ids.len()); let mut is_compatible = true; diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 421e351636..a0e3694df9 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -896,9 +896,9 @@ impl Global { } // Rebind resource - if state.binder.pipeline_layout_id != Some(pipeline.layout_id) { - let pipeline_layout = &pipeline_layout_guard[pipeline.layout_id]; - state.binder.pipeline_layout_id = Some(pipeline.layout_id); + if state.binder.pipeline_layout_id != Some(pipeline.layout_id.value) { + let pipeline_layout = &pipeline_layout_guard[pipeline.layout_id.value]; + state.binder.pipeline_layout_id = Some(pipeline.layout_id.value); state .binder .reset_expectations(pipeline_layout.bind_group_layout_ids.len()); diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index e24898d84d..765193adba 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -29,6 +29,7 @@ pub struct SuspectedResources { pub(crate) bind_groups: Vec, pub(crate) compute_pipelines: Vec, pub(crate) render_pipelines: Vec, + pub(crate) pipeline_layouts: Vec>, } impl SuspectedResources { @@ -40,6 +41,7 @@ impl SuspectedResources { self.bind_groups.clear(); self.compute_pipelines.clear(); self.render_pipelines.clear(); + self.pipeline_layouts.clear(); } pub fn extend(&mut self, other: &Self) { @@ -52,6 +54,8 @@ impl SuspectedResources { .extend_from_slice(&other.compute_pipelines); self.render_pipelines .extend_from_slice(&other.render_pipelines); + self.pipeline_layouts + .extend_from_slice(&other.pipeline_layouts); } } @@ -68,6 +72,7 @@ struct NonReferencedResources { desc_sets: Vec>, compute_pipes: Vec, graphics_pipes: Vec, + pipeline_layouts: Vec, } impl NonReferencedResources { @@ -81,6 +86,7 @@ impl NonReferencedResources { desc_sets: Vec::new(), compute_pipes: Vec::new(), graphics_pipes: Vec::new(), + pipeline_layouts: Vec::new(), } } @@ -139,6 +145,9 @@ impl NonReferencedResources { for raw in self.graphics_pipes.drain(..) { device.destroy_graphics_pipeline(raw); } + for raw in self.pipeline_layouts.drain(..) { + device.destroy_pipeline_layout(raw); + } } } @@ -445,6 +454,23 @@ impl LifetimeTracker { } } } + + if !self.suspected_resources.pipeline_layouts.is_empty() { + let (mut guard, _) = hub.pipeline_layouts.write(token); + + for Stored { + value: id, + ref_count, + } in self.suspected_resources.pipeline_layouts.drain(..) + { + //Note: this has to happen after all the suspected pipelines are destroyed + if ref_count.load() == 1 { + hub.pipeline_layouts.free_id(id); + let layout = guard.remove(id).unwrap(); + self.free_resources.pipeline_layouts.push(layout.raw); + } + } + } } pub(crate) fn triage_mapped( diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index 536aaf2022..925c926c8a 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -1005,6 +1005,7 @@ impl Global { value: device_id, ref_count: device.life_guard.add_ref(), }, + life_guard: LifeGuard::new(), bind_group_layout_ids: bind_group_layout_ids.iter().cloned().collect(), }; hub.pipeline_layouts @@ -1014,15 +1015,24 @@ impl Global { pub fn pipeline_layout_destroy(&self, pipeline_layout_id: id::PipelineLayoutId) { let hub = B::hub(self); let mut token = Token::root(); + let (device_id, ref_count) = { + let (mut pipeline_layout_guard, _) = hub.pipeline_layouts.write(&mut token); + let layout = &mut pipeline_layout_guard[pipeline_layout_id]; + ( + layout.device_id.value, + layout.life_guard.ref_count.take().unwrap(), + ) + }; + let (device_guard, mut token) = hub.devices.read(&mut token); - let (pipeline_layout, _) = hub + device_guard[device_id] + .lock_life(&mut token) + .suspected_resources .pipeline_layouts - .unregister(pipeline_layout_id, &mut token); - unsafe { - device_guard[pipeline_layout.device_id.value] - .raw - .destroy_pipeline_layout(pipeline_layout.raw); - } + .push(Stored { + value: pipeline_layout_id, + ref_count, + }); } pub fn device_create_bind_group( @@ -1667,9 +1677,9 @@ impl Global { let (device_guard, mut token) = hub.devices.read(&mut token); let device = &device_guard[device_id]; - let raw_pipeline = { + let (raw_pipeline, layout_ref_count) = { let (pipeline_layout_guard, mut token) = hub.pipeline_layouts.read(&mut token); - let layout = &pipeline_layout_guard[desc.layout].raw; + let layout = &pipeline_layout_guard[desc.layout]; let (shader_module_guard, _) = hub.shader_modules.read(&mut token); let rp_key = RenderPassKey { @@ -1777,19 +1787,20 @@ impl Global { depth_stencil, multisampling, baked_states, - layout, + layout: &layout.raw, subpass, flags, parent, }; // TODO: cache - unsafe { + let pipeline = unsafe { device .raw .create_graphics_pipeline(&pipeline_desc, None) .unwrap() - } + }; + (pipeline, layout.life_guard.add_ref()) }; let pass_context = RenderPassContext { @@ -1812,7 +1823,10 @@ impl Global { let pipeline = pipeline::RenderPipeline { raw: raw_pipeline, - layout_id: desc.layout, + layout_id: Stored { + value: desc.layout, + ref_count: layout_ref_count, + }, device_id: Stored { value: device_id, ref_count: device.life_guard.add_ref(), @@ -1834,18 +1848,22 @@ impl Global { let mut token = Token::root(); let (device_guard, mut token) = hub.devices.read(&mut token); - let device_id = { + let (device_id, layout_id) = { let (mut pipeline_guard, _) = hub.render_pipelines.write(&mut token); let pipeline = &mut pipeline_guard[render_pipeline_id]; pipeline.life_guard.ref_count.take(); - pipeline.device_id.value + (pipeline.device_id.value, pipeline.layout_id.clone()) }; - device_guard[device_id] - .lock_life(&mut token) + let mut life_lock = device_guard[device_id].lock_life(&mut token); + life_lock .suspected_resources .render_pipelines .push(render_pipeline_id); + life_lock + .suspected_resources + .pipeline_layouts + .push(layout_id); } pub fn device_create_compute_pipeline( @@ -1859,9 +1877,9 @@ impl Global { let (device_guard, mut token) = hub.devices.read(&mut token); let device = &device_guard[device_id]; - let raw_pipeline = { + let (raw_pipeline, layout_ref_count) = { let (pipeline_layout_guard, mut token) = hub.pipeline_layouts.read(&mut token); - let layout = &pipeline_layout_guard[desc.layout].raw; + let layout = &pipeline_layout_guard[desc.layout]; let pipeline_stage = &desc.compute_stage; let (shader_module_guard, _) = hub.shader_modules.read(&mut token); @@ -1881,22 +1899,26 @@ impl Global { let pipeline_desc = hal::pso::ComputePipelineDesc { shader, - layout, + layout: &layout.raw, flags, parent, }; - unsafe { + let pipeline = unsafe { device .raw .create_compute_pipeline(&pipeline_desc, None) .unwrap() - } + }; + (pipeline, layout.life_guard.add_ref()) }; let pipeline = pipeline::ComputePipeline { raw: raw_pipeline, - layout_id: desc.layout, + layout_id: Stored { + value: desc.layout, + ref_count: layout_ref_count, + }, device_id: Stored { value: device_id, ref_count: device.life_guard.add_ref(), @@ -1915,18 +1937,22 @@ impl Global { let mut token = Token::root(); let (device_guard, mut token) = hub.devices.read(&mut token); - let device_id = { + let (device_id, layout_id) = { let (mut pipeline_guard, _) = hub.compute_pipelines.write(&mut token); let pipeline = &mut pipeline_guard[compute_pipeline_id]; pipeline.life_guard.ref_count.take(); - pipeline.device_id.value + (pipeline.device_id.value, pipeline.layout_id.clone()) }; - device_guard[device_id] - .lock_life(&mut token) + let mut life_lock = device_guard[device_id].lock_life(&mut token); + life_lock .suspected_resources .compute_pipelines .push(compute_pipeline_id); + life_lock + .suspected_resources + .pipeline_layouts + .push(layout_id); } pub fn device_create_swap_chain( diff --git a/wgpu-core/src/pipeline.rs b/wgpu-core/src/pipeline.rs index d76d0b0443..95340529c8 100644 --- a/wgpu-core/src/pipeline.rs +++ b/wgpu-core/src/pipeline.rs @@ -59,7 +59,7 @@ pub struct ComputePipelineDescriptor { #[derive(Debug)] pub struct ComputePipeline { pub(crate) raw: B::ComputePipeline, - pub(crate) layout_id: PipelineLayoutId, + pub(crate) layout_id: Stored, pub(crate) device_id: Stored, pub(crate) life_guard: LifeGuard, } @@ -98,7 +98,7 @@ bitflags::bitflags! { #[derive(Debug)] pub struct RenderPipeline { pub(crate) raw: B::GraphicsPipeline, - pub(crate) layout_id: PipelineLayoutId, + pub(crate) layout_id: Stored, pub(crate) device_id: Stored, pub(crate) pass_context: RenderPassContext, pub(crate) flags: PipelineFlags,