1000: Elide redundant set_pipeline calls. r=kvark a=Kimundi

**Description**
This adds an check in each `set_pipeline()` call for wether the same pipeline id has already been set previously. This should cause free performance wins for suboptimal usage of the wgpu API, while having neglible overhead otherwise.

An example scenario for where this would be useful is a game that just blindly sets all render state for each object, but has few actually different objects:

```rust
for game_obj in game_objs {
    rpass.set_pipeline(...);
    rpass.set_bind_group(...);
    rpass.set_vertex_buffer(...);
    rpass.draw(...);
}
```

**Testing**
Ideally we would have tests that check that pipeline changes have been ellided, but I'm not sure how to write them in the current codebase.

**Future work**
- We could extend this kind of redundant state change detection to most `.set_*` methods on `RenderPass`es.
- If we want to guarantee this behavior in the API, we should also document it.

Co-authored-by: Marvin Löbel <loebel.marvin@gmail.com>
This commit is contained in:
bors[bot] 2020-10-22 17:18:24 +00:00 committed by GitHub
commit 6c519e0d92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 46 additions and 18 deletions

View File

@ -38,7 +38,7 @@
#![allow(clippy::reversed_empty_ranges)]
use crate::{
command::{BasePass, DrawError, RenderCommand, RenderCommandError},
command::{BasePass, DrawError, RenderCommand, RenderCommandError, StateChange},
conv,
device::{
AttachmentData, DeviceError, RenderPassContext, MAX_VERTEX_BUFFERS, SHADER_STAGE_COUNT,
@ -515,6 +515,7 @@ struct State {
raw_dynamic_offsets: Vec<wgt::DynamicOffset>,
flat_dynamic_offsets: Vec<wgt::DynamicOffset>,
used_bind_groups: usize,
pipeline: StateChange<id::RenderPipelineId>,
}
impl State {
@ -698,6 +699,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
raw_dynamic_offsets: Vec::new(),
flat_dynamic_offsets: Vec::new(),
used_bind_groups: 0,
pipeline: StateChange::new(),
};
let mut commands = Vec::new();
let mut base = bundle_encoder.base.as_ref();
@ -746,6 +748,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
state.trackers.merge_extend(&bind_group.used)?;
}
RenderCommand::SetPipeline(pipeline_id) => {
if state.pipeline.set_and_check_redundant(pipeline_id) {
continue;
}
let pipeline = state
.trackers
.render_pipes

View File

@ -6,7 +6,7 @@ use crate::{
binding_model::{BindError, BindGroup, PushConstantUploadError},
command::{
bind::{Binder, LayoutChange},
BasePass, BasePassRef, CommandBuffer, CommandEncoderError,
BasePass, BasePassRef, CommandBuffer, CommandEncoderError, StateChange,
},
hub::{GfxBackend, Global, GlobalIdentityHandlerFactory, Storage, Token},
id,
@ -137,16 +137,10 @@ pub enum ComputePassError {
PushConstants(#[from] PushConstantUploadError),
}
#[derive(Debug, PartialEq)]
enum PipelineState {
Required,
Set,
}
#[derive(Debug)]
struct State {
binder: Binder,
pipeline: PipelineState,
pipeline: StateChange<id::ComputePipelineId>,
trackers: TrackerSet,
debug_scope_depth: u32,
}
@ -161,7 +155,7 @@ impl State {
index: bind_mask.trailing_zeros(),
});
}
if self.pipeline == PipelineState::Required {
if self.pipeline.is_unset() {
return Err(DispatchError::MissingPipeline);
}
Ok(())
@ -235,7 +229,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let mut state = State {
binder: Binder::new(cmd_buf.limits.max_bind_groups),
pipeline: PipelineState::Required,
pipeline: StateChange::new(),
trackers: TrackerSet::new(B::VARIANT),
debug_scope_depth: 0,
};
@ -293,7 +287,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
}
ComputeCommand::SetPipeline(pipeline_id) => {
state.pipeline = PipelineState::Set;
if state.pipeline.set_and_check_redundant(pipeline_id) {
continue;
}
let pipeline = cmd_buf
.trackers
.compute_pipes

View File

@ -270,3 +270,25 @@ where
count_words += size_to_write_words;
}
}
#[derive(Debug)]
struct StateChange<T> {
last_state: Option<T>,
}
impl<T: Copy + PartialEq> StateChange<T> {
fn new() -> Self {
Self { last_state: None }
}
fn set_and_check_redundant(&mut self, new_state: T) -> bool {
let already_set = self.last_state == Some(new_state);
self.last_state = Some(new_state);
already_set
}
fn is_unset(&self) -> bool {
self.last_state.is_none()
}
fn reset(&mut self) {
self.last_state = None;
}
}

View File

@ -7,7 +7,7 @@ use crate::{
command::{
bind::{Binder, LayoutChange},
BasePass, BasePassRef, CommandBuffer, CommandEncoderError, DrawError, ExecutionError,
RenderCommand, RenderCommandError,
RenderCommand, RenderCommandError, StateChange,
},
conv,
device::{
@ -273,7 +273,7 @@ struct State {
binder: Binder,
blend_color: OptionalState,
stencil_reference: u32,
pipeline: OptionalState,
pipeline: StateChange<id::RenderPipelineId>,
index: IndexState,
vertex: VertexState,
debug_scope_depth: u32,
@ -289,7 +289,7 @@ impl State {
index: bind_mask.trailing_zeros(),
});
}
if self.pipeline == OptionalState::Required {
if self.pipeline.is_unset() {
return Err(DrawError::MissingPipeline);
}
if self.blend_color == OptionalState::Required {
@ -301,7 +301,7 @@ impl State {
/// Reset the `RenderBundle`-related states.
fn reset_bundle(&mut self) {
self.binder.reset();
self.pipeline = OptionalState::Required;
self.pipeline.reset();
self.index.reset();
self.vertex.reset();
}
@ -945,7 +945,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
binder: Binder::new(cmd_buf.limits.max_bind_groups),
blend_color: OptionalState::Unused,
stencil_reference: 0,
pipeline: OptionalState::Required,
pipeline: StateChange::new(),
index: IndexState::default(),
vertex: VertexState::default(),
debug_scope_depth: 0,
@ -1008,6 +1008,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
};
}
RenderCommand::SetPipeline(pipeline_id) => {
if state.pipeline.set_and_check_redundant(pipeline_id) {
continue;
}
let pipeline = trackers
.render_pipes
.use_extend(&*pipeline_guard, pipeline_id, (), ())
@ -1017,7 +1021,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.check_compatible(&pipeline.pass_context)
.map_err(RenderCommandError::IncompatiblePipeline)?;
state.pipeline = OptionalState::Set;
state.pipeline_flags = pipeline.flags;
if pipeline.flags.contains(PipelineFlags::WRITES_DEPTH_STENCIL)