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.

Sign up for GitHub

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

Various Metal performance optimizations #2185

Merged
merged 4 commits into from
Jun 28, 2018
Merged

Various Metal performance optimizations #2185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2dc191
Avoid allocation on vertex buffer binding
kvark Jun 27, 2018
a275491
[mtl] cache attachments in clear_image
kvark Jun 27, 2018
d6b8b93
[mtl] avoid heap allocation and an extra autorelease pool at begin_re…
kvark Jun 27, 2018
96871af
[mtl] ban non-fast hash maps entirely
kvark Jun 27, 2018
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
12 changes: 9 additions & 3 deletions src/backend/metal/src/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,11 +821,11 @@ impl CommandSink {
) where
I: Iterator<Item = soft::RenderCommand<&'a soft::Own>>,
{
//assert!(AutoReleasePool::is_active());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is intentional - we need to make sure there is an active autorelease pool set up by the caller here, but I don't know how to code it yet

self.stop_encoding();

match *self {
CommandSink::Immediate { ref cmd_buffer, ref mut encoder_state, .. } => {
let _ap = AutoreleasePool::new();
let encoder = cmd_buffer.new_render_command_encoder(descriptor);
for command in init_commands {
exec_render(encoder, command);
Expand Down Expand Up @@ -1446,7 +1446,7 @@ impl pool::RawCommandPool<Backend> for CommandPool {
framebuffer_inner: native::FramebufferInner {
extent: Extent::default(),
aspects: Aspects::empty(),
colors: Vec::new(),
colors: SmallVec::new(),
depth_stencil: None,
}
},
Expand Down Expand Up @@ -1653,6 +1653,8 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
T: IntoIterator,
T::Item: Borrow<SubresourceRange>,
{
let _ap = AutoreleasePool::new();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fix the leak?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possible! my motivation was to avoid 2 autorelease pools during a regular begin_render_pass, so now I removed the inner one and made sure all the callers set up one.


let CommandBufferInner {
ref mut retained_textures,
ref mut sink,
Expand Down Expand Up @@ -2017,6 +2019,8 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
T: IntoIterator,
T::Item: Borrow<com::ImageBlit>
{
let _ap = AutoreleasePool::new();

let vertices = &mut self.temp.blit_vertices;
vertices.clear();

Expand Down Expand Up @@ -2220,7 +2224,9 @@ impl com::RawCommandBuffer<Backend> for CommandBuffer {
.chain(&extra)
.cloned();

inner.sink().begin_render_pass(false, &descriptor, commands);
inner
.sink()
.begin_render_pass(false, &descriptor, commands);
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/backend/metal/src/device.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use metal::{self,
MTLVertexStepFunction, MTLSamplerBorderColor, MTLSamplerMipFilter, MTLTextureType,
CaptureManager
};
use smallvec::SmallVec;
use spirv_cross::{msl, spirv, ErrorCode as SpirvErrorCode};
use foreign_types::ForeignType;

Expand Down Expand Up @@ -1124,7 +1125,7 @@ impl hal::Device<Backend> for Device {
let mut inner = n::FramebufferInner {
extent,
aspects: format::Aspects::empty(),
colors: Vec::new(),
colors: SmallVec::new(),
depth_stencil: None,
};

Expand Down
3 changes: 2 additions & 1 deletion src/backend/metal/src/native.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use hal::format::{Aspects, Format, FormatDesc};

use cocoa::foundation::{NSUInteger};
use metal;
use smallvec::SmallVec;
use spirv_cross::{msl, spirv};
use foreign_types::ForeignType;

Expand Down Expand Up @@ -51,7 +52,7 @@ pub struct ColorAttachment {
pub struct FramebufferInner {
pub extent: image::Extent,
pub aspects: Aspects,
pub colors: Vec<ColorAttachment>,
pub colors: SmallVec<[ColorAttachment; 4]>,
pub depth_stencil: Option<metal::MTLPixelFormat>,
}

Expand Down