From ac166ead29782cf48949e065d4d52d9d51515e84 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 9 Feb 2024 11:16:46 +0100 Subject: [PATCH 1/4] Simplify the ID allocation in IdentityValues --- wgpu-core/src/identity.rs | 48 +++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/wgpu-core/src/identity.rs b/wgpu-core/src/identity.rs index 0e34055c74..a8b19f2d3c 100644 --- a/wgpu-core/src/identity.rs +++ b/wgpu-core/src/identity.rs @@ -3,7 +3,7 @@ use wgt::Backend; use crate::{ id::{Id, Marker}, - Epoch, FastHashMap, Index, + Epoch, Index, }; use std::{fmt::Debug, marker::PhantomData}; @@ -37,9 +37,12 @@ use std::{fmt::Debug, marker::PhantomData}; #[derive(Debug, Default)] pub(super) struct IdentityValues { free: Vec<(Index, Epoch)>, - //sorted by Index - used: FastHashMap>, + next_index: Index, count: usize, + // Sanity check: The allocation logic works under the assumption that we don't + // do a mix of allocating ids from here and providing ids manually for the same + // storage container. + allocate_ids: Option, } impl IdentityValues { @@ -48,28 +51,32 @@ impl IdentityValues { /// The backend is incorporated into the id, so that ids allocated with /// different `backend` values are always distinct. pub fn alloc(&mut self, backend: Backend) -> Id { + assert!( + self.allocate_ids != Some(false), + "Mix of internally allocated and externally provided IDs" + ); + self.allocate_ids = Some(true); + self.count += 1; match self.free.pop() { Some((index, epoch)) => Id::zip(index, epoch + 1, backend), None => { + let index = self.next_index; + self.next_index += 1; let epoch = 1; - let used = self.used.entry(epoch).or_insert_with(Default::default); - let index = if let Some(i) = used.iter().max_by_key(|v| *v) { - i + 1 - } else { - 0 - }; - used.push(index); Id::zip(index, epoch, backend) } } } pub fn mark_as_used(&mut self, id: Id) -> Id { + assert!( + self.allocate_ids != Some(true), + "Mix of internally allocated and externally provided IDs" + ); + self.allocate_ids = Some(false); + self.count += 1; - let (index, epoch, _backend) = id.unzip(); - let used = self.used.entry(epoch).or_insert_with(Default::default); - used.push(index); id } @@ -106,7 +113,12 @@ impl IdentityManager { impl IdentityManager { pub fn new() -> Self { Self { - values: Mutex::new(IdentityValues::default()), + values: Mutex::new(IdentityValues { + free: Vec::new(), + next_index: 0, + count: 0, + allocate_ids: None, + }), _phantom: PhantomData, } } @@ -115,15 +127,11 @@ impl IdentityManager { #[test] fn test_epoch_end_of_life() { use crate::id; - let man = IdentityManager::::new(); - 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); + assert_eq!(id1.unzip(), (0, 1, Backend::Empty)); 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); + assert_eq!(id2.unzip(), (0, 2, Backend::Empty)); } From 421137dffde34242994760e7e7bce2bd49cb7d26 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Fri, 9 Feb 2024 12:00:49 +0100 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 999e221f1a..b3c7c0f825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ Bottom level categories: - Fix panic when creating a surface while no backend is available. By @wumpf [#5166](https://github.com/gfx-rs/wgpu/pull/5166) - Correctly compute minimum buffer size for array-typed `storage` and `uniform` vars. By @jimblandy [#5222](https://github.com/gfx-rs/wgpu/pull/5222) - Fix timeout when presenting a surface where no work has been done. By @waywardmonkeys in [#5200](https://github.com/gfx-rs/wgpu/pull/5200) +- Simplify and speed up the allocation of internal IDs. By @nical in [#5229](https://github.com/gfx-rs/wgpu/pull/5229) #### WGL From 1d87b9a2c7e7a228178882fe58a9e59a4863304e Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Thu, 15 Feb 2024 10:52:03 +0100 Subject: [PATCH 3/4] Use an enum --- wgpu-core/src/identity.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/wgpu-core/src/identity.rs b/wgpu-core/src/identity.rs index a8b19f2d3c..e3e0fb0ac5 100644 --- a/wgpu-core/src/identity.rs +++ b/wgpu-core/src/identity.rs @@ -7,6 +7,13 @@ use crate::{ }; use std::{fmt::Debug, marker::PhantomData}; +#[derive(Copy, Clone, Debug, PartialEq)] +enum IdSource { + External, + Allocated, + None, +} + /// A simple structure to allocate [`Id`] identifiers. /// /// Calling [`alloc`] returns a fresh, never-before-seen id. Calling [`free`] @@ -34,7 +41,7 @@ use std::{fmt::Debug, marker::PhantomData}; /// [`Backend`]: wgt::Backend; /// [`alloc`]: IdentityManager::alloc /// [`free`]: IdentityManager::free -#[derive(Debug, Default)] +#[derive(Debug)] pub(super) struct IdentityValues { free: Vec<(Index, Epoch)>, next_index: Index, @@ -42,7 +49,7 @@ pub(super) struct IdentityValues { // Sanity check: The allocation logic works under the assumption that we don't // do a mix of allocating ids from here and providing ids manually for the same // storage container. - allocate_ids: Option, + id_source: IdSource, } impl IdentityValues { @@ -52,10 +59,10 @@ impl IdentityValues { /// different `backend` values are always distinct. pub fn alloc(&mut self, backend: Backend) -> Id { assert!( - self.allocate_ids != Some(false), + self.id_source != IdSource::External, "Mix of internally allocated and externally provided IDs" ); - self.allocate_ids = Some(true); + self.id_source = IdSource::Allocated; self.count += 1; match self.free.pop() { @@ -71,10 +78,10 @@ impl IdentityValues { pub fn mark_as_used(&mut self, id: Id) -> Id { assert!( - self.allocate_ids != Some(true), + self.id_source != IdSource::Allocated, "Mix of internally allocated and externally provided IDs" ); - self.allocate_ids = Some(false); + self.id_source = IdSource::External; self.count += 1; id @@ -117,7 +124,7 @@ impl IdentityManager { free: Vec::new(), next_index: 0, count: 0, - allocate_ids: None, + id_source: IdSource::None, }), _phantom: PhantomData, } From 3c6c0882e5d4d0ea850bf12d90c6ce90f044e3d4 Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Thu, 15 Feb 2024 17:33:11 +0100 Subject: [PATCH 4/4] Don't build a free list of IDs if they are provided externally --- wgpu-core/src/identity.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/wgpu-core/src/identity.rs b/wgpu-core/src/identity.rs index e3e0fb0ac5..d76d29341a 100644 --- a/wgpu-core/src/identity.rs +++ b/wgpu-core/src/identity.rs @@ -89,8 +89,10 @@ impl IdentityValues { /// Free `id`. It will never be returned from `alloc` again. pub fn release(&mut self, id: Id) { - let (index, epoch, _backend) = id.unzip(); - self.free.push((index, epoch)); + if let IdSource::Allocated = self.id_source { + let (index, epoch, _backend) = id.unzip(); + self.free.push((index, epoch)); + } self.count -= 1; }