Skip to content

Commit

Permalink
Merge #1417
Browse files Browse the repository at this point in the history
1417: Split the tracker into stateful/stateless to reduce the overhead r=cwfitzgerald a=kvark

**Connections**
Implements #1413 (comment)
Reduces the overhead for resource tracking in the Animometer benchmark by up to 50%.

**Description**
We used to use the full tracker set on the usage scopes associated with compute/render passes. A resource tracker has 2 responsibilities: ensuring the resource is held alive, and validating and recording the state transitions. This PR exploits the fact that the latter responsibility is only applicable for buffers and textures. So doing all the lifetime tracking for a pass is a waste: we can instead just attach the lifetimes to the parent command buffer, straight.

In the Animometer benchmark, there is one large buffer, and thousands of bind groups pointing to different offsets into it. The old code would fill up the pass tracker with those bind groups, and then merge it into the command buffer tracker. The new code would just fill up the command buffer tracker instead. Since there is only one buffer, the pass tracking becomes much lighter.

**Testing**
Untested. It would be nice to have some benchmarks here, possibly after #1397 ?

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
  • Loading branch information
bors[bot] and kvark authored Jun 1, 2021
2 parents c332c7f + 7b7e72b commit 9cd0163
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 37 deletions.
2 changes: 1 addition & 1 deletion wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ impl RenderBundleEncoder {
state.set_bind_group(index, bind_group_id, bind_group.layout_id, offsets);
state
.trackers
.merge_extend(&bind_group.used)
.merge_extend_all(&bind_group.used)
.map_pass_err(scope)?;
}
RenderCommand::SetPipeline(pipeline_id) => {
Expand Down
10 changes: 6 additions & 4 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
id,
memory_init_tracker::{MemoryInitKind, MemoryInitTrackerAction},
resource::{Buffer, BufferUse, Texture},
track::{TrackerSet, UsageConflict},
track::{StatefulTrackerSubset, TrackerSet, UsageConflict},
validation::{check_buffer_usage, MissingBufferUsageError},
Label, DOWNLEVEL_ERROR_WARNING_MESSAGE,
};
Expand Down Expand Up @@ -194,7 +194,7 @@ where
struct State {
binder: Binder,
pipeline: StateChange<id::ComputePipelineId>,
trackers: TrackerSet,
trackers: StatefulTrackerSubset,
debug_scope_depth: u32,
}

Expand Down Expand Up @@ -224,14 +224,16 @@ impl State {
) -> Result<(), UsageConflict> {
for id in self.binder.list_active() {
self.trackers.merge_extend(&bind_group_guard[id].used)?;
base_trackers.merge_extend_stateless(&bind_group_guard[id].used);
}

log::trace!("Encoding dispatch barriers");

CommandBuffer::insert_barriers(
raw_cmd_buf,
base_trackers,
&self.trackers,
&self.trackers.buffers,
&self.trackers.textures,
buffer_guard,
texture_guard,
);
Expand Down Expand Up @@ -306,7 +308,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let mut state = State {
binder: Binder::new(),
pipeline: StateChange::new(),
trackers: TrackerSet::new(B::VARIANT),
trackers: StatefulTrackerSubset::new(B::VARIANT),
debug_scope_depth: 0,
};
let mut temp_offsets = Vec::new();
Expand Down
19 changes: 6 additions & 13 deletions wgpu-core/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use crate::{
id,
memory_init_tracker::MemoryInitTrackerAction,
resource::{Buffer, Texture},
track::TrackerSet,
track::{BufferState, ResourceTracker, TextureState, TrackerSet},
Label, PrivateFeatures, Stored,
};

Expand Down Expand Up @@ -91,33 +91,26 @@ impl<B: GfxBackend> CommandBuffer<B> {
pub(crate) fn insert_barriers(
raw: &mut B::CommandBuffer,
base: &mut TrackerSet,
head: &TrackerSet,
head_buffers: &ResourceTracker<BufferState>,
head_textures: &ResourceTracker<TextureState>,
buffer_guard: &Storage<Buffer<B>, id::BufferId>,
texture_guard: &Storage<Texture<B>, id::TextureId>,
) {
use hal::command::CommandBuffer as _;

profiling::scope!("insert_barriers");
debug_assert_eq!(B::VARIANT, base.backend());
debug_assert_eq!(B::VARIANT, head.backend());

let buffer_barriers = base.buffers.merge_replace(&head.buffers).map(|pending| {
let buffer_barriers = base.buffers.merge_replace(head_buffers).map(|pending| {
let buf = &buffer_guard[pending.id];
pending.into_hal(buf)
});
let texture_barriers = base.textures.merge_replace(&head.textures).map(|pending| {
let texture_barriers = base.textures.merge_replace(head_textures).map(|pending| {
let tex = &texture_guard[pending.id];
pending.into_hal(tex)
});
base.views.merge_extend(&head.views).unwrap();
base.bind_groups.merge_extend(&head.bind_groups).unwrap();
base.samplers.merge_extend(&head.samplers).unwrap();
base.compute_pipes
.merge_extend(&head.compute_pipes)
.unwrap();
base.render_pipes.merge_extend(&head.render_pipes).unwrap();
base.bundles.merge_extend(&head.bundles).unwrap();

//TODO: be more deliberate about the stages
let stages = all_buffer_stages() | all_image_stages();
unsafe {
raw.pipeline_barrier(
Expand Down
36 changes: 21 additions & 15 deletions wgpu-core/src/command/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
memory_init_tracker::{MemoryInitKind, MemoryInitTrackerAction},
pipeline::PipelineFlags,
resource::{BufferUse, Texture, TextureUse, TextureView, TextureViewInner},
track::{TextureSelector, TrackerSet, UsageConflict},
track::{StatefulTrackerSubset, TextureSelector, UsageConflict},
validation::{
check_buffer_usage, check_texture_usage, MissingBufferUsageError, MissingTextureUsageError,
},
Expand Down Expand Up @@ -505,7 +505,7 @@ struct RenderAttachment<'a> {

struct RenderPassInfo<'a, B: hal::Backend> {
context: RenderPassContext,
trackers: TrackerSet,
trackers: StatefulTrackerSubset,
render_attachments: AttachmentDataVec<RenderAttachment<'a>>,
used_swap_chain: Option<Stored<id::SwapChainId>>,
is_ds_read_only: bool,
Expand All @@ -518,7 +518,7 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
raw: &mut B::CommandBuffer,
color_attachments: &[RenderPassColorAttachment],
depth_stencil_attachment: Option<&RenderPassDepthStencilAttachment>,
cmd_buf: &CommandBuffer<B>,
cmd_buf: &mut CommandBuffer<B>,
device: &Device<B>,
view_guard: &'a Storage<TextureView<B>, id::TextureViewId>,
) -> Result<Self, RenderPassErrorInner> {
Expand All @@ -537,7 +537,6 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
let mut sample_count = 0;
let mut depth_stencil_aspects = hal::format::Aspects::empty();
let mut used_swap_chain = None::<Stored<id::SwapChainId>>;
let mut trackers = TrackerSet::new(B::VARIANT);

let mut add_view = |view: &TextureView<B>, type_name| {
if let Some(ex) = extent {
Expand Down Expand Up @@ -565,7 +564,8 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
let rp_key = {
let depth_stencil = match depth_stencil_attachment {
Some(at) => {
let view = trackers
let view = cmd_buf
.trackers
.views
.use_extend(&*view_guard, at.view, (), ())
.map_err(|_| RenderPassErrorInner::InvalidAttachment(at.view))?;
Expand Down Expand Up @@ -628,7 +628,8 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
let mut resolves = ArrayVec::new();

for at in color_attachments {
let view = trackers
let view = cmd_buf
.trackers
.views
.use_extend(&*view_guard, at.view, (), ())
.map_err(|_| RenderPassErrorInner::InvalidAttachment(at.view))?;
Expand Down Expand Up @@ -691,7 +692,8 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
}

for resolve_target in color_attachments.iter().flat_map(|at| at.resolve_target) {
let view = trackers
let view = cmd_buf
.trackers
.views
.use_extend(&*view_guard, resolve_target, (), ())
.map_err(|_| RenderPassErrorInner::InvalidAttachment(resolve_target))?;
Expand Down Expand Up @@ -962,7 +964,7 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {

Ok(Self {
context,
trackers,
trackers: StatefulTrackerSubset::new(B::VARIANT),
render_attachments,
used_swap_chain,
is_ds_read_only,
Expand All @@ -974,7 +976,8 @@ impl<'a, B: GfxBackend> RenderPassInfo<'a, B> {
fn finish(
mut self,
texture_guard: &Storage<Texture<B>, id::TextureId>,
) -> Result<(TrackerSet, Option<Stored<id::SwapChainId>>), RenderPassErrorInner> {
) -> Result<(StatefulTrackerSubset, Option<Stored<id::SwapChainId>>), RenderPassErrorInner>
{
profiling::scope!("finish", "RenderPassInfo");

for ra in self.render_attachments {
Expand Down Expand Up @@ -1134,7 +1137,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
);
dynamic_offset_count += num_dynamic_offsets as usize;

let bind_group = info
let bind_group = cmd_buf
.trackers
.bind_groups
.use_extend(&*bind_group_guard, bind_group_id, (), ())
Expand All @@ -1144,9 +1147,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.validate_dynamic_bindings(&temp_offsets)
.map_pass_err(scope)?;

// merge the resource tracker in
info.trackers
.merge_extend(&bind_group.used)
.map_pass_err(scope)?;
cmd_buf.trackers.merge_extend_stateless(&bind_group.used);

cmd_buf.buffer_memory_init_actions.extend(
bind_group.used_buffer_ranges.iter().filter_map(|action| {
Expand Down Expand Up @@ -1196,7 +1201,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
continue;
}

let pipeline = info
let pipeline = cmd_buf
.trackers
.render_pipes
.use_extend(&*pipeline_guard, pipeline_id, (), ())
Expand Down Expand Up @@ -1838,7 +1843,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
} => {
let scope = PassErrorScope::WriteTimestamp;

let query_set = info
let query_set = cmd_buf
.trackers
.query_sets
.use_extend(&*query_set_guard, query_set_id, (), ())
Expand All @@ -1865,7 +1870,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
} => {
let scope = PassErrorScope::BeginPipelineStatisticsQuery;

let query_set = info
let query_set = cmd_buf
.trackers
.query_sets
.use_extend(&*query_set_guard, query_set_id, (), ())
Expand Down Expand Up @@ -1899,7 +1904,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
RenderCommand::ExecuteBundle(bundle_id) => {
let scope = PassErrorScope::ExecuteBundle;
let bundle = info
let bundle = cmd_buf
.trackers
.bundles
.use_extend(&*bundle_guard, bundle_id, (), ())
Expand Down Expand Up @@ -1987,7 +1992,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
super::CommandBuffer::insert_barriers(
last_cmd_buf,
&mut cmd_buf.trackers,
&trackers,
&trackers.buffers,
&trackers.textures,
&*buffer_guard,
&*texture_guard,
);
Expand Down
4 changes: 3 additions & 1 deletion wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -735,10 +735,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.begin_primary(hal::command::CommandBufferFlags::ONE_TIME_SUBMIT);
}
log::trace!("Stitching command buffer {:?} before submission", cmb_id);
trackers.merge_extend_stateless(&cmdbuf.trackers);
CommandBuffer::insert_barriers(
&mut transit,
&mut *trackers,
&cmdbuf.trackers,
&cmdbuf.trackers.buffers,
&cmdbuf.trackers.textures,
&*buffer_guard,
&*texture_guard,
);
Expand Down
41 changes: 38 additions & 3 deletions wgpu-core/src/track/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,10 +625,17 @@ impl TrackerSet {
}

/// Merge all the trackers of another instance by extending
/// the usage. Panics on a conflict.
pub fn merge_extend(&mut self, other: &Self) -> Result<(), UsageConflict> {
/// the usage. Panics on a stateless conflict, returns a conflict otherwise.
pub fn merge_extend_all(&mut self, other: &Self) -> Result<(), UsageConflict> {
self.buffers.merge_extend(&other.buffers)?;
self.textures.merge_extend(&other.textures)?;
self.merge_extend_stateless(other);
Ok(())
}

/// Merge all the stateless trackers of another instance by extending
/// the usage. Panics on a conflict.
pub fn merge_extend_stateless(&mut self, other: &Self) {
self.views.merge_extend(&other.views).unwrap();
self.bind_groups.merge_extend(&other.bind_groups).unwrap();
self.samplers.merge_extend(&other.samplers).unwrap();
Expand All @@ -638,10 +645,38 @@ impl TrackerSet {
self.render_pipes.merge_extend(&other.render_pipes).unwrap();
self.bundles.merge_extend(&other.bundles).unwrap();
self.query_sets.merge_extend(&other.query_sets).unwrap();
Ok(())
}

pub fn backend(&self) -> wgt::Backend {
self.buffers.backend
}
}

#[derive(Debug)]
pub(crate) struct StatefulTrackerSubset {
pub buffers: ResourceTracker<BufferState>,
pub textures: ResourceTracker<TextureState>,
}

impl StatefulTrackerSubset {
/// Create an empty set.
pub fn new(backend: wgt::Backend) -> Self {
Self {
buffers: ResourceTracker::new(backend),
textures: ResourceTracker::new(backend),
}
}

/// Clear all the trackers.
pub fn clear(&mut self) {
self.buffers.clear();
self.textures.clear();
}

/// Merge all the trackers of another tracker the usage.
pub fn merge_extend(&mut self, other: &TrackerSet) -> Result<(), UsageConflict> {
self.buffers.merge_extend(&other.buffers)?;
self.textures.merge_extend(&other.textures)?;
Ok(())
}
}

0 comments on commit 9cd0163

Please sign in to comment.