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 latest_submission_index #5976

Merged
merged 4 commits into from
Jul 18, 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
2 changes: 1 addition & 1 deletion benches/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ pollster.workspace = true
profiling.workspace = true
rayon.workspace = true
tracy-client = { workspace = true, optional = true }
wgpu = { workspace = true, features = ["wgsl"] }
wgpu = { workspace = true, features = ["wgsl", "metal", "dx12"] }
47 changes: 37 additions & 10 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::{
present,
resource::{
self, BufferAccessError, BufferAccessResult, BufferMapOperation, CreateBufferError,
Trackable,
},
storage::Storage,
Label,
Expand Down Expand Up @@ -260,15 +259,25 @@ impl Global {
) -> Result<(), WaitIdleError> {
let hub = A::hub(self);

let last_submission = match hub.buffers.read().get(buffer_id) {
Ok(buffer) => buffer.submission_index(),
let device = hub
.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?;

let buffer = match hub.buffers.get(buffer_id) {
Ok(buffer) => buffer,
Err(_) => return Ok(()),
};

hub.devices
.get(device_id)
.map_err(|_| DeviceError::InvalidDeviceId)?
.wait_for_submit(last_submission)
let last_submission = device
.lock_life()
.get_buffer_latest_submission_index(&buffer);

if let Some(last_submission) = last_submission {
device.wait_for_submit(last_submission)
} else {
Ok(())
}
}

#[doc(hidden)]
Expand Down Expand Up @@ -424,7 +433,13 @@ impl Global {
);

if wait {
let last_submit_index = buffer.submission_index();
let Some(last_submit_index) = buffer
.device
.lock_life()
.get_buffer_latest_submission_index(&buffer)
else {
return;
};
match buffer.device.wait_for_submit(last_submit_index) {
Ok(()) => (),
Err(e) => log::error!("Failed to wait for buffer {:?}: {}", buffer_id, e),
Expand Down Expand Up @@ -599,7 +614,13 @@ impl Global {
}

if wait {
let last_submit_index = texture.submission_index();
let Some(last_submit_index) = texture
.device
.lock_life()
.get_texture_latest_submission_index(&texture)
else {
return;
};
match texture.device.wait_for_submit(last_submit_index) {
Ok(()) => (),
Err(e) => log::error!("Failed to wait for texture {texture_id:?}: {e}"),
Expand Down Expand Up @@ -672,7 +693,13 @@ impl Global {
}

if wait {
let last_submit_index = view.submission_index();
let Some(last_submit_index) = view
.device
.lock_life()
.get_texture_latest_submission_index(&view.parent)
else {
return Ok(());
};
match view.device.wait_for_submit(last_submit_index) {
Ok(()) => (),
Err(e) => {
Expand Down
106 changes: 96 additions & 10 deletions wgpu-core/src/device/life.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
},
hal_api::HalApi,
id,
resource::{self, Buffer, Labeled, Trackable},
resource::{self, Buffer, Labeled, Texture, Trackable},
snatch::SnatchGuard,
SubmissionIndex,
};
Expand Down Expand Up @@ -55,6 +55,58 @@ struct ActiveSubmission<A: HalApi> {
work_done_closures: SmallVec<[SubmittedWorkDoneClosure; 1]>,
}

impl<A: HalApi> ActiveSubmission<A> {
/// Returns true if this submission contains the given buffer.
///
/// This only uses constant-time operations.
pub fn contains_buffer(&self, buffer: &Buffer<A>) -> bool {
for encoder in &self.encoders {
// The ownership location of buffers depends on where the command encoder
// came from. If it is the staging command encoder on the queue, it is
// in the pending buffer list. If it came from a user command encoder,
// it is in the tracker.

if encoder.trackers.buffers.contains(buffer) {
return true;
}

if encoder
.pending_buffers
.contains_key(&buffer.tracker_index())
{
return true;
}
}

false
}

/// Returns true if this submission contains the given texture.
///
/// This only uses constant-time operations.
pub fn contains_texture(&self, texture: &Texture<A>) -> bool {
for encoder in &self.encoders {
// The ownership location of textures depends on where the command encoder
// came from. If it is the staging command encoder on the queue, it is
// in the pending buffer list. If it came from a user command encoder,
// it is in the tracker.

if encoder.trackers.textures.contains(texture) {
return true;
}

if encoder
.pending_textures
.contains_key(&texture.tracker_index())
{
return true;
}
}

false
}
}

#[derive(Clone, Debug, Error)]
#[non_exhaustive]
pub enum WaitIdleError {
Expand Down Expand Up @@ -165,6 +217,40 @@ impl<A: HalApi> LifetimeTracker<A> {
self.mapped.push(value.clone());
}

/// Returns the submission index of the most recent submission that uses the
/// given buffer.
pub fn get_buffer_latest_submission_index(
&self,
buffer: &Buffer<A>,
) -> Option<SubmissionIndex> {
// We iterate in reverse order, so that we can bail out early as soon
// as we find a hit.
self.active.iter().rev().find_map(|submission| {
if submission.contains_buffer(buffer) {
Some(submission.index)
} else {
None
}
})
}

/// Returns the submission index of the most recent submission that uses the
/// given texture.
pub fn get_texture_latest_submission_index(
&self,
texture: &Texture<A>,
) -> Option<SubmissionIndex> {
// We iterate in reverse order, so that we can bail out early as soon
// as we find a hit.
self.active.iter().rev().find_map(|submission| {
if submission.contains_texture(texture) {
Some(submission.index)
} else {
None
}
})
}

/// Sort out the consequences of completed submissions.
///
/// Assume that all submissions up through `last_done` have completed.
Expand Down Expand Up @@ -236,9 +322,7 @@ impl<A: HalApi> LifetimeTracker<A> {
}
}
}
}

impl<A: HalApi> LifetimeTracker<A> {
/// Determine which buffers are ready to map, and which must wait for the
/// GPU.
///
Expand All @@ -249,17 +333,19 @@ impl<A: HalApi> LifetimeTracker<A> {
}

for buffer in self.mapped.drain(..) {
let submit_index = buffer.submission_index();
let submission = self
.active
.iter_mut()
.rev()
.find(|a| a.contains_buffer(&buffer));

log::trace!(
"Mapping of {} at submission {:?} gets assigned to active {:?}",
"Mapping of {} at submission {:?}",
buffer.error_ident(),
submit_index,
self.active.iter().position(|a| a.index == submit_index)
submission.as_deref().map(|s| s.index)
);

self.active
.iter_mut()
.find(|a| a.index == submit_index)
submission
.map_or(&mut self.ready_to_map, |a| &mut a.mapped)
.push(buffer);
}
Expand Down
85 changes: 8 additions & 77 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,12 @@ pub enum TempResource<A: HalApi> {
pub(crate) struct EncoderInFlight<A: HalApi> {
raw: A::CommandEncoder,
cmd_buffers: Vec<A::CommandBuffer>,
trackers: Tracker<A>,
pub(crate) trackers: Tracker<A>,

/// These are the buffers that have been tracked by `PendingWrites`.
pending_buffers: Vec<Arc<Buffer<A>>>,
pub(crate) pending_buffers: FastHashMap<TrackerIndex, Arc<Buffer<A>>>,
/// These are the textures that have been tracked by `PendingWrites`.
pending_textures: Vec<Arc<Texture<A>>>,
pub(crate) pending_textures: FastHashMap<TrackerIndex, Arc<Texture<A>>>,
}

impl<A: HalApi> EncoderInFlight<A> {
Expand Down Expand Up @@ -268,8 +268,8 @@ impl<A: HalApi> PendingWrites<A> {
queue: &A::Queue,
) -> Result<Option<EncoderInFlight<A>>, DeviceError> {
if self.is_recording {
let pending_buffers = self.dst_buffers.drain().map(|(_, b)| b).collect();
let pending_textures = self.dst_textures.drain().map(|(_, t)| t).collect();
let pending_buffers = mem::take(&mut self.dst_buffers);
let pending_textures = mem::take(&mut self.dst_textures);

let cmd_buf = unsafe { self.command_encoder.end_encoding()? };
self.is_recording = false;
Expand Down Expand Up @@ -570,8 +570,6 @@ impl Global {

self.queue_validate_write_buffer_impl(&dst, buffer_offset, staging_buffer.size)?;

dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let region = hal::BufferCopy {
src_offset: 0,
dst_offset: buffer_offset,
Expand Down Expand Up @@ -762,7 +760,6 @@ impl Global {
// call above. Since we've held `texture_guard` the whole time, we know
// the texture hasn't gone away in the mean time, so we can unwrap.
let dst = hub.textures.get(destination.texture).unwrap();
dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let dst_raw = dst.try_raw(&snatch_guard)?;

Expand Down Expand Up @@ -1007,7 +1004,6 @@ impl Global {
.drain(init_layer_range);
}
}
dst.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let snatch_guard = device.snatchable_lock.read();
let dst_raw = dst.try_raw(&snatch_guard)?;
Expand Down Expand Up @@ -1126,7 +1122,7 @@ impl Global {
}

{
profiling::scope!("update submission ids");
profiling::scope!("check resource state");

let cmd_buf_data = cmdbuf.data.lock();
let cmd_buf_trackers = &cmd_buf_data.as_ref().unwrap().trackers;
Expand All @@ -1136,7 +1132,6 @@ impl Global {
profiling::scope!("buffers");
for buffer in cmd_buf_trackers.buffers.used_resources() {
buffer.check_destroyed(&snatch_guard)?;
buffer.use_at(submit_index);

match *buffer.map_state.lock() {
BufferMapState::Idle => (),
Expand All @@ -1163,7 +1158,6 @@ impl Global {
true
}
};
texture.use_at(submit_index);
if should_extend {
unsafe {
used_surface_textures
Expand All @@ -1177,69 +1171,6 @@ impl Global {
}
}
}
{
profiling::scope!("views");
for texture_view in cmd_buf_trackers.views.used_resources() {
texture_view.use_at(submit_index);
}
}
{
profiling::scope!("bind groups (+ referenced views/samplers)");
for bg in cmd_buf_trackers.bind_groups.used_resources() {
bg.use_at(submit_index);
// We need to update the submission indices for the contained
// state-less (!) resources as well, so that they don't get
// deleted too early if the parent bind group goes out of scope.
for view in bg.used.views.used_resources() {
view.use_at(submit_index);
}
for sampler in bg.used.samplers.used_resources() {
sampler.use_at(submit_index);
}
}
}
{
profiling::scope!("compute pipelines");
for compute_pipeline in
cmd_buf_trackers.compute_pipelines.used_resources()
{
compute_pipeline.use_at(submit_index);
}
}
{
profiling::scope!("render pipelines");
for render_pipeline in
cmd_buf_trackers.render_pipelines.used_resources()
{
render_pipeline.use_at(submit_index);
}
}
{
profiling::scope!("query sets");
for query_set in cmd_buf_trackers.query_sets.used_resources() {
query_set.use_at(submit_index);
}
}
{
profiling::scope!(
"render bundles (+ referenced pipelines/query sets)"
);
for bundle in cmd_buf_trackers.bundles.used_resources() {
bundle.use_at(submit_index);
// We need to update the submission indices for the contained
// state-less (!) resources as well, excluding the bind groups.
// They don't get deleted too early if the bundle goes out of scope.
for render_pipeline in
bundle.used.render_pipelines.read().used_resources()
{
render_pipeline.use_at(submit_index);
}
for query_set in bundle.used.query_sets.read().used_resources()
{
query_set.use_at(submit_index);
}
}
}
}
let mut baked = cmdbuf.from_arc_into_baked();

Expand Down Expand Up @@ -1303,8 +1234,8 @@ impl Global {
raw: baked.encoder,
cmd_buffers: baked.list,
trackers: baked.trackers,
pending_buffers: Vec::new(),
pending_textures: Vec::new(),
pending_buffers: FastHashMap::default(),
pending_textures: FastHashMap::default(),
});
}

Expand Down
Loading
Loading