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

Improve lifetime management of command encoders and buffers #6544

Merged
merged 4 commits into from
Dec 2, 2024
Merged
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
15 changes: 0 additions & 15 deletions wgpu-core/src/command/allocator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use crate::resource_log;

use crate::lock::{rank, Mutex};

/// A pool of free [`wgpu_hal::CommandEncoder`]s, owned by a `Device`.
Expand Down Expand Up @@ -49,17 +47,4 @@ impl CommandAllocator {
let mut free_encoders = self.free_encoders.lock();
free_encoders.push(encoder);
}

/// Free the pool of command encoders.
///
/// This is only called when the `Device` is dropped.
pub(crate) fn dispose(&self, device: &dyn hal::DynDevice) {
let mut free_encoders = self.free_encoders.lock();
resource_log!("CommandAllocator::dispose encoders {}", free_encoders.len());
for cmd_encoder in free_encoders.drain(..) {
unsafe {
device.destroy_command_encoder(cmd_encoder);
}
}
}
}
19 changes: 14 additions & 5 deletions wgpu-core/src/command/clear.rs
teoxoy marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,9 @@ impl Global {
let cmd_buf = hub
.command_buffers
.get(command_encoder_id.into_command_buffer_id());
let mut cmd_buf_data = cmd_buf.try_get()?;
cmd_buf_data.check_recording()?;
let mut cmd_buf_data = cmd_buf.data.lock();
let mut cmd_buf_data_guard = cmd_buf_data.record()?;
let cmd_buf_data = &mut *cmd_buf_data_guard;

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf_data.commands {
Expand Down Expand Up @@ -138,6 +139,8 @@ impl Global {

if offset == end_offset {
log::trace!("Ignoring fill_buffer of size 0");

cmd_buf_data_guard.mark_successful();
return Ok(());
}

Expand All @@ -157,6 +160,8 @@ impl Global {
cmd_buf_raw.transition_buffers(dst_barrier.as_slice());
cmd_buf_raw.clear_buffer(dst_raw, offset..end_offset);
}

cmd_buf_data_guard.mark_successful();
Ok(())
}

Expand All @@ -174,8 +179,9 @@ impl Global {
let cmd_buf = hub
.command_buffers
.get(command_encoder_id.into_command_buffer_id());
let mut cmd_buf_data = cmd_buf.try_get()?;
cmd_buf_data.check_recording()?;
let mut cmd_buf_data = cmd_buf.data.lock();
let mut cmd_buf_data_guard = cmd_buf_data.record()?;
let cmd_buf_data = &mut *cmd_buf_data_guard;

#[cfg(feature = "trace")]
if let Some(ref mut list) = cmd_buf_data.commands {
Expand Down Expand Up @@ -243,7 +249,10 @@ impl Global {
&device.alignments,
device.zero_buffer.as_ref(),
&snatch_guard,
)
)?;

cmd_buf_data_guard.mark_successful();
Ok(())
}
}

Expand Down
34 changes: 13 additions & 21 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::{
end_pipeline_statistics_query,
memory_init::{fixup_discarded_surfaces, SurfacesInDiscardState},
validate_and_begin_pipeline_statistics_query, ArcPassTimestampWrites, BasePass,
BindGroupStateChange, CommandBuffer, CommandEncoderError, CommandEncoderStatus, MapPassErr,
PassErrorScope, PassTimestampWrites, QueryUseError, StateChange,
BindGroupStateChange, CommandBuffer, CommandEncoderError, MapPassErr, PassErrorScope,
PassTimestampWrites, QueryUseError, StateChange,
},
device::{Device, DeviceError, MissingDownlevelFlags, MissingFeatures},
global::Global,
Expand All @@ -30,8 +30,7 @@ use wgt::{BufferAddress, DynamicOffset};

use super::{bind::BinderError, memory_init::CommandBufferTextureMemoryActions};
use crate::ray_tracing::TlasAction;
use std::sync::Arc;
use std::{fmt, mem::size_of, str};
use std::{fmt, mem::size_of, str, sync::Arc};

pub struct ComputePass {
/// All pass data & records is stored here.
Expand Down Expand Up @@ -282,7 +281,9 @@ impl Global {
/// If creation fails, an invalid pass is returned.
/// Any operation on an invalid pass will return an error.
///
/// If successful, puts the encoder into the [`CommandEncoderStatus::Locked`] state.
/// If successful, puts the encoder into the [`Locked`] state.
///
/// [`Locked`]: crate::command::CommandEncoderStatus::Locked
pub fn command_encoder_create_compute_pass(
&self,
encoder_id: id::CommandEncoderId,
Expand All @@ -299,11 +300,7 @@ impl Global {

let cmd_buf = hub.command_buffers.get(encoder_id.into_command_buffer_id());

match cmd_buf
.try_get()
.map_err(|e| e.into())
.and_then(|mut cmd_buf_data| cmd_buf_data.lock_encoder())
{
match cmd_buf.data.lock().lock_encoder() {
Ok(_) => {}
Err(e) => return make_err(e, arc_desc),
};
Expand Down Expand Up @@ -340,7 +337,8 @@ impl Global {
.hub
.command_buffers
.get(encoder_id.into_command_buffer_id());
let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?;
let mut cmd_buf_data = cmd_buf.data.lock();
let cmd_buf_data = cmd_buf_data.get_inner().map_pass_err(pass_scope)?;

if let Some(ref mut list) = cmd_buf_data.commands {
list.push(crate::device::trace::Command::RunComputePass {
Expand Down Expand Up @@ -408,19 +406,16 @@ impl Global {
let device = &cmd_buf.device;
device.check_is_valid().map_pass_err(pass_scope)?;

let mut cmd_buf_data = cmd_buf.try_get().map_pass_err(pass_scope)?;
cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?;
let cmd_buf_data = &mut *cmd_buf_data;
let mut cmd_buf_data = cmd_buf.data.lock();
let mut cmd_buf_data_guard = cmd_buf_data.unlock_encoder().map_pass_err(pass_scope)?;
let cmd_buf_data = &mut *cmd_buf_data_guard;

let encoder = &mut cmd_buf_data.encoder;
let status = &mut cmd_buf_data.status;

// We automatically keep extending command buffers over time, and because
// we want to insert a command buffer _before_ what we're about to record,
// we need to make sure to close the previous one.
encoder.close(&cmd_buf.device).map_pass_err(pass_scope)?;
// will be reset to true if recording is done without errors
*status = CommandEncoderStatus::Error;
let raw_encoder = encoder.open(&cmd_buf.device).map_pass_err(pass_scope)?;

let mut state = State {
Expand Down Expand Up @@ -590,10 +585,6 @@ impl Global {
state.raw_encoder.end_compute_pass();
}

// We've successfully recorded the compute pass, bring the
// command buffer out of the error state.
*status = CommandEncoderStatus::Recording;

let State {
snatch_guard,
tracker,
Expand Down Expand Up @@ -626,6 +617,7 @@ impl Global {
encoder
.close_and_swap(&cmd_buf.device)
.map_pass_err(pass_scope)?;
cmd_buf_data_guard.mark_successful();

Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions wgpu-core/src/command/memory_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl BakedCommands {
let raw_buf = buffer.try_raw(snatch_guard)?;

unsafe {
self.encoder.transition_buffers(
self.encoder.raw.transition_buffers(
transition
.map(|pending| pending.into_hal(&buffer, snatch_guard))
.as_slice(),
Expand All @@ -240,7 +240,7 @@ impl BakedCommands {
);

unsafe {
self.encoder.clear_buffer(raw_buf, range.clone());
self.encoder.raw.clear_buffer(raw_buf, range.clone());
}
}
}
Expand Down Expand Up @@ -293,7 +293,7 @@ impl BakedCommands {
let clear_result = clear_texture(
&texture_use.texture,
range,
self.encoder.as_mut(),
self.encoder.raw.as_mut(),
&mut device_tracker.textures,
&device.alignments,
device.zero_buffer.as_ref(),
Expand Down
Loading
Loading