Skip to content

Commit

Permalink
Avoid cloning Arcs unnecessarily when iterating trackers (#6721)
Browse files Browse the repository at this point in the history
* Avoid cloning Arcs unnecessarily when iterating trackers

* Changelog entry
  • Loading branch information
nical authored Dec 13, 2024
1 parent 3918a09 commit 3bcfe84
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
- Check that at least one index is specified.
- Reject destroyed buffers in query set resolution. By @ErichDonGubler in [#6579](https://github.com/gfx-rs/wgpu/pull/6579).
- Fix panic when dropping `Device` on some environments. By @Dinnerbone in [#6681](https://github.com/gfx-rs/wgpu/pull/6681).
- Reduced the overhead of command buffer validation. By @nical in [#6721](https://github.com/gfx-rs/wgpu/pull/6721).

#### Naga

Expand Down
5 changes: 2 additions & 3 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,16 +1568,15 @@ fn validate_command_buffer(
TextureInner::Native { .. } => false,
TextureInner::Surface { .. } => {
// Compare the Arcs by pointer as Textures don't implement Eq.
submit_surface_textures_owned
.insert(Arc::as_ptr(&texture), texture.clone());
submit_surface_textures_owned.insert(Arc::as_ptr(texture), texture.clone());

true
}
};
if should_extend {
unsafe {
used_surface_textures
.merge_single(&texture, None, hal::TextureUses::PRESENT)
.merge_single(texture, None, hal::TextureUses::PRESENT)
.unwrap();
};
}
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3637,12 +3637,12 @@ impl Device {
// During these iterations, we discard all errors. We don't care!
let trackers = self.trackers.lock();
for buffer in trackers.buffers.used_resources() {
if let Some(buffer) = Weak::upgrade(&buffer) {
if let Some(buffer) = Weak::upgrade(buffer) {
let _ = buffer.destroy();
}
}
for texture in trackers.textures.used_resources() {
if let Some(texture) = Weak::upgrade(&texture) {
if let Some(texture) = Weak::upgrade(texture) {
let _ = texture.destroy();
}
}
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/track/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ impl BufferTracker {
}

/// Returns a list of all buffers tracked.
pub fn used_resources(&self) -> impl Iterator<Item = Arc<Buffer>> + '_ {
pub fn used_resources(&self) -> impl Iterator<Item = &Arc<Buffer>> + '_ {
self.metadata.owned_resources()
}

Expand Down Expand Up @@ -559,7 +559,7 @@ impl DeviceBufferTracker {
}

/// Returns a list of all buffers tracked.
pub fn used_resources(&self) -> impl Iterator<Item = Weak<Buffer>> + '_ {
pub fn used_resources(&self) -> impl Iterator<Item = &Weak<Buffer>> + '_ {
self.metadata.owned_resources()
}

Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/track/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,13 @@ impl<T: Clone> ResourceMetadata<T> {
}

/// Returns an iterator over the resources owned by `self`.
pub(super) fn owned_resources(&self) -> impl Iterator<Item = T> + '_ {
pub(super) fn owned_resources(&self) -> impl Iterator<Item = &T> + '_ {
if !self.owned.is_empty() {
self.tracker_assert_in_bounds(self.owned.len() - 1)
};
iterate_bitvec_indices(&self.owned).map(move |index| {
let resource = unsafe { self.resources.get_unchecked(index) };
resource.as_ref().unwrap().clone()
resource.as_ref().unwrap()
})
}

Expand Down
5 changes: 2 additions & 3 deletions wgpu-core/src/track/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,10 +429,9 @@ impl TextureTracker {
}

/// Returns a list of all textures tracked.
pub fn used_resources(&self) -> impl Iterator<Item = Arc<Texture>> + '_ {
pub fn used_resources(&self) -> impl Iterator<Item = &Arc<Texture>> + '_ {
self.metadata.owned_resources()
}

/// Drain all currently pending transitions.
pub fn drain_transitions<'a>(
&'a mut self,
Expand Down Expand Up @@ -672,7 +671,7 @@ impl DeviceTextureTracker {
}

/// Returns a list of all textures tracked.
pub fn used_resources(&self) -> impl Iterator<Item = Weak<Texture>> + '_ {
pub fn used_resources(&self) -> impl Iterator<Item = &Weak<Texture>> + '_ {
self.metadata.owned_resources()
}

Expand Down

0 comments on commit 3bcfe84

Please sign in to comment.