Skip to content

Commit

Permalink
Merge #582
Browse files Browse the repository at this point in the history
582: Track pipeline layout lifetime r=grovesNL a=kvark

Fixes #580
This one is interesting: it's not instantly destroyed when dropped, like bind group layouts. And it's not tracked for GPU usage like the resources. It's somewhat in-between. We have the following classes of tracking now:
  1. no tracking, destroyed on drop. Applies to: bind group layouts, adapters
  2. only CPU-side tracking. They go to "suspected" list on drop of self or anything that can point to them. Applies to: pipeline layouts
  3. full GPU tracking. They go to "suspected" list on drop, then get associated with active submission before destruction. Applies to: buffers, textures, views, bind groups, pipelines

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
  • Loading branch information
bors[bot] and kvark authored Apr 14, 2020
2 parents 96a4a34 + 3cc4fa5 commit 729ecb1
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 35 deletions.
1 change: 1 addition & 0 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub struct PipelineLayoutDescriptor {
pub struct PipelineLayout<B: hal::Backend> {
pub(crate) raw: B::PipelineLayout,
pub(crate) device_id: Stored<DeviceId>,
pub(crate) life_guard: LifeGuard,
pub(crate) bind_group_layout_ids: ArrayVec<[BindGroupLayoutId; wgt::MAX_BIND_GROUPS]>,
}

Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

// 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;

Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -896,9 +896,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

// 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());
Expand Down
26 changes: 26 additions & 0 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub struct SuspectedResources {
pub(crate) bind_groups: Vec<id::BindGroupId>,
pub(crate) compute_pipelines: Vec<id::ComputePipelineId>,
pub(crate) render_pipelines: Vec<id::RenderPipelineId>,
pub(crate) pipeline_layouts: Vec<Stored<id::PipelineLayoutId>>,
}

impl SuspectedResources {
Expand All @@ -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) {
Expand All @@ -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);
}
}

Expand All @@ -68,6 +72,7 @@ struct NonReferencedResources<B: hal::Backend> {
desc_sets: Vec<DescriptorSet<B>>,
compute_pipes: Vec<B::ComputePipeline>,
graphics_pipes: Vec<B::GraphicsPipeline>,
pipeline_layouts: Vec<B::PipelineLayout>,
}

impl<B: hal::Backend> NonReferencedResources<B> {
Expand All @@ -81,6 +86,7 @@ impl<B: hal::Backend> NonReferencedResources<B> {
desc_sets: Vec::new(),
compute_pipes: Vec::new(),
graphics_pipes: Vec::new(),
pipeline_layouts: Vec::new(),
}
}

Expand Down Expand Up @@ -139,6 +145,9 @@ impl<B: hal::Backend> NonReferencedResources<B> {
for raw in self.graphics_pipes.drain(..) {
device.destroy_graphics_pipeline(raw);
}
for raw in self.pipeline_layouts.drain(..) {
device.destroy_pipeline_layout(raw);
}
}
}

Expand Down Expand Up @@ -445,6 +454,23 @@ impl<B: GfxBackend> LifetimeTracker<B> {
}
}
}

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<G: GlobalIdentityHandlerFactory>(
Expand Down
80 changes: 53 additions & 27 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
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
Expand All @@ -1006,15 +1007,24 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
pub fn pipeline_layout_destroy<B: GfxBackend>(&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<B: GfxBackend>(
Expand Down Expand Up @@ -1659,9 +1669,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

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 {
Expand Down Expand Up @@ -1769,19 +1779,20 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
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 {
Expand All @@ -1804,7 +1815,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

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(),
Expand All @@ -1826,18 +1840,22 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
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<B: GfxBackend>(
Expand All @@ -1851,9 +1869,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

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);

Expand All @@ -1873,22 +1891,26 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

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(),
Expand All @@ -1907,18 +1929,22 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
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<B: GfxBackend>(
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub struct ComputePipelineDescriptor {
#[derive(Debug)]
pub struct ComputePipeline<B: hal::Backend> {
pub(crate) raw: B::ComputePipeline,
pub(crate) layout_id: PipelineLayoutId,
pub(crate) layout_id: Stored<PipelineLayoutId>,
pub(crate) device_id: Stored<DeviceId>,
pub(crate) life_guard: LifeGuard,
}
Expand Down Expand Up @@ -98,7 +98,7 @@ bitflags::bitflags! {
#[derive(Debug)]
pub struct RenderPipeline<B: hal::Backend> {
pub(crate) raw: B::GraphicsPipeline,
pub(crate) layout_id: PipelineLayoutId,
pub(crate) layout_id: Stored<PipelineLayoutId>,
pub(crate) device_id: Stored<DeviceId>,
pub(crate) pass_context: RenderPassContext,
pub(crate) flags: PipelineFlags,
Expand Down

0 comments on commit 729ecb1

Please sign in to comment.