Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Clone Staring Me In the Face #6969

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions wgpu-core/src/track/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use crate::{
resource::{Buffer, Trackable},
snatch::SnatchGuard,
track::{
invalid_resource_state, skip_barrier, ResourceMetadata, ResourceMetadataProvider,
ResourceUsageCompatibilityError, ResourceUses,
invalid_resource_state, metadata::ArcEq, skip_barrier, ResourceMetadata,
ResourceMetadataProvider, ResourceUsageCompatibilityError, ResourceUses,
},
};
use hal::{BufferBarrier, BufferUses};
Expand Down Expand Up @@ -681,7 +681,7 @@ impl BufferStateProvider<'_> {
}

#[inline(always)]
unsafe fn insert<T: Clone>(
unsafe fn insert<T: Clone + ArcEq>(
start_states: Option<&mut [BufferUses]>,
current_states: &mut [BufferUses],
resource_metadata: &mut ResourceMetadata<T>,
Expand All @@ -706,7 +706,7 @@ unsafe fn insert<T: Clone>(
*current_states.get_unchecked_mut(index) = new_end_state;

let resource = metadata_provider.get(index);
resource_metadata.insert(index, resource.clone());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bad boi

resource_metadata.insert(index, resource);
}
}

Expand Down
35 changes: 29 additions & 6 deletions wgpu-core/src/track/metadata.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,26 @@
//! The `ResourceMetadata` type.

use std::sync::{Arc, Weak};

use bit_vec::BitVec;
use wgt::strict_assert;

pub(super) trait ArcEq {
fn arc_eq(&self, other: &Self) -> bool;
}

impl<T> ArcEq for Arc<T> {
fn arc_eq(&self, other: &Self) -> bool {
Arc::ptr_eq(self, other)
}
}

impl<T> ArcEq for Weak<T> {
fn arc_eq(&self, _other: &Self) -> bool {
true
}
}

/// A set of resources, holding a `Arc<T>` and epoch for each member.
///
/// Testing for membership is fast, and iterating over members is
Expand All @@ -11,15 +29,15 @@ use wgt::strict_assert;
/// members, but a bit vector tracks occupancy, so iteration touches
/// only occupied elements.
#[derive(Debug)]
pub(super) struct ResourceMetadata<T: Clone> {
pub(super) struct ResourceMetadata<T: Clone + ArcEq> {
/// If the resource with index `i` is a member, `owned[i]` is `true`.
owned: BitVec<usize>,

/// A vector holding clones of members' `T`s.
resources: Vec<Option<T>>,
}

impl<T: Clone> ResourceMetadata<T> {
impl<T: Clone + ArcEq> ResourceMetadata<T> {
pub(super) fn new() -> Self {
Self {
owned: BitVec::default(),
Expand Down Expand Up @@ -88,10 +106,15 @@ impl<T: Clone> ResourceMetadata<T> {
/// The given `index` must be in bounds for this `ResourceMetadata`'s
/// existing tables. See `tracker_assert_in_bounds`.
#[inline(always)]
pub(super) unsafe fn insert(&mut self, index: usize, resource: T) -> &T {
pub(super) unsafe fn insert(&mut self, index: usize, new_resource: &T) {
self.owned.set(index, true);
let resource_dst = unsafe { self.resources.get_unchecked_mut(index) };
resource_dst.insert(resource)
match resource_dst {
Some(old_resource) => debug_assert!(old_resource.arc_eq(new_resource)),
None => {
*resource_dst = Some(new_resource.clone());
}
}
}

/// Get the resource with the given index.
Expand Down Expand Up @@ -142,13 +165,13 @@ impl<T: Clone> ResourceMetadata<T> {
///
/// This is used to abstract over the various places
/// trackers can get new resource metadata from.
pub(super) enum ResourceMetadataProvider<'a, T: Clone> {
pub(super) enum ResourceMetadataProvider<'a, T: Clone + ArcEq> {
/// Comes directly from explicit values.
Direct { resource: &'a T },
/// Comes from another metadata tracker.
Indirect { metadata: &'a ResourceMetadata<T> },
}
impl<T: Clone> ResourceMetadataProvider<'_, T> {
impl<T: Clone + ArcEq> ResourceMetadataProvider<'_, T> {
/// Get a reference to the resource from this.
///
/// # Safety
Expand Down
7 changes: 3 additions & 4 deletions wgpu-core/src/track/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ use crate::{
resource::{Texture, TextureInner, TextureView, Trackable},
snatch::SnatchGuard,
track::{
invalid_resource_state, skip_barrier, ResourceMetadata, ResourceMetadataProvider,
ResourceUsageCompatibilityError, ResourceUses,
invalid_resource_state, metadata::ArcEq, skip_barrier, ResourceMetadata, ResourceMetadataProvider, ResourceUsageCompatibilityError, ResourceUses
},
};
use hal::{TextureBarrier, TextureUses};
Expand Down Expand Up @@ -1082,7 +1081,7 @@ unsafe fn insert_or_barrier_update(
}

#[inline(always)]
unsafe fn insert<T: Clone>(
unsafe fn insert<T: Clone + ArcEq>(
texture_selector: Option<&TextureSelector>,
start_state: Option<&mut TextureStateSet>,
end_state: &mut TextureStateSet,
Expand Down Expand Up @@ -1152,7 +1151,7 @@ unsafe fn insert<T: Clone>(

unsafe {
let resource = metadata_provider.get(index);
resource_metadata.insert(index, resource.clone());
resource_metadata.insert(index, resource);
}
}

Expand Down
Loading