From 44dafd3b4d5cbb15dd1559f9902d987f76365e09 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 31 Mar 2022 09:36:50 -0700 Subject: [PATCH] Documentation for various things. --- wgpu-core/src/device/life.rs | 59 ++++++++++++++++++++++++++++++++++-- wgpu-core/src/device/mod.rs | 3 ++ wgpu-core/src/hub.rs | 20 ++++++++++-- wgpu-core/src/track/mod.rs | 38 +++++++++++++++++++---- 4 files changed, 108 insertions(+), 12 deletions(-) diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 2cdb848bf1..9d0c8d96e3 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -361,10 +361,24 @@ impl LifetimeTracker { /// Sort out the consequences of completed submissions. /// /// Assume that all submissions up through `last_done` have completed. - /// Buffers they used are now ready to map. Resources for which they were - /// the final use are now ready to free. /// - /// Return a list of `SubmittedWorkDoneClosure`s to run. + /// - Buffers used by those submissions are now ready to map, if + /// requested. Add any buffers in the submission's [`mapped`] list to + /// [`self.ready_to_map`], where [`LifetimeTracker::handle_mapping`] will find + /// them. + /// + /// - Resources whose final use was in those submissions are now ready to + /// free. Add any resources in the submission's [`last_resources`] table + /// to [`self.free_resources`], where [`LifetimeTracker::cleanup`] will find + /// them. + /// + /// Return a list of [`SubmittedWorkDoneClosure`]s to run. + /// + /// [`mapped`]: ActiveSubmission::mapped + /// [`self.ready_to_map`]: LifetimeTracker::ready_to_map + /// [`last_resources`]: ActiveSubmission::last_resources + /// [`self.free_resources`]: LifetimeTracker::free_resources + /// [`SubmittedWorkDoneClosure`]: crate::device::queue::SubmittedWorkDoneClosure #[must_use] pub fn triage_submissions( &mut self, @@ -435,6 +449,45 @@ impl LifetimeTracker { } impl LifetimeTracker { + /// Identify resources to free, according to `trackers` and `self.suspected_resources`. + /// + /// Given `trackers`, the [`TrackerSet`] belonging to same [`Device`] as + /// `self`, and `hub`, the [`Hub`] to which that `Device` belongs: + /// + /// Remove from `trackers` each resource mentioned in + /// [`self.suspected_resources`]. If `trackers` held the final reference to + /// that resource, add it to the appropriate free list, to be destroyed by + /// the hal: + /// + /// - Add resources used by queue submissions still in flight to the + /// [`last_resources`] table of the last such submission's entry in + /// [`self.active`]. When that submission has finished execution. the + /// [`triage_submissions`] method will move them to + /// [`self.free_resources`]. + /// + /// - Add resources that can be freed right now to [`self.free_resources`] + /// directly. [`LifetimeTracker::cleanup`] will take care of them as + /// part of this poll. + /// + /// ## Entrained resources + /// + /// This function finds resources that are used only by other resources + /// ready to be freed, and adds those to the free lists as well. For + /// example, if there's some texture `T` used only by some texture view + /// `TV`, then if `TV` can be freed, `T` gets added to the free lists too. + /// + /// Since `wgpu-core` resource ownership patterns are acyclic, we can visit + /// each type that can be owned after all types that could possibly own + /// it. This way, we can detect all free-able objects in a single pass, + /// simply by starting with types that are roots of the ownership DAG (like + /// render bundles) and working our way towards leaf types (like buffers). + /// + /// [`Device`]: super::Device + /// [`self.suspected_resources`]: LifetimeTracker::suspected_resources + /// [`last_resources`]: ActiveSubmission::last_resources + /// [`self.active`]: LifetimeTracker::active + /// [`triage_submissions`]: LifetimeTracker::triage_submissions + /// [`self.free_resources`]: LifetimeTracker::free_resources pub(super) fn triage_suspected( &mut self, hub: &Hub, diff --git a/wgpu-core/src/device/mod.rs b/wgpu-core/src/device/mod.rs index c7a451f544..e09329f6b3 100644 --- a/wgpu-core/src/device/mod.rs +++ b/wgpu-core/src/device/mod.rs @@ -277,6 +277,9 @@ pub struct Device { command_allocator: Mutex>, pub(crate) active_submission_index: SubmissionIndex, fence: A::Fence, + + /// All live resources allocated with this [`Device`]. + /// /// Has to be locked temporarily only (locked last) pub(crate) trackers: Mutex, // Life tracker should be locked right after the device and before anything else. diff --git a/wgpu-core/src/hub.rs b/wgpu-core/src/hub.rs index 8a56fd12f8..57f6a2dfe9 100644 --- a/wgpu-core/src/hub.rs +++ b/wgpu-core/src/hub.rs @@ -24,8 +24,7 @@ use std::{fmt::Debug, marker::PhantomData, mem, ops}; /// Use `IdentityManager::default` to construct new instances. /// /// `IdentityManager` returns `Id`s whose index values are suitable for use as -/// indices into a vector of information (which you maintain elsewhere) about -/// those ids' referents: +/// indices into a `Storage` that holds those ids' referents: /// /// - Every live id has a distinct index value. Each live id's index selects a /// distinct element in the vector. @@ -88,10 +87,20 @@ impl IdentityManager { } } +/// An entry in a `Storage::map` table. #[derive(Debug)] enum Element { + /// There are no live ids with this index. Vacant, + + /// There is one live id with this index, allocated at the given + /// epoch. Occupied(T, Epoch), + + /// Like `Occupied`, but an error occurred when creating the + /// resource. + /// + /// The given `String` is the resource's descriptor label. Error(Epoch, String), } @@ -112,6 +121,11 @@ impl StorageReport { #[derive(Clone, Debug)] pub(crate) struct InvalidId; +/// A table of `T` values indexed by the id type `I`. +/// +/// The table is represented as a vector indexed by the ids' index +/// values, so you should use an id allocator like `IdentityManager` +/// that keeps the index values dense and close to zero. #[derive(Debug)] pub struct Storage { map: Vec>, @@ -269,7 +283,7 @@ impl Storage { /// If type A implements `Access`, that means we are allowed to proceed /// with locking resource `B` after we lock `A`. /// -/// The implenentations basically describe the edges in a directed graph +/// The implementations basically describe the edges in a directed graph /// of lock transitions. As long as it doesn't have loops, we can have /// multiple concurrent paths on this graph (from multiple threads) without /// deadlocks, i.e. there is always a path whose next resource is not locked diff --git a/wgpu-core/src/track/mod.rs b/wgpu-core/src/track/mod.rs index e71da4ea41..bcf5eb4a4b 100644 --- a/wgpu-core/src/track/mod.rs +++ b/wgpu-core/src/track/mod.rs @@ -230,9 +230,17 @@ impl ResourceTracker { } } - /// Remove the resource from the tracker if it is holding the last reference. + /// Remove the resource `id`, only if `self` is holding the last reference to it. /// - /// Return `true` if we did remove the resource. + /// Return `true` if we did remove the resource; the underlying hal resource + /// is ready to be freed. + /// + /// This is generally only meaningful to apply to members of + /// [`Device::trackers`], which holds all resources allocated with that + /// [`Device`]. Other trackers should never be the final reference. + /// + /// [`Device`]: crate::device::Device + /// [`Device::trackers`]: crate::device::Device::trackers pub(crate) fn remove_abandoned(&mut self, id: Valid) -> bool { let (index, epoch, backend) = id.0.unzip(); debug_assert_eq!(backend, self.backend); @@ -287,7 +295,9 @@ impl ResourceTracker { self.map.clear(); } - /// Initialize a resource to be used. + /// Begin tracking a new resource `id` in state `state`. + /// + /// Hold `ref_count` in the tracker. /// /// Returns false if the resource is already registered. pub(crate) fn init( @@ -324,8 +334,18 @@ impl ResourceTracker { res.state.query(selector) } - /// Make sure that a resource is tracked, and return a mutable - /// reference to it. + /// Make sure that a resource is tracked, and return a mutable reference to it. + /// + /// If the resource isn't tracked, start it in the default state, and take a + /// clone of `ref_count`. + /// + /// The `self_backend` and `map` arguments should be the `backend` and `map` + /// fields of a `ResourceTracker`. Ideally this function would just take + /// `&mut self` and access those from there, but that would upset the borrow + /// checker in some callers, who want to borrow `ResourceTracker::temp` + /// alongside our return value. The approach taken here has the caller + /// borrow both `map` and `temp`, so the borrow checker can see that they + /// don't alias. fn get_or_insert<'a>( self_backend: wgt::Backend, map: &'a mut FastHashMap>, @@ -347,6 +367,7 @@ impl ResourceTracker { } } + /// Return a mutable reference to `id`'s state. fn get<'a>( self_backend: wgt::Backend, map: &'a mut FastHashMap>, @@ -359,7 +380,7 @@ impl ResourceTracker { e } - /// Extend the usage of a specified resource. + /// Extend the usage of `id`, tracking it if necessary. /// /// Returns conflicting transition as an error. pub(crate) fn change_extend( @@ -553,6 +574,11 @@ pub enum UsageConflict { } /// A set of trackers for all relevant resources. +/// +/// `Device` uses this to track all resources allocated from that device. +/// Resources like `BindGroup`, `CommandBuffer`, and so on that may own a +/// variety of other resources also use a value of this type to keep track of +/// everything they're depending on. #[derive(Debug)] pub(crate) struct TrackerSet { pub buffers: ResourceTracker,