From 45a618f8782e1a0aba7c7b168ce6035c0e4bb2c0 Mon Sep 17 00:00:00 2001 From: gents83 Date: Sat, 16 Sep 2023 20:00:21 +0200 Subject: [PATCH] Fix merge conflicts and improve id mngmnt --- player/src/lib.rs | 11 +- tests/tests/device.rs | 2 +- tests/tests/mem_leaks.rs | 13 +- wgpu-core/src/device/global.rs | 220 +++++++++++++++-------------- wgpu-core/src/device/life.rs | 233 ++++++++++--------------------- wgpu-core/src/device/queue.rs | 91 ++++++------ wgpu-core/src/device/resource.rs | 23 ++- wgpu-core/src/identity.rs | 84 +++++------ wgpu-core/src/registry.rs | 38 +++-- wgpu-core/src/resource.rs | 4 +- 10 files changed, 335 insertions(+), 384 deletions(-) diff --git a/player/src/lib.rs b/player/src/lib.rs index 063c614bd1..ec8b91371a 100644 --- a/player/src/lib.rs +++ b/player/src/lib.rs @@ -8,21 +8,22 @@ #![cfg(not(target_arch = "wasm32"))] #![warn(unsafe_op_in_unsafe_fn)] -use wgc::{device::trace, identity::IdentityManager}; +use wgc::device::trace; -use std::{borrow::Cow, fs, path::Path, sync::Arc}; +use std::{borrow::Cow, fs, path::Path}; pub struct IdentityPassThroughFactory; impl wgc::identity::IdentityHandlerFactory for IdentityPassThroughFactory { type Input = I; - fn spawn(&self) -> Option>> { - None - } fn input_to_id(id_in: Self::Input) -> I { id_in } + + fn autogenerate_ids() -> bool { + false + } } impl wgc::identity::GlobalIdentityHandlerFactory for IdentityPassThroughFactory {} diff --git a/tests/tests/device.rs b/tests/tests/device.rs index 28ba9465e8..1b8c4d765c 100644 --- a/tests/tests/device.rs +++ b/tests/tests/device.rs @@ -104,7 +104,7 @@ fn request_device_error_on_native() { async fn request_device_error_message() { // Not using initialize_test() because that doesn't let us catch the error // nor .await anything - let (adapter, _surface_guard) = wgpu_test::initialize_adapter(); + let (_instance, adapter, _surface_guard) = wgpu_test::initialize_adapter(); let device_error = adapter .request_device( diff --git a/tests/tests/mem_leaks.rs b/tests/tests/mem_leaks.rs index 7cf167288a..dd1b93e7af 100644 --- a/tests/tests/mem_leaks.rs +++ b/tests/tests/mem_leaks.rs @@ -156,7 +156,8 @@ fn draw_test_with_reports( let global_report = ctx.instance.generate_report(); let report = global_report.hub_report(); assert_eq!(report.buffers.num_allocated, 1); - assert_eq!(report.texture_views.num_allocated, 1); + //1 is clear_view and 1 is user's texture_view + assert_eq!(report.texture_views.num_allocated, 2); assert_eq!(report.textures.num_allocated, 1); drop(texture); @@ -164,7 +165,7 @@ fn draw_test_with_reports( let global_report = ctx.instance.generate_report(); let report = global_report.hub_report(); assert_eq!(report.buffers.num_allocated, 1); - assert_eq!(report.texture_views.num_allocated, 1); + assert_eq!(report.texture_views.num_allocated, 2); assert_eq!(report.texture_views.num_kept_from_user, 1); assert_eq!(report.textures.num_allocated, 1); assert_eq!(report.textures.num_kept_from_user, 0); @@ -203,7 +204,7 @@ fn draw_test_with_reports( assert_eq!(report.compute_pipelines.num_allocated, 0); assert_eq!(report.command_buffers.num_allocated, 1); assert_eq!(report.render_bundles.num_allocated, 0); - assert_eq!(report.texture_views.num_allocated, 1); + assert_eq!(report.texture_views.num_allocated, 2); assert_eq!(report.textures.num_allocated, 1); function(&mut rpass); @@ -228,7 +229,7 @@ fn draw_test_with_reports( assert_eq!(report.bind_group_layouts.num_kept_from_user, 0); assert_eq!(report.bind_groups.num_allocated, 1); assert_eq!(report.bind_groups.num_kept_from_user, 0); - assert_eq!(report.texture_views.num_allocated, 1); + assert_eq!(report.texture_views.num_allocated, 2); assert_eq!(report.texture_views.num_kept_from_user, 0); assert_eq!(report.buffers.num_allocated, 1); assert_eq!(report.buffers.num_kept_from_user, 0); @@ -250,8 +251,10 @@ fn draw_test_with_reports( assert_eq!(report.bind_groups.num_allocated, 0); assert_eq!(report.bind_group_layouts.num_allocated, 0); assert_eq!(report.pipeline_layouts.num_allocated, 0); - assert_eq!(report.texture_views.num_allocated, 0); assert_eq!(report.buffers.num_allocated, 0); + //surface is still there + assert_eq!(report.texture_views.num_allocated, 1); + assert_eq!(report.textures.num_allocated, 1); drop(ctx.queue); drop(ctx.device); diff --git a/wgpu-core/src/device/global.rs b/wgpu-core/src/device/global.rs index bc84865d40..39b351ae9b 100644 --- a/wgpu-core/src/device/global.rs +++ b/wgpu-core/src/device/global.rs @@ -22,7 +22,7 @@ use smallvec::SmallVec; use wgt::{BufferAddress, TextureFormat}; -use std::{borrow::Cow, iter, ops::Range, ptr, sync::Arc}; +use std::{borrow::Cow, iter, ops::Range, ptr}; use super::{ImplicitPipelineIds, InvalidDevice, UserClosures}; @@ -132,122 +132,133 @@ impl Global { let hub = A::hub(self); let fid = hub.buffers.prepare::(id_in); - let error = loop { - let device = match hub.devices.get(device_id) { - Ok(device) => device, - Err(_) => break DeviceError::Invalid.into(), - }; + let device = match hub.devices.get(device_id) { + Ok(device) => device, + Err(_) => { + let id = fid.assign_error(desc.label.borrow_or_default()); + return (id, Some(DeviceError::Invalid.into())); + } + }; - if desc.usage.is_empty() { - // Per spec, `usage` must not be zero. - break resource::CreateBufferError::InvalidUsage(desc.usage); + if desc.usage.is_empty() { + // Per spec, `usage` must not be zero. + let id = fid.assign_error(desc.label.borrow_or_default()); + return ( + id, + Some(resource::CreateBufferError::InvalidUsage(desc.usage)), + ); + } + + #[cfg(feature = "trace")] + if let Some(ref mut trace) = *device.trace.lock() { + let mut desc = desc.clone(); + let mapped_at_creation = std::mem::replace(&mut desc.mapped_at_creation, false); + if mapped_at_creation && !desc.usage.contains(wgt::BufferUsages::MAP_WRITE) { + desc.usage |= wgt::BufferUsages::COPY_DST; } + trace.add(trace::Action::CreateBuffer(fid.id(), desc)); + } - #[cfg(feature = "trace")] - if let Some(ref mut trace) = *device.trace.lock() { - let mut desc = desc.clone(); - let mapped_at_creation = std::mem::replace(&mut desc.mapped_at_creation, false); - if mapped_at_creation && !desc.usage.contains(wgt::BufferUsages::MAP_WRITE) { - desc.usage |= wgt::BufferUsages::COPY_DST; - } - trace.add(trace::Action::CreateBuffer(fid.id(), desc)); + let buffer = match device.create_buffer(device_id, desc, false) { + Ok(buffer) => buffer, + Err(e) => { + let id = fid.assign_error(desc.label.borrow_or_default()); + return (id, Some(e)); } + }; - let buffer = match device.create_buffer(device_id, desc, false) { - Ok(buffer) => buffer, - Err(e) => break e, - }; + let (id, resource) = fid.assign(buffer); + log::info!("Created Buffer {:?} with {:?}", id, desc); - let buffer_use = if !desc.mapped_at_creation { - hal::BufferUses::empty() - } else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) { - // buffer is mappable, so we are just doing that at start - 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) { - Ok(ptr) => ptr, - Err(e) => { - device.lock_life().schedule_resource_destruction( - queue::TempResource::Buffer(Arc::new(buffer)), - !0, - ); - break e.into(); - } - } - }; - *buffer.map_state.lock() = resource::BufferMapState::Active { - ptr, - range: 0..map_size, - host: HostMap::Write, - }; - hal::BufferUses::MAP_WRITE + let buffer_use = if !desc.mapped_at_creation { + hal::BufferUses::empty() + } else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) { + // buffer is mappable, so we are just doing that at start + let map_size = resource.size; + let ptr = if map_size == 0 { + std::ptr::NonNull::dangling() } else { - // buffer needs staging area for initialization only - let stage_desc = wgt::BufferDescriptor { - label: Some(Cow::Borrowed( - "(wgpu internal) initializing unmappable buffer", - )), - size: desc.size, - usage: wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::COPY_SRC, - mapped_at_creation: false, - }; - let stage = match device.create_buffer(device_id, &stage_desc, true) { - Ok(stage) => stage, + match map_buffer(device.raw(), &resource, 0, map_size, HostMap::Write) { + Ok(ptr) => ptr, Err(e) => { device.lock_life().schedule_resource_destruction( - queue::TempResource::Buffer(Arc::new(buffer)), + queue::TempResource::Buffer(resource), !0, ); - break e; + hub.buffers + .force_replace_with_error(id, desc.label.borrow_or_default()); + return (id, Some(e.into())); } - }; - let mapping = match unsafe { device.raw().map_buffer(stage.raw(), 0..stage.size) } { - Ok(mapping) => mapping, - Err(e) => { - let mut life_lock = device.lock_life(); - life_lock.schedule_resource_destruction( - queue::TempResource::Buffer(Arc::new(buffer)), - !0, - ); - life_lock.schedule_resource_destruction( - queue::TempResource::Buffer(Arc::new(stage)), - !0, - ); - break DeviceError::from(e).into(); - } - }; - - assert_eq!(buffer.size % wgt::COPY_BUFFER_ALIGNMENT, 0); - // Zero initialize memory and then mark both staging and buffer as initialized - // (it's guaranteed that this is the case by the time the buffer is usable) - unsafe { ptr::write_bytes(mapping.ptr.as_ptr(), 0, buffer.size as usize) }; - buffer.initialization_status.write().drain(0..buffer.size); - stage.initialization_status.write().drain(0..buffer.size); - - *buffer.map_state.lock() = resource::BufferMapState::Init { - ptr: mapping.ptr, - needs_flush: !mapping.is_coherent, - stage_buffer: Arc::new(stage), - }; - hal::BufferUses::COPY_DST + } }; + *resource.map_state.lock() = resource::BufferMapState::Active { + ptr, + range: 0..map_size, + host: HostMap::Write, + }; + hal::BufferUses::MAP_WRITE + } else { + // buffer needs staging area for initialization only + let stage_desc = wgt::BufferDescriptor { + label: Some(Cow::Borrowed( + "(wgpu internal) initializing unmappable buffer", + )), + size: desc.size, + usage: wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::COPY_SRC, + mapped_at_creation: false, + }; + let stage = match device.create_buffer(device_id, &stage_desc, true) { + Ok(stage) => stage, + Err(e) => { + device + .lock_life() + .schedule_resource_destruction(queue::TempResource::Buffer(resource), !0); + hub.buffers + .force_replace_with_error(id, desc.label.borrow_or_default()); + return (id, Some(e)); + } + }; + let stage_fid = hub.buffers.request::(); + let stage = stage_fid.init(stage); - let (id, resource) = fid.assign(buffer); - log::info!("Created Buffer {:?} with {:?}", id, desc); - - device - .trackers - .lock() - .buffers - .insert_single(id, resource, buffer_use); + let mapping = match unsafe { device.raw().map_buffer(stage.raw(), 0..stage.size) } { + Ok(mapping) => mapping, + Err(e) => { + let mut life_lock = device.lock_life(); + life_lock + .schedule_resource_destruction(queue::TempResource::Buffer(resource), !0); + life_lock.schedule_resource_destruction(queue::TempResource::Buffer(stage), !0); + hub.buffers + .force_replace_with_error(id, desc.label.borrow_or_default()); + return (id, Some(DeviceError::from(e).into())); + } + }; - return (id, None); + assert_eq!(resource.size % wgt::COPY_BUFFER_ALIGNMENT, 0); + // Zero initialize memory and then mark both staging and buffer as initialized + // (it's guaranteed that this is the case by the time the buffer is usable) + unsafe { ptr::write_bytes(mapping.ptr.as_ptr(), 0, resource.size as usize) }; + resource + .initialization_status + .write() + .drain(0..resource.size); + stage.initialization_status.write().drain(0..resource.size); + + *resource.map_state.lock() = resource::BufferMapState::Init { + ptr: mapping.ptr, + needs_flush: !mapping.is_coherent, + stage_buffer: stage, + }; + hal::BufferUses::COPY_DST }; - let id = fid.assign_error(desc.label.borrow_or_default()); - (id, Some(error)) + device + .trackers + .lock() + .buffers + .insert_single(id, resource, buffer_use); + + (id, None) } /// Assign `id_in` an error with the given `label`. @@ -455,7 +466,7 @@ impl Global { let hub = A::hub(self); if let Some(buffer) = hub.buffers.unregister(buffer_id) { - if buffer.is_unique() { + if buffer.ref_count() == 1 { buffer.destroy().ok(); } @@ -548,7 +559,9 @@ impl Global { }, }; let view = device.create_texture_view(&resource, &desc).unwrap(); - clear_views.push(Arc::new(view)); + let view_fid = hub.texture_views.request::(); + let view = view_fid.init(view); + clear_views.push(view); } } let mut clear_mode = resource.clear_mode.write(); @@ -798,6 +811,7 @@ impl Global { } } + #[allow(unused_unsafe)] pub fn texture_create_view( &self, texture_id: id::TextureId, @@ -825,7 +839,7 @@ impl Global { }); } - let view = match device.create_texture_view(&texture, desc) { + let view = match unsafe { device.create_texture_view(&texture, desc) } { Ok(view) => view, Err(e) => break e, }; diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 8559c60369..a2a2c68a56 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -11,7 +11,7 @@ use crate::{ hub::Hub, id::{self}, pipeline::{ComputePipeline, RenderPipeline}, - resource::{self, Buffer, QuerySet, Resource, Sampler, Texture, TextureView}, + resource::{self, Buffer, QuerySet, Resource, Sampler, StagingBuffer, Texture, TextureView}, track::Tracker, SubmissionIndex, }; @@ -23,8 +23,9 @@ use thiserror::Error; use std::{collections::HashMap, sync::Arc}; /// A struct that keeps lists of resources that are no longer needed by the user. -pub(crate) struct SuspectedResources { +pub(crate) struct ResourceMaps { pub(crate) buffers: HashMap>>, + pub(crate) staging_buffers: HashMap>>, pub(crate) textures: HashMap>>, pub(crate) texture_views: HashMap>>, pub(crate) samplers: HashMap>>, @@ -37,10 +38,11 @@ pub(crate) struct SuspectedResources { pub(crate) query_sets: HashMap>>, } -impl SuspectedResources { +impl ResourceMaps { pub(crate) fn new() -> Self { Self { buffers: HashMap::new(), + staging_buffers: HashMap::new(), textures: HashMap::new(), texture_views: HashMap::new(), samplers: HashMap::new(), @@ -55,6 +57,7 @@ impl SuspectedResources { } pub(crate) fn clear(&mut self) { self.buffers.clear(); + self.staging_buffers.clear(); self.textures.clear(); self.texture_views.clear(); self.samplers.clear(); @@ -67,128 +70,18 @@ impl SuspectedResources { self.query_sets.clear(); } - pub(crate) fn extend(&mut self, other: &Self) { - other.buffers.iter().for_each(|(id, v)| { - self.buffers.insert(*id, v.clone()); - }); - other.textures.iter().for_each(|(id, v)| { - self.textures.insert(*id, v.clone()); - }); - other.texture_views.iter().for_each(|(id, v)| { - self.texture_views.insert(*id, v.clone()); - }); - other.samplers.iter().for_each(|(id, v)| { - self.samplers.insert(*id, v.clone()); - }); - other.bind_groups.iter().for_each(|(id, v)| { - self.bind_groups.insert(*id, v.clone()); - }); - other.compute_pipelines.iter().for_each(|(id, v)| { - self.compute_pipelines.insert(*id, v.clone()); - }); - other.render_pipelines.iter().for_each(|(id, v)| { - self.render_pipelines.insert(*id, v.clone()); - }); - other.bind_group_layouts.iter().for_each(|(id, v)| { - self.bind_group_layouts.insert(*id, v.clone()); - }); - other.pipeline_layouts.iter().for_each(|(id, v)| { - self.pipeline_layouts.insert(*id, v.clone()); - }); - other.render_bundles.iter().for_each(|(id, v)| { - self.render_bundles.insert(*id, v.clone()); - }); - other.query_sets.iter().for_each(|(id, v)| { - self.query_sets.insert(*id, v.clone()); - }); - } -} - -/// Raw backend resources that should be freed shortly. -#[derive(Debug)] -struct NonReferencedResources { - buffers: Vec>>, - textures: Vec>>, - texture_views: Vec>>, - samplers: Vec>>, - bind_groups: Vec>>, - compute_pipes: Vec>>, - render_pipes: Vec>>, - bind_group_layouts: Vec>>, - pipeline_layouts: Vec>>, - query_sets: Vec>>, -} - -impl NonReferencedResources { - fn new() -> Self { - Self { - buffers: Vec::new(), - textures: Vec::new(), - texture_views: Vec::new(), - samplers: Vec::new(), - bind_groups: Vec::new(), - compute_pipes: Vec::new(), - render_pipes: Vec::new(), - bind_group_layouts: Vec::new(), - pipeline_layouts: Vec::new(), - query_sets: Vec::new(), - } - } - - fn extend(&mut self, other: Self) { + pub(crate) fn extend(&mut self, other: Self) { self.buffers.extend(other.buffers); + self.staging_buffers.extend(other.staging_buffers); self.textures.extend(other.textures); self.texture_views.extend(other.texture_views); self.samplers.extend(other.samplers); self.bind_groups.extend(other.bind_groups); - self.compute_pipes.extend(other.compute_pipes); - self.render_pipes.extend(other.render_pipes); + self.compute_pipelines.extend(other.compute_pipelines); + self.render_pipelines.extend(other.render_pipelines); + self.bind_group_layouts.extend(other.bind_group_layouts); + self.pipeline_layouts.extend(other.pipeline_layouts); self.query_sets.extend(other.query_sets); - assert!(other.bind_group_layouts.is_empty()); - assert!(other.pipeline_layouts.is_empty()); - } - - unsafe fn clean(&mut self) { - if !self.buffers.is_empty() { - profiling::scope!("destroy_buffers"); - self.buffers.clear(); - } - if !self.textures.is_empty() { - profiling::scope!("destroy_textures"); - self.textures.clear(); - } - if !self.texture_views.is_empty() { - profiling::scope!("destroy_texture_views"); - self.texture_views.clear(); - } - if !self.samplers.is_empty() { - profiling::scope!("destroy_samplers"); - self.samplers.clear(); - } - if !self.bind_groups.is_empty() { - profiling::scope!("destroy_bind_groups"); - self.bind_groups.clear(); - } - if !self.compute_pipes.is_empty() { - profiling::scope!("destroy_compute_pipelines"); - self.compute_pipes.clear(); - } - if !self.render_pipes.is_empty() { - profiling::scope!("destroy_render_pipelines"); - self.render_pipes.clear(); - } - if !self.bind_group_layouts.is_empty() { - profiling::scope!("destroy_bind_group_layouts"); - self.bind_group_layouts.clear(); - } - if !self.pipeline_layouts.is_empty() { - profiling::scope!("destroy_pipeline_layouts"); - self.pipeline_layouts.clear(); - } - if !self.query_sets.is_empty() { - profiling::scope!("destroy_query_sets"); - self.query_sets.clear(); - } } } @@ -210,7 +103,7 @@ struct ActiveSubmission { /// This includes things like temporary resources and resources that are /// used by submitted commands but have been dropped by the user (meaning that /// this submission is their last reference.) - last_resources: NonReferencedResources, + last_resources: ResourceMaps, /// Buffers to be mapped once this submission has completed. mapped: Vec>>, @@ -284,7 +177,7 @@ pub(crate) struct LifetimeTracker { /// Resources whose user handle has died (i.e. drop/destroy has been called) /// and will likely be ready for destruction soon. - pub suspected_resources: SuspectedResources, + pub suspected_resources: ResourceMaps, /// Resources used by queue submissions still in flight. One entry per /// submission, with older submissions appearing before younger. @@ -299,7 +192,7 @@ pub(crate) struct LifetimeTracker { /// These are freed by `LifeTracker::cleanup`, which is called from periodic /// maintenance functions like `Global::device_poll`, and when a device is /// destroyed. - free_resources: NonReferencedResources, + free_resources: ResourceMaps, /// Buffers the user has asked us to map, and which are not used by any /// queue submission still in flight. @@ -318,9 +211,9 @@ impl LifetimeTracker { mapped: Vec::new(), future_suspected_buffers: Vec::new(), future_suspected_textures: Vec::new(), - suspected_resources: SuspectedResources::new(), + suspected_resources: ResourceMaps::new(), active: Vec::new(), - free_resources: NonReferencedResources::new(), + free_resources: ResourceMaps::new(), ready_to_map: Vec::new(), work_done_closures: SmallVec::new(), } @@ -338,13 +231,22 @@ impl LifetimeTracker { temp_resources: impl Iterator>, encoders: Vec>, ) { - let mut last_resources = NonReferencedResources::new(); + let mut last_resources = ResourceMaps::new(); for res in temp_resources { match res { - TempResource::Buffer(raw) => last_resources.buffers.push(raw), + TempResource::Buffer(raw) => { + last_resources.buffers.insert(raw.as_info().id(), raw); + } + TempResource::StagingBuffer(raw) => { + last_resources + .staging_buffers + .insert(raw.as_info().id(), raw); + } TempResource::Texture(raw, views) => { - last_resources.textures.push(raw); - last_resources.texture_views.extend(views); + last_resources.textures.insert(raw.as_info().id(), raw); + views.into_iter().for_each(|v| { + last_resources.texture_views.insert(v.as_info().id(), v); + }); } } } @@ -426,9 +328,7 @@ impl LifetimeTracker { pub fn cleanup(&mut self) { profiling::scope!("LifetimeTracker::cleanup"); - unsafe { - self.free_resources.clean(); - } + self.free_resources.clear(); } pub fn schedule_resource_destruction( @@ -442,10 +342,17 @@ impl LifetimeTracker { .find(|a| a.index == last_submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources); match temp_resource { - TempResource::Buffer(raw) => resources.buffers.push(raw), + TempResource::Buffer(raw) => { + resources.buffers.insert(raw.as_info().id(), raw); + } + TempResource::StagingBuffer(raw) => { + resources.staging_buffers.insert(raw.as_info().id(), raw); + } TempResource::Texture(raw, views) => { - resources.texture_views.extend(views); - resources.textures.push(raw); + views.into_iter().for_each(|v| { + resources.texture_views.insert(v.as_info().id(), v); + }); + resources.textures.insert(raw.as_info().id(), raw); } } } @@ -575,7 +482,7 @@ impl LifetimeTracker { .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) .bind_groups - .push(bind_group.clone()); + .insert(bind_group_id, bind_group.clone()); } !is_removed }); @@ -619,7 +526,7 @@ impl LifetimeTracker { .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) .texture_views - .push(view.clone()); + .insert(view_id, view.clone()); } !is_removed }); @@ -659,11 +566,15 @@ impl LifetimeTracker { ref clear_views, .. } = &*texture.clear_mode.read() { - non_referenced_resources - .texture_views - .extend(clear_views.iter().cloned()); + clear_views.into_iter().for_each(|v| { + non_referenced_resources + .texture_views + .insert(v.as_info().id(), v.clone()); + }); } - non_referenced_resources.textures.push(texture.clone()); + non_referenced_resources + .textures + .insert(texture_id, texture.clone()); } !is_removed }); @@ -702,7 +613,7 @@ impl LifetimeTracker { .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) .samplers - .push(sampler.clone()); + .insert(sampler_id, sampler.clone()); } !is_removed }); @@ -740,14 +651,16 @@ impl LifetimeTracker { ref stage_buffer, .. } = *buffer.map_state.lock() { - self.free_resources.buffers.push(stage_buffer.clone()); + self.free_resources + .buffers + .insert(stage_buffer.as_info().id(), stage_buffer.clone()); } self.active .iter_mut() .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) .buffers - .push(buffer.clone()); + .insert(buffer_id, buffer.clone()); } !is_removed }); @@ -793,8 +706,8 @@ impl LifetimeTracker { .iter_mut() .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) - .compute_pipes - .push(compute_pipeline.clone()); + .compute_pipelines + .insert(compute_pipeline_id, compute_pipeline.clone()); } !is_removed }, @@ -842,8 +755,8 @@ impl LifetimeTracker { .iter_mut() .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) - .render_pipes - .push(render_pipeline.clone()); + .render_pipelines + .insert(render_pipeline_id, render_pipeline.clone()); } !is_removed }); @@ -871,12 +784,12 @@ impl LifetimeTracker { .find(|a| a.index == *submit_index) .map_or(&self.free_resources, |a| &a.last_resources); - resources.compute_pipes.iter().for_each(|p| { + resources.compute_pipelines.iter().for_each(|(_id, p)| { if p.layout.as_info().id() == *pipeline_layout_id { num_ref_in_nonreferenced_resources += 1; } }); - resources.render_pipes.iter().for_each(|p| { + resources.render_pipelines.iter().for_each(|(_id, p)| { if p.layout.as_info().id() == *pipeline_layout_id { num_ref_in_nonreferenced_resources += 1; } @@ -898,7 +811,7 @@ impl LifetimeTracker { } self.free_resources .pipeline_layouts - .push(pipeline_layout.clone()); + .insert(*pipeline_layout_id, pipeline_layout.clone()); return false; } else { @@ -936,13 +849,13 @@ impl LifetimeTracker { .find(|a| a.index == *submit_index) .map_or(&self.free_resources, |a| &a.last_resources); - resources.bind_groups.iter().for_each(|b| { + resources.bind_groups.iter().for_each(|(_id, b)| { if b.layout.as_info().id() == *bind_group_layout_id { num_ref_in_nonreferenced_resources += 1; } }); - resources.bind_group_layouts.iter().for_each(|b| { - if b.as_info().id() == *bind_group_layout_id { + resources.bind_group_layouts.iter().for_each(|(id, _b)| { + if id == bind_group_layout_id { num_ref_in_nonreferenced_resources += 1; } }); @@ -954,21 +867,21 @@ impl LifetimeTracker { .find(|a| a.index == *submit_index) .map_or(&self.free_resources, |a| &a.last_resources); - resources.compute_pipes.iter().for_each(|p| { + resources.compute_pipelines.iter().for_each(|(_id, p)| { p.layout.bind_group_layouts.iter().for_each(|b| { if b.as_info().id() == *bind_group_layout_id { num_ref_in_nonreferenced_resources += 1; } }); }); - resources.render_pipes.iter().for_each(|p| { + resources.render_pipelines.iter().for_each(|(_id, p)| { p.layout.bind_group_layouts.iter().for_each(|b| { if b.as_info().id() == *bind_group_layout_id { num_ref_in_nonreferenced_resources += 1; } }); }); - resources.pipeline_layouts.iter().for_each(|p| { + resources.pipeline_layouts.iter().for_each(|(_id, p)| { p.bind_group_layouts.iter().for_each(|b| { if b.as_info().id() == *bind_group_layout_id { num_ref_in_nonreferenced_resources += 1; @@ -991,7 +904,7 @@ impl LifetimeTracker { self.free_resources .bind_group_layouts - .push(bind_group_layout.clone()); + .insert(*bind_group_layout_id, bind_group_layout.clone()); return false; } else { @@ -1036,7 +949,7 @@ impl LifetimeTracker { .find(|a| a.index == submit_index) .map_or(&mut self.free_resources, |a| &mut a.last_resources) .query_sets - .push(query_set.clone()); + .insert(query_set_id, query_set.clone()); } !is_removed }); @@ -1217,7 +1130,9 @@ impl LifetimeTracker { if is_removed { *buffer.map_state.lock() = resource::BufferMapState::Idle; log::info!("Buffer {:?} is not tracked anymore", buffer_id); - self.free_resources.buffers.push(buffer.clone()); + self.free_resources + .buffers + .insert(buffer_id, buffer.clone()); } else { let mapping = match std::mem::replace( &mut *buffer.map_state.lock(), diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index e450cbe090..8f103a9ae6 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -6,13 +6,13 @@ use crate::{ ClearError, CommandBuffer, CopySide, ImageCopyTexture, TransferError, }, conv, - device::{DeviceError, WaitIdleError}, + device::{life::ResourceMaps, DeviceError, WaitIdleError}, get_lowest_common_denom, global::Global, hal_api::HalApi, id::{self, QueueId}, identity::{GlobalIdentityHandlerFactory, Input}, - init_tracker::{has_copy_partial_init_tracker_coverage, BufferInitTracker, TextureInitRange}, + init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange}, resource::{ Buffer, BufferAccessError, BufferMapState, Resource, ResourceInfo, StagingBuffer, Texture, TextureInner, TextureView, @@ -21,7 +21,7 @@ use crate::{ }; use hal::{CommandEncoder as _, Device as _, Queue as _}; -use parking_lot::{Mutex, RwLock}; +use parking_lot::Mutex; use smallvec::SmallVec; use std::{ iter, mem, ptr, @@ -160,6 +160,7 @@ pub struct WrappedSubmissionIndex { #[derive(Debug)] pub enum TempResource { Buffer(Arc>), + StagingBuffer(Arc>), Texture(Arc>, SmallVec<[Arc>; 1]>), } @@ -235,23 +236,9 @@ impl PendingWrites { self.temp_resources.push(resource); } - fn consume(&mut self, device: &Arc>, buffer: Arc>) { + fn consume(&mut self, buffer: Arc>) { self.temp_resources - .push(TempResource::Buffer(Arc::new(Buffer:: { - raw: buffer.raw.lock().take(), - device: device.clone(), - usage: wgt::BufferUsages::empty(), - size: buffer.size, - initialization_status: RwLock::new(BufferInitTracker::new(buffer.size)), - sync_mapped_writes: Mutex::new(None), - map_state: Mutex::new(crate::resource::BufferMapState::Idle), - info: ResourceInfo::new( - #[cfg(debug_assertions)] - &buffer.info.label, - #[cfg(not(debug_assertions))] - "", - ), - }))); + .push(TempResource::StagingBuffer(buffer)); } #[must_use] @@ -431,12 +418,15 @@ impl Global { let mut pending_writes = device.pending_writes.lock(); let pending_writes = pending_writes.as_mut().unwrap(); + let stage_fid = hub.staging_buffers.request::(); + let staging_buffer = stage_fid.init(staging_buffer); + if let Err(flush_error) = unsafe { profiling::scope!("copy"); ptr::copy_nonoverlapping(data.as_ptr(), staging_buffer_ptr, data.len()); staging_buffer.flush(device.raw()) } { - pending_writes.consume(device, Arc::new(staging_buffer)); + pending_writes.consume(staging_buffer); return Err(flush_error.into()); } @@ -448,7 +438,7 @@ impl Global { buffer_offset, ); - pending_writes.consume(device, Arc::new(staging_buffer)); + pending_writes.consume(staging_buffer); result } @@ -510,7 +500,7 @@ impl Global { // be freed, even if an error occurs. All paths from here must call // `device.pending_writes.consume`. if let Err(flush_error) = unsafe { staging_buffer.flush(device.raw()) } { - pending_writes.consume(device, staging_buffer); + pending_writes.consume(staging_buffer); return Err(flush_error.into()); } @@ -522,7 +512,7 @@ impl Global { buffer_offset, ); - pending_writes.consume(device, staging_buffer); + pending_writes.consume(staging_buffer); result } @@ -820,6 +810,9 @@ impl Global { // `device.pending_writes.consume`. let (staging_buffer, staging_buffer_ptr) = prepare_staging_buffer(device, stage_size)?; + let stage_fid = hub.staging_buffers.request::(); + let staging_buffer = stage_fid.init(staging_buffer); + if stage_bytes_per_row == bytes_per_row { profiling::scope!("copy aligned"); // Fast path if the data is already being aligned optimally. @@ -854,7 +847,7 @@ impl Global { } if let Err(e) = unsafe { staging_buffer.flush(device.raw()) } { - pending_writes.consume(device, Arc::new(staging_buffer)); + pending_writes.consume(staging_buffer); return Err(e.into()); } @@ -893,7 +886,7 @@ impl Global { } } - pending_writes.consume(device, Arc::new(staging_buffer)); + pending_writes.consume(staging_buffer); pending_writes .dst_textures .insert(destination.texture, dst.clone()); @@ -1119,8 +1112,6 @@ impl Global { let mut fence = device.fence.write(); let fence = fence.as_mut().unwrap(); - device.temp_suspected.lock().clear(); - let submit_index = device .active_submission_index .fetch_add(1, Ordering::Relaxed) @@ -1137,6 +1128,12 @@ impl Global { //TODO: if multiple command buffers are submitted, we can re-use the last // native command buffer of the previous chain instead of always creating // a temporary one, since the chains are not finished. + let mut temp_suspected = device.temp_suspected.lock(); + { + let mut suspected = temp_suspected.take().unwrap(); + suspected.clear(); + temp_suspected.replace(ResourceMaps::new()); + } // finish all the command buffers first for &cmb_id in command_buffer_ids { @@ -1200,9 +1197,9 @@ impl Global { unsafe { device.raw().unmap_buffer(raw_buf) } .map_err(DeviceError::from)?; } - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .buffers .insert(id, buffer.clone()); } else { @@ -1226,9 +1223,9 @@ impl Global { }; texture.info.use_at(submit_index); if texture.is_unique() { - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .textures .insert(id, texture.clone()); } @@ -1243,9 +1240,9 @@ impl Global { for texture_view in cmd_buf_trackers.views.used_resources() { texture_view.info.use_at(submit_index); if texture_view.is_unique() { - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .texture_views .insert(texture_view.as_info().id(), texture_view.clone()); } @@ -1263,9 +1260,9 @@ impl Global { sampler.info.use_at(submit_index); } if bg.is_unique() { - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .bind_groups .insert(bg.as_info().id(), bg.clone()); } @@ -1277,7 +1274,7 @@ impl Global { { compute_pipeline.info.use_at(submit_index); if compute_pipeline.is_unique() { - device.temp_suspected.lock().compute_pipelines.insert( + temp_suspected.as_mut().unwrap().compute_pipelines.insert( compute_pipeline.as_info().id(), compute_pipeline.clone(), ); @@ -1288,7 +1285,7 @@ impl Global { { render_pipeline.info.use_at(submit_index); if render_pipeline.is_unique() { - device.temp_suspected.lock().render_pipelines.insert( + temp_suspected.as_mut().unwrap().render_pipelines.insert( render_pipeline.as_info().id(), render_pipeline.clone(), ); @@ -1297,9 +1294,9 @@ impl Global { for query_set in cmd_buf_trackers.query_sets.used_resources() { query_set.info.use_at(submit_index); if query_set.is_unique() { - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .query_sets .insert(query_set.as_info().id(), query_set.clone()); } @@ -1317,9 +1314,9 @@ impl Global { query_set.info.use_at(submit_index); } if bundle.is_unique() { - device - .temp_suspected - .lock() + temp_suspected + .as_mut() + .unwrap() .render_bundles .insert(bundle.as_info().id(), bundle.clone()); } diff --git a/wgpu-core/src/device/resource.rs b/wgpu-core/src/device/resource.rs index c95c7a8274..105d8c7f62 100644 --- a/wgpu-core/src/device/resource.rs +++ b/wgpu-core/src/device/resource.rs @@ -51,7 +51,7 @@ use std::{ }; use super::{ - life::{self, SuspectedResources}, + life::{self, ResourceMaps}, queue::{self}, DeviceDescriptor, DeviceError, ImplicitPipelineContext, UserClosures, EP_FAILURE, IMPLICIT_FAILURE, ZERO_BUFFER_SIZE, @@ -104,7 +104,7 @@ pub struct Device { life_tracker: Mutex>, /// Temporary storage for resource management functions. Cleared at the end /// of every call (unless an error occurs). - pub(crate) temp_suspected: Mutex>, + pub(crate) temp_suspected: Mutex>>, pub(crate) alignments: hal::Alignments, pub(crate) limits: wgt::Limits, pub(crate) features: wgt::Features, @@ -240,7 +240,7 @@ impl Device { fence: RwLock::new(Some(fence)), trackers: Mutex::new(Tracker::new()), life_tracker: Mutex::new(life::LifetimeTracker::new()), - temp_suspected: Mutex::new(life::SuspectedResources::new()), + temp_suspected: Mutex::new(Some(life::ResourceMaps::new())), #[cfg(feature = "trace")] trace: Mutex::new(trace_path.and_then(|path| match trace::Trace::new(path) { Ok(mut trace) => { @@ -299,10 +299,11 @@ impl Device { // call, and cleared by the end. But `Global::queue_submit` is // fallible; if it exits early, it may leave some resources in // `temp_suspected`. - life_tracker - .suspected_resources - .extend(&self.temp_suspected.lock()); - self.temp_suspected.lock().clear(); + { + let temp_suspected = self.temp_suspected.lock().take().unwrap(); + life_tracker.suspected_resources.extend(temp_suspected); + } + self.temp_suspected.lock().replace(ResourceMaps::new()); life_tracker.triage_suspected( hub, @@ -356,7 +357,7 @@ impl Device { } pub(crate) fn untrack(&self, trackers: &Tracker) { - let mut temp_suspected = self.temp_suspected.lock(); + let mut temp_suspected = self.temp_suspected.lock().take().unwrap(); temp_suspected.clear(); // As the tracker is cleared/dropped, we need to consider all the resources // that it references for destruction in the next GC pass. @@ -418,10 +419,8 @@ impl Device { } } } - - self.lock_life().suspected_resources.extend(&temp_suspected); - - temp_suspected.clear(); + self.lock_life().suspected_resources.extend(temp_suspected); + self.temp_suspected.lock().replace(ResourceMaps::new()); } pub(crate) fn create_buffer( diff --git a/wgpu-core/src/identity.rs b/wgpu-core/src/identity.rs index e5a8b79665..fa7d057dd5 100644 --- a/wgpu-core/src/identity.rs +++ b/wgpu-core/src/identity.rs @@ -1,8 +1,11 @@ use parking_lot::Mutex; use wgt::Backend; -use crate::{id, Epoch, Index}; -use std::{fmt::Debug, marker::PhantomData, sync::Arc}; +use crate::{ + id::{self}, + Epoch, Index, +}; +use std::{collections::HashMap, fmt::Debug, marker::PhantomData, sync::Arc}; /// A simple structure to allocate [`Id`] identifiers. /// @@ -33,19 +36,9 @@ use std::{fmt::Debug, marker::PhantomData, sync::Arc}; /// [`free`]: IdentityManager::free #[derive(Debug, Default)] pub(super) struct IdentityValues { - /// Available index values. If empty, then `epochs.len()` is the next index - /// to allocate. - free: Vec, - - /// The next or currently-live epoch value associated with each `Id` index. - /// - /// If there is a live id with index `i`, then `epochs[i]` is its epoch; any - /// id with the same index but an older epoch is dead. - /// - /// If index `i` is currently unused, `epochs[i]` is the epoch to use in its - /// next `Id`. - epochs: Vec, - + free: Vec<(Index, Epoch)>, + //sorted by Index + used: HashMap>, count: usize, } @@ -57,28 +50,34 @@ impl IdentityValues { pub fn alloc(&mut self, backend: Backend) -> I { self.count += 1; match self.free.pop() { - Some(index) => I::zip(index, self.epochs[index as usize], backend), + Some((index, epoch)) => I::zip(index, epoch + 1, backend), None => { let epoch = 1; - let id = I::zip(self.epochs.len() as Index, epoch, backend); - self.epochs.push(epoch); - id + let used = self.used.entry(epoch).or_default(); + let index = if let Some(i) = used.iter().max_by_key(|v| *v) { + i + 1 + } else { + 0 + }; + used.push(index); + I::zip(index, epoch, backend) } } } + pub fn mark_as_used(&mut self, id: I) -> I { + self.count += 1; + let (index, epoch, _backend) = id.unzip(); + let used = self.used.entry(epoch).or_default(); + used.push(index); + id + } + /// Free `id`. It will never be returned from `alloc` again. pub fn release(&mut self, id: I) { let (index, epoch, _backend) = id.unzip(); - let pe = &mut self.epochs[index as usize]; - assert_eq!(*pe, epoch); - // If the epoch reaches EOL, the index doesn't go - // into the free list, will never be reused again. - if epoch < id::EPOCH_MASK { - *pe = epoch + 1; - self.free.push(index); - self.count -= 1; - } + self.free.push((index, epoch)); + self.count -= 1; } pub fn count(&self) -> usize { @@ -96,6 +95,9 @@ impl IdentityManager { pub fn process(&self, backend: Backend) -> I { self.values.lock().alloc(backend) } + pub fn mark_as_used(&self, id: I) -> I { + self.values.lock().mark_as_used(id) + } pub fn free(&self, id: I) { self.values.lock().release(id) } @@ -121,8 +123,10 @@ pub trait IdentityHandlerFactory { /// and are not generated by wgpu /// /// [`IdentityManager`]: IdentityManager - fn spawn(&self) -> Option>>; - + fn spawn(&self) -> Arc> { + Arc::new(IdentityManager::new()) + } + fn autogenerate_ids() -> bool; fn input_to_id(id_in: Self::Input) -> I; } @@ -136,9 +140,8 @@ pub struct IdentityManagerFactory; impl IdentityHandlerFactory for IdentityManagerFactory { type Input = (); - - fn spawn(&self) -> Option>> { - Some(Arc::new(IdentityManager::new())) + fn autogenerate_ids() -> bool { + true } fn input_to_id(_id_in: Self::Input) -> I { @@ -177,12 +180,13 @@ pub type Input = >::Input; fn test_epoch_end_of_life() { use id::TypedId as _; let man = IdentityManager::::new(); - man.values.lock().epochs.push(id::EPOCH_MASK); - man.values.lock().free.push(0); - let id1 = man.values.lock().alloc::(Backend::Empty); - assert_eq!(id1.unzip().0, 0); - man.values.lock().release(id1); - let id2 = man.values.lock().alloc::(Backend::Empty); - // confirm that the index 0 is no longer re-used + let forced_id = man.mark_as_used(id::BufferId::zip(0, 1, Backend::Empty)); + assert_eq!(forced_id.unzip().0, 0); + let id1 = man.process(Backend::Empty); + assert_eq!(id1.unzip().0, 1); + man.free(id1); + let id2 = man.process(Backend::Empty); + // confirm that the epoch 1 is no longer re-used assert_eq!(id2.unzip().0, 1); + assert_eq!(id2.unzip().1, 2); } diff --git a/wgpu-core/src/registry.rs b/wgpu-core/src/registry.rs index 770738977c..310c6814a0 100644 --- a/wgpu-core/src/registry.rs +++ b/wgpu-core/src/registry.rs @@ -39,7 +39,7 @@ impl RegistryReport { /// #[derive(Debug)] pub struct Registry> { - identity: Option>>, + identity: Arc>, storage: RwLock>, backend: Backend, } @@ -61,7 +61,7 @@ impl> Registry { #[must_use] pub(crate) struct FutureId<'a, I: id::TypedId, T: Resource> { id: I, - identity: Option>>, + identity: Arc>, data: &'a RwLock>, } @@ -75,9 +75,13 @@ impl> FutureId<'_, I, T> { self.id } - pub fn assign(self, mut value: T) -> (I, Arc) { + pub fn init(&self, mut value: T) -> Arc { value.as_info_mut().set_id(self.id, &self.identity); - self.data.write().insert(self.id, Arc::new(value)); + Arc::new(value) + } + + pub fn assign(self, value: T) -> (I, Arc) { + self.data.write().insert(self.id, self.init(value)); (self.id, self.data.read().get(self.id).unwrap().clone()) } @@ -100,14 +104,25 @@ impl> Registry { F: IdentityHandlerFactory, { FutureId { - id: match self.identity.as_ref() { - Some(identity) => identity.process(self.backend), - _ => F::input_to_id(id_in), + id: if F::autogenerate_ids() { + self.identity.process(self.backend) + } else { + self.identity.mark_as_used(F::input_to_id(id_in)) }, identity: self.identity.clone(), data: &self.storage, } } + pub(crate) fn request(&self) -> FutureId + where + F: IdentityHandlerFactory, + { + FutureId { + id: self.identity.process(self.backend), + identity: self.identity.clone(), + data: &self.storage, + } + } pub(crate) fn contains(&self, id: I) -> bool { self.read().contains(id) } @@ -131,6 +146,11 @@ impl> Registry { value.as_info_mut().set_id(id, &self.identity); storage.force_replace(id, value) } + pub fn force_replace_with_error(&self, id: I, label: &str) { + let mut storage = self.storage.write(); + storage.remove(id); + storage.insert_error(id, label); + } pub(crate) fn unregister(&self, id: I) -> Option> { let value = self.storage.write().remove(id); //Returning None is legal if it's an error ID @@ -164,9 +184,7 @@ impl> Registry { element_size: std::mem::size_of::(), ..Default::default() }; - if let Some(identity) = self.identity.as_ref() { - report.num_allocated = identity.values.lock().count(); - } + report.num_allocated = self.identity.values.lock().count(); for element in storage.map.iter() { match *element { Element::Occupied(..) => report.num_kept_from_user += 1, diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index 82dd9600b5..bb28ce805e 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -115,9 +115,9 @@ impl ResourceInfo { self.id.unwrap() } - pub(crate) fn set_id(&mut self, id: Id, identity: &Option>>) { + pub(crate) fn set_id(&mut self, id: Id, identity: &Arc>) { self.id = Some(id); - self.identity = identity.clone(); + self.identity = Some(identity.clone()); } /// Record that this resource will be used by the queue submission with the