Skip to content

Commit 362e440

Browse files
committed
add temp_resources to CommandBufferMutable, BakedCommands & EncoderInFlight, remove it from ActiveSubmission
Use `CommandBufferMutable.temp_resources` to store the ray tracing related staging buffers and scratch buffers. These shouldn't have been stored in `PendingWrites.temp_resources` because they can get destroyed early (before the command buffer that uses them has been submitted) if any other command buffer gets submitted first.
1 parent 43e0ebd commit 362e440

File tree

4 files changed

+26
-45
lines changed

4 files changed

+26
-45
lines changed

wgpu-core/src/command/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub use timestamp_writes::PassTimestampWrites;
2828

2929
use self::memory_init::CommandBufferTextureMemoryActions;
3030

31+
use crate::device::queue::TempResource;
3132
use crate::device::{Device, DeviceError, MissingFeatures};
3233
use crate::lock::{rank, Mutex};
3334
use crate::snatch::SnatchGuard;
@@ -432,6 +433,7 @@ impl Drop for CommandEncoder {
432433
pub(crate) struct BakedCommands {
433434
pub(crate) encoder: CommandEncoder,
434435
pub(crate) trackers: Tracker,
436+
pub(crate) temp_resources: Vec<TempResource>,
435437
buffer_memory_init_actions: Vec<BufferInitTrackerAction>,
436438
texture_memory_actions: CommandBufferTextureMemoryActions,
437439
}
@@ -460,6 +462,7 @@ pub struct CommandBufferMutable {
460462

461463
blas_actions: Vec<BlasAction>,
462464
tlas_actions: Vec<TlasAction>,
465+
temp_resources: Vec<TempResource>,
463466

464467
#[cfg(feature = "trace")]
465468
pub(crate) commands: Option<Vec<TraceCommand>>,
@@ -479,6 +482,7 @@ impl CommandBufferMutable {
479482
BakedCommands {
480483
encoder: self.encoder,
481484
trackers: self.trackers,
485+
temp_resources: self.temp_resources,
482486
buffer_memory_init_actions: self.buffer_memory_init_actions,
483487
texture_memory_actions: self.texture_memory_actions,
484488
}
@@ -545,6 +549,7 @@ impl CommandBuffer {
545549
pending_query_resets: QueryResetMap::new(),
546550
blas_actions: Default::default(),
547551
tlas_actions: Default::default(),
552+
temp_resources: Default::default(),
548553
#[cfg(feature = "trace")]
549554
commands: if device.trace.lock().is_some() {
550555
Some(Vec::new())

wgpu-core/src/command/ray_tracing.rs

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -329,12 +329,9 @@ impl Global {
329329
}
330330
}
331331

332-
if let Some(queue) = device.get_queue() {
333-
queue
334-
.pending_writes
335-
.lock()
336-
.consume_temp(TempResource::ScratchBuffer(scratch_buffer));
337-
}
332+
cmd_buf_data
333+
.temp_resources
334+
.push(TempResource::ScratchBuffer(scratch_buffer));
338335

339336
cmd_buf_data_guard.mark_successful();
340337
Ok(())
@@ -725,21 +722,15 @@ impl Global {
725722
}
726723

727724
if let Some(staging_buffer) = staging_buffer {
728-
if let Some(queue) = device.get_queue() {
729-
queue
730-
.pending_writes
731-
.lock()
732-
.consume_temp(TempResource::StagingBuffer(staging_buffer));
733-
}
725+
cmd_buf_data
726+
.temp_resources
727+
.push(TempResource::StagingBuffer(staging_buffer));
734728
}
735729
}
736730

737-
if let Some(queue) = device.get_queue() {
738-
queue
739-
.pending_writes
740-
.lock()
741-
.consume_temp(TempResource::ScratchBuffer(scratch_buffer));
742-
}
731+
cmd_buf_data
732+
.temp_resources
733+
.push(TempResource::ScratchBuffer(scratch_buffer));
743734

744735
cmd_buf_data_guard.mark_successful();
745736
Ok(())

wgpu-core/src/device/life.rs

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ struct ActiveSubmission {
2929
/// submission has completed.
3030
index: SubmissionIndex,
3131

32-
/// Temporary resources to be freed once this queue submission has completed.
33-
temp_resources: Vec<TempResource>,
34-
3532
/// Buffers to be mapped once this submission has completed.
3633
mapped: Vec<Arc<Buffer>>,
3734

@@ -107,11 +104,6 @@ impl ActiveSubmission {
107104

108105
pub fn contains_blas(&self, blas: &Blas) -> bool {
109106
for encoder in &self.encoders {
110-
// The ownership location of blas's depends on where the command encoder
111-
// came from. If it is the staging command encoder on the queue, it is
112-
// in the pending buffer list. If it came from a user command encoder,
113-
// it is in the tracker.
114-
115107
if encoder.trackers.blas_s.contains(blas) {
116108
return true;
117109
}
@@ -122,11 +114,6 @@ impl ActiveSubmission {
122114

123115
pub fn contains_tlas(&self, tlas: &Tlas) -> bool {
124116
for encoder in &self.encoders {
125-
// The ownership location of tlas's depends on where the command encoder
126-
// came from. If it is the staging command encoder on the queue, it is
127-
// in the pending buffer list. If it came from a user command encoder,
128-
// it is in the tracker.
129-
130117
if encoder.trackers.tlas_s.contains(tlas) {
131118
return true;
132119
}
@@ -203,15 +190,9 @@ impl LifetimeTracker {
203190
}
204191

205192
/// Start tracking resources associated with a new queue submission.
206-
pub fn track_submission(
207-
&mut self,
208-
index: SubmissionIndex,
209-
temp_resources: impl Iterator<Item = TempResource>,
210-
encoders: Vec<EncoderInFlight>,
211-
) {
193+
pub fn track_submission(&mut self, index: SubmissionIndex, encoders: Vec<EncoderInFlight>) {
212194
self.active.push(ActiveSubmission {
213195
index,
214-
temp_resources: temp_resources.collect(),
215196
mapped: Vec::new(),
216197
encoders,
217198
work_done_closures: SmallVec::new(),
@@ -332,7 +313,6 @@ impl LifetimeTracker {
332313
profiling::scope!("drop command buffer trackers");
333314
drop(encoder);
334315
}
335-
drop(a.temp_resources);
336316
work_done_closures.extend(a.work_done_closures);
337317
}
338318
work_done_closures
@@ -347,7 +327,12 @@ impl LifetimeTracker {
347327
.active
348328
.iter_mut()
349329
.find(|a| a.index == last_submit_index)
350-
.map(|a| &mut a.temp_resources);
330+
.map(|a| {
331+
// Because this resource's `last_submit_index` matches `a.index`,
332+
// we know that we must have done something with the resource,
333+
// so `a.encoders` should not be empty.
334+
&mut a.encoders.last_mut().unwrap().temp_resources
335+
});
351336
if let Some(resources) = resources {
352337
resources.push(temp_resource);
353338
}

wgpu-core/src/device/queue.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ pub enum TempResource {
268268
pub(crate) struct EncoderInFlight {
269269
inner: crate::command::CommandEncoder,
270270
pub(crate) trackers: Tracker,
271+
pub(crate) temp_resources: Vec<TempResource>,
271272

272273
/// These are the buffers that have been tracked by `PendingWrites`.
273274
pub(crate) pending_buffers: FastHashMap<TrackerIndex, Arc<Buffer>>,
@@ -377,6 +378,7 @@ impl PendingWrites {
377378
hal_label: None,
378379
},
379380
trackers: Tracker::new(),
381+
temp_resources: mem::take(&mut self.temp_resources),
380382
pending_buffers,
381383
pending_textures,
382384
};
@@ -1195,6 +1197,7 @@ impl Queue {
11951197
active_executions.push(EncoderInFlight {
11961198
inner: baked.encoder,
11971199
trackers: baked.trackers,
1200+
temp_resources: baked.temp_resources,
11981201
pending_buffers: FastHashMap::default(),
11991202
pending_textures: FastHashMap::default(),
12001203
});
@@ -1293,11 +1296,8 @@ impl Queue {
12931296
profiling::scope!("cleanup");
12941297

12951298
// this will register the new submission to the life time tracker
1296-
self.lock_life().track_submission(
1297-
submit_index,
1298-
pending_writes.temp_resources.drain(..),
1299-
active_executions,
1300-
);
1299+
self.lock_life()
1300+
.track_submission(submit_index, active_executions);
13011301
drop(pending_writes);
13021302

13031303
// This will schedule destruction of all resources that are no longer needed

0 commit comments

Comments
 (0)