YaLTeR / niri

A scrollable-tiling Wayland compositor.
https://matrix.to/#/#niri:matrix.org
GNU General Public License v3.0
4.12k stars 123 forks source link

ShaderRenderElement use borrowed Uniforms to minimize copy #756

Closed gmorer closed 4 weeks ago

gmorer commented 4 weeks ago

Hello, i was checking the mem allocations. i noticed on using niri less than a minute a suprisingly high number caused by a clone in, the highest count of allocation around 70k (total, not size): https://github.com/YaLTeR/niri/blob/289ae3604d705cebc82cbcd23ee4534ded16d3af/src/layout/focus_ring.rs#L239

It clone a BorderRenderElement which clone a ShaderRenderElement which clone Vec<Uniform<'static>>. The Uniform struct contain a name: Cow<'a, str>.

All the name given to theses Uniform are &static str example: https://github.com/YaLTeR/niri/blob/289ae3604d705cebc82cbcd23ee4534ded16d3af/src/render_helpers/resize.rs#L92-L103

So these cow should be Borrowed a not Owned.

Unsure why there is specific code to owned it, but making it a Cow::Owned will trigger a reallocation of the str.

Stack trace of the allocations:

#39 [niri] niri::layout::tile::Tile<W>::render [tile.rs:851]
#40 [niri] niri::layout::tile::Tile<W>::render_inner [tile.rs:815]
#41 [niri] core::bool::<impl bool>::then [bool.rs:60]
#42 [niri] niri::layout::tile::Tile<W>::render_inner::{{closure}} [tile.rs:815]
#43 [niri] niri::layout::focus_ring::FocusRing::render [focus_ring.rs:248]
#44 [niri] niri::layout::focus_ring::FocusRing::render::{{closure}} [focus_ring.rs:239]
#45 [niri] <niri::render_helpers::border::BorderRenderElement as core::clone::Clone>::clone [border.rs:25]
#46 [niri] <niri::render_helpers::shader_element::ShaderRenderElement as core::clone::Clone>::clone [shader_element.rs:31]
#47 [niri] <alloc::vec::Vec<T,A> as core::clone::Clone>::clone [mod.rs:2851]
#48 [niri] alloc::slice::<impl [T]>::to_vec_in [slice.rs:477]
#49 [niri] alloc::slice::hack::to_vec [slice.rs:110]
#50 [niri] <T as alloc::slice::hack::ConvertVec>::to_vec [slice.rs:145]
#51 [niri] <smithay::backend::renderer::gles::uniform::Uniform as core::clone::Clone>::clone [uniform.rs:65]
#52 [niri] <alloc::borrow::Cow<B> as core::clone::Clone>::clone [borrow.rs:199]
#53 [niri] alloc::str::<impl alloc::borrow::ToOwned for str>::to_owned [str.rs:210]
#54 [niri] alloc::slice::<impl alloc::borrow::ToOwned for [T]>::to_owned [slice.rs:859]
#55 [niri] alloc::slice::<impl [T]>::to_vec [slice.rs:452]
#56 [niri] alloc::slice::<impl [T]>::to_vec_in [slice.rs:477]
#57 [niri] alloc::slice::hack::to_vec [slice.rs:110]
#58 [niri] <T as alloc::slice::hack::ConvertVec>::to_vec [slice.rs:161]
#59 [niri] alloc::vec::Vec<T,A>::with_capacity_in [mod.rs:698]
#60 [niri] alloc::raw_vec::RawVec<T,A>::with_capacity_in [raw_vec.rs:202]
#61 [niri] alloc::raw_vec::RawVecInner<A>::with_capacity_in [raw_vec.rs:425]
#62 [niri] alloc::raw_vec::RawVecInner<A>::try_allocate_in [raw_vec.rs:478]
#63 [niri] <alloc::alloc::Global as core::alloc::Allocator>::allocate [alloc.rs:241]
#64 [niri] alloc::alloc::Global::alloc_impl [alloc.rs:181]
#65 [niri] alloc::alloc::alloc [alloc.rs:98]

After the changes the allocation does not happen again.

YaLTeR commented 4 weeks ago

Thanks. I haven't been profiling allocations yet.

Unsure why there is specific code to owned it

I took this code from Smithay which needs to handle the generic runtime string case and didn't change it.

YaLTeR commented 4 weeks ago

Need to do something with the Vec too. My idea was that cloning a render element like that should be cheap, but I forgot about this Vec in the shader.