-
Notifications
You must be signed in to change notification settings - Fork 543
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
[mtl] Refined passes #2288
[mtl] Refined passes #2288
Conversation
hmm, I detect a performance regression here, investigating the cause now... |
Good news - the regression is solely caused by the extra commit here, so I dropped it. It's an interesting piece that's not obvious to me: so building one gigantic chain of commands and recording them in one go is slower than calling |
@@ -759,12 +759,29 @@ impl<'a> PreRender<'a> { | |||
PreRender::Void => (), | |||
} | |||
} | |||
|
|||
fn issue_many<'b, I>(&mut self, commands: I) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really important for now, but it seems like we should be able to collapse these functions by being generic over the command type
src/backend/metal/src/command.rs
Outdated
{ | ||
{ | ||
let (mut pre, switch) = self.switch_compute(); | ||
assert!(switch); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this assertion guaranteed? It seems valid to have an encoder left open but call {fill|copy}_buffer
, although I'm probably missing something here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! my assumption was wrong here
bors=grovesNL |
bors r=grovesNL |
2288: [mtl] Refined passes r=grovesNL a=kvark Prevents rebinding of all the state and resources in case of consecutive compute dispatches. ~~Also, tries to make a list of commands for graphics pipeline setup and pass it as one big thing (instead of calling `pre.issue` for each individual one). This is piggy-backing on the `issue_many` function of the base commit here.~~ Edit: removed this commit for causing a performance regression. Changes the sink API to be more clear about how the passes are started/ended, it's now the same across compute/render and distinct from blits. PR checklist: - [ ] `make` succeeds (on *nix) - [x] `make reftests` succeeds - [x] tested examples with the following backends: metal - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <[email protected]>
2292: Metal command recording option r=grovesNL a=kvark Based on #2288 Implements first bits of #2236 Allows the client to control the recording option without re-building gfx-rs. Also hides the remote sink behind a feature. We can't CI-test it until SSheldon/rust-dispatch#10 is accepted and published. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <[email protected]>
2292: Metal command recording option r=grovesNL a=kvark Based on #2288 Implements first bits of #2236 Allows the client to control the recording option without re-building gfx-rs. Also hides the remote sink behind a feature. We can't CI-test it until SSheldon/rust-dispatch#10 is accepted and published. PR checklist: - [ ] `make` succeeds (on *nix) - [ ] `make reftests` succeeds - [ ] tested examples with the following backends: - [ ] `rustfmt` run on changed code Co-authored-by: Dzmitry Malyshau <[email protected]>
Prevents rebinding of all the state and resources in case of consecutive compute dispatches.
Also, tries to make a list of commands for graphics pipeline setup and pass it as one big thing (instead of callingEdit: removed this commit for causing a performance regression.pre.issue
for each individual one). This is piggy-backing on theissue_many
function of the base commit here.Changes the sink API to be more clear about how the passes are started/ended, it's now the same across compute/render and distinct from blits.
PR checklist:
make
succeeds (on *nix)make reftests
succeedsrustfmt
run on changed code