Use a unique tracker index per resource instead of the ID in trackers (…

…gfx-rs#5244)

Co-authored-by: Connor Fitzgerald <[email protected]>

CHANGELOG.md

@@ -119,6 +119,7 @@ Bottom level categories:
 - 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)
 - Fix an issue where command encoders weren't properly freed if an error occurred during command encoding. By @ErichDonGubler in [#5251](https://github.com/gfx-rs/wgpu/pull/5251).
+- Fix registry leaks with de-duplicated resources. By @nical in [#5244](https://github.com/gfx-rs/wgpu/pull/5244)
 - Fix behavior of integer `clamp` when `min` argument > `max` argument. By @cwfitzgerald in [#5300](https://github.com/gfx-rs/wgpu/pull/5300).
 - Fix missing validation for `Device::clear_buffer` where `offset + size buffer.size` was not checked when `size` was omitted. By @ErichDonGubler in [#5282](https://github.com/gfx-rs/wgpu/pull/5282).
 

tests/tests/bind_group_layout_dedup.rs

@@ -128,8 +128,12 @@ async fn bgl_dedupe(ctx: TestingContext) {
         .panic_on_timeout();
 
     if ctx.adapter_info.backend != wgt::Backend::BrowserWebGpu {
+        // Indices are made reusable as soon as the handle is dropped so we keep them around
+        // for the duration of the loop.
+        let mut bgls = Vec::new();
+        let mut indices = Vec::new();
         // Now all of the BGL ids should be dead, so we should get the same ids again.
-        for i in 0..=2 {
+        for _ in 0..=2 {
             let test_bgl = ctx
                 .device
                 .create_bind_group_layout(&wgpu::BindGroupLayoutDescriptor {
@@ -138,15 +142,14 @@ async fn bgl_dedupe(ctx: TestingContext) {
                 });
 
             let test_bgl_idx = test_bgl.global_id().inner() & 0xFFFF_FFFF;
-
-            // https://github.com/gfx-rs/wgpu/issues/4912
-            //
-            // ID 2 is the deduplicated ID, which is never properly recycled.
-            if i == 2 {
-                assert_eq!(test_bgl_idx, 3);
-            } else {
-                assert_eq!(test_bgl_idx, i);
-            }
+            bgls.push(test_bgl);
+            indices.push(test_bgl_idx);
+        }
+        // We don't guarantee that the IDs will appear in the same order. Sort them
+        // and check that they all appear exactly once.
+        indices.sort();
+        for (i, index) in indices.iter().enumerate() {
+            assert_eq!(*index, i as u64);
         }
     }
 }

tests/tests/mem_leaks.rs

@@ -127,7 +127,7 @@ async fn draw_test_with_reports(
 
     let global_report = ctx.instance.generate_report().unwrap();
     let report = global_report.hub_report(ctx.adapter_info.backend);
-    assert_eq!(report.shader_modules.num_allocated, 1);
+    assert_eq!(report.shader_modules.num_allocated, 0);
     assert_eq!(report.shader_modules.num_kept_from_user, 0);
     assert_eq!(report.textures.num_allocated, 0);
     assert_eq!(report.texture_views.num_allocated, 0);
@@ -166,7 +166,7 @@ async fn draw_test_with_reports(
     assert_eq!(report.buffers.num_allocated, 1);
     assert_eq!(report.texture_views.num_allocated, 1);
     assert_eq!(report.texture_views.num_kept_from_user, 1);
-    assert_eq!(report.textures.num_allocated, 1);
+    assert_eq!(report.textures.num_allocated, 0);
     assert_eq!(report.textures.num_kept_from_user, 0);
 
     let mut encoder = ctx
@@ -204,7 +204,7 @@ async fn draw_test_with_reports(
     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.textures.num_allocated, 1);
+    assert_eq!(report.textures.num_allocated, 0);
 
     function(&mut rpass);
 
@@ -227,19 +227,20 @@ async fn draw_test_with_reports(
     assert_eq!(report.texture_views.num_kept_from_user, 0);
     assert_eq!(report.textures.num_kept_from_user, 0);
     assert_eq!(report.command_buffers.num_allocated, 1);
-    assert_eq!(report.render_pipelines.num_allocated, 1);
-    assert_eq!(report.pipeline_layouts.num_allocated, 1);
-    assert_eq!(report.bind_group_layouts.num_allocated, 1);
-    assert_eq!(report.bind_groups.num_allocated, 1);
-    assert_eq!(report.buffers.num_allocated, 1);
-    assert_eq!(report.texture_views.num_allocated, 1);
-    assert_eq!(report.textures.num_allocated, 1);
+    assert_eq!(report.render_pipelines.num_allocated, 0);
+    assert_eq!(report.pipeline_layouts.num_allocated, 0);
+    assert_eq!(report.bind_group_layouts.num_allocated, 0);
+    assert_eq!(report.bind_groups.num_allocated, 0);
+    assert_eq!(report.buffers.num_allocated, 0);
+    assert_eq!(report.texture_views.num_allocated, 0);
+    assert_eq!(report.textures.num_allocated, 0);
 
     let submit_index = ctx.queue.submit(Some(encoder.finish()));
 
-    let global_report = ctx.instance.generate_report().unwrap();
-    let report = global_report.hub_report(ctx.adapter_info.backend);
-    assert_eq!(report.command_buffers.num_allocated, 0);
+    // TODO: fix in https://github.com/gfx-rs/wgpu/pull/5141
+    // let global_report = ctx.instance.generate_report().unwrap();
+    // let report = global_report.hub_report(ctx.adapter_info.backend);
+    // assert_eq!(report.command_buffers.num_allocated, 0);
 
     ctx.async_poll(wgpu::Maintain::wait_for(submit_index))
         .await

wgpu-core/src/command/bundle.rs

@@ -758,7 +758,10 @@ impl RenderBundleEncoder {
             buffer_memory_init_actions,
             texture_memory_init_actions,
             context: self.context,
-            info: ResourceInfo::new(desc.label.borrow_or_default()),
+            info: ResourceInfo::new(
+                desc.label.borrow_or_default(),
+                Some(device.tracker_indices.bundles.clone()),
+            ),
             discard_hal_labels: device
                 .instance_flags
                 .contains(wgt::InstanceFlags::DISCARD_HAL_LABELS),

wgpu-core/src/command/compute.rs

@@ -1,6 +1,7 @@
 use crate::device::DeviceError;
 use crate::resource::Resource;
 use crate::snatch::SnatchGuard;
+use crate::track::TrackerIndex;
 use crate::{
     binding_model::{
         BindError, BindGroup, LateMinBufferBindingSizeMismatch, PushConstantUploadError,
@@ -305,7 +306,7 @@ impl<A: HalApi> State<A> {
         raw_encoder: &mut A::CommandEncoder,
         base_trackers: &mut Tracker<A>,
         bind_group_guard: &Storage<BindGroup<A>>,
-        indirect_buffer: Option<id::BufferId>,
+        indirect_buffer: Option<TrackerIndex>,
         snatch_guard: &SnatchGuard,
     ) -> Result<(), UsageConflict> {
         for id in self.binder.list_active() {
@@ -402,12 +403,11 @@ impl Global {
         let pipeline_guard = hub.compute_pipelines.read();
         let query_set_guard = hub.query_sets.read();
         let buffer_guard = hub.buffers.read();
-        let texture_guard = hub.textures.read();
 
         let mut state = State {
             binder: Binder::new(),
             pipeline: None,
-            scope: UsageScope::new(&*buffer_guard, &*texture_guard),
+            scope: UsageScope::new(&device.tracker_indices),
             debug_scope_depth: 0,
         };
         let mut temp_offsets = Vec::new();
@@ -452,17 +452,14 @@ impl Global {
 
         let snatch_guard = device.snatchable_lock.read();
 
-        tracker.set_size(
-            Some(&*buffer_guard),
-            Some(&*texture_guard),
-            None,
-            None,
-            Some(&*bind_group_guard),
-            Some(&*pipeline_guard),
-            None,
-            None,
-            Some(&*query_set_guard),
-        );
+        let indices = &device.tracker_indices;
+        tracker.buffers.set_size(indices.buffers.size());
+        tracker.textures.set_size(indices.textures.size());
+        tracker.bind_groups.set_size(indices.bind_groups.size());
+        tracker
+            .compute_pipelines
+            .set_size(indices.compute_pipelines.size());
+        tracker.query_sets.set_size(indices.query_sets.size());
 
         let discard_hal_labels = self
             .instance
@@ -757,7 +754,7 @@ impl Global {
                             raw,
                             &mut intermediate_trackers,
                             &*bind_group_guard,
-                            Some(buffer_id),
+                            Some(indirect_buffer.as_info().tracker_index()),
                             &snatch_guard,
                         )
                         .map_pass_err(scope)?;

wgpu-core/src/command/draw.rs

@@ -277,7 +277,6 @@ pub enum ArcRenderCommand<A: HalApi> {
     SetStencilReference(u32),
     SetViewport {
         rect: Rect<f32>,
-        //TODO: use half-float to reduce the size?
         depth_min: f32,
         depth_max: f32,
     },

wgpu-core/src/command/mod.rs

@@ -174,6 +174,7 @@ impl<A: HalApi> CommandBuffer<A> {
                     .as_ref()
                     .unwrap_or(&String::from("<CommandBuffer>"))
                     .as_str(),
+                None,
             ),
             data: Mutex::new(Some(CommandBufferMutable {
                 encoder: CommandEncoder {

wgpu-core/src/command/render.rs

@@ -22,7 +22,7 @@ use crate::{
     hal_label, id,
     init_tracker::{MemoryInitKind, TextureInitRange, TextureInitTrackerAction},
     pipeline::{self, PipelineFlags},
-    resource::{Buffer, QuerySet, Texture, TextureView, TextureViewNotRenderableReason},
+    resource::{QuerySet, Texture, TextureView, TextureViewNotRenderableReason},
     storage::Storage,
     track::{TextureSelector, Tracker, UsageConflict, UsageScope},
     validation::{
@@ -801,8 +801,6 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
         texture_memory_actions: &mut CommandBufferTextureMemoryActions<A>,
         pending_query_resets: &mut QueryResetMap<A>,
         view_guard: &'a Storage<TextureView<A>>,
-        buffer_guard: &'a Storage<Buffer<A>>,
-        texture_guard: &'a Storage<Texture<A>>,
         query_set_guard: &'a Storage<QuerySet<A>>,
         snatch_guard: &SnatchGuard<'a>,
     ) -> Result<Self, RenderPassErrorInner> {
@@ -1216,7 +1214,7 @@ impl<'a, A: HalApi> RenderPassInfo<'a, A> {
 
         Ok(Self {
             context,
-            usage_scope: UsageScope::new(buffer_guard, texture_guard),
+            usage_scope: UsageScope::new(&device.tracker_indices),
             render_attachments,
             is_depth_read_only,
             is_stencil_read_only,
@@ -1388,7 +1386,6 @@ impl Global {
             let render_pipeline_guard = hub.render_pipelines.read();
             let query_set_guard = hub.query_sets.read();
             let buffer_guard = hub.buffers.read();
-            let texture_guard = hub.textures.read();
             let view_guard = hub.texture_views.read();
 
             log::trace!(
@@ -1408,24 +1405,21 @@ impl Global {
                 texture_memory_actions,
                 pending_query_resets,
                 &*view_guard,
-                &*buffer_guard,
-                &*texture_guard,
                 &*query_set_guard,
                 &snatch_guard,
             )
             .map_pass_err(pass_scope)?;
 
-            tracker.set_size(
-                Some(&*buffer_guard),
-                Some(&*texture_guard),
-                Some(&*view_guard),
-                None,
-                Some(&*bind_group_guard),
-                None,
-                Some(&*render_pipeline_guard),
-                Some(&*bundle_guard),
-                Some(&*query_set_guard),
-            );
+            let indices = &device.tracker_indices;
+            tracker.buffers.set_size(indices.buffers.size());
+            tracker.textures.set_size(indices.textures.size());
+            tracker.views.set_size(indices.texture_views.size());
+            tracker.bind_groups.set_size(indices.bind_groups.size());
+            tracker
+                .render_pipelines
+                .set_size(indices.render_pipelines.size());
+            tracker.bundles.set_size(indices.bundles.size());
+            tracker.query_sets.set_size(indices.query_sets.size());
 
             let raw = &mut encoder.raw;