From 648f3ce715d7ca28ff978ada7cfafe2ad87b331a Mon Sep 17 00:00:00 2001 From: marc0246 <40955683+marc0246@users.noreply.github.com> Date: Wed, 20 Dec 2023 11:50:57 +0100 Subject: [PATCH] Improve clarity of `MemoryTypeFilter` docs (#2428) * Make the combination disclaimers **impossible** to miss * Add headings to the examples for improved visibility * Update vulkano/src/memory/allocator/mod.rs Co-authored-by: Rua --------- Co-authored-by: Rua --- vulkano/src/memory/allocator/mod.rs | 60 ++++++++++++++++++----------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/vulkano/src/memory/allocator/mod.rs b/vulkano/src/memory/allocator/mod.rs index 905cf4e3..c7aa9301 100644 --- a/vulkano/src/memory/allocator/mod.rs +++ b/vulkano/src/memory/allocator/mod.rs @@ -367,6 +367,8 @@ impl Debug for dyn MemoryAllocator { /// /// # Examples /// +/// #### Direct device access only +/// /// For resources that the device frequently accesses, e.g. textures, render targets, or /// intermediary buffers, you want device-local memory without any host access: /// @@ -396,8 +398,10 @@ impl Debug for dyn MemoryAllocator { /// .unwrap(); /// ``` /// +/// #### Sequential writes from host, indirect device access +/// /// For staging, the resource is only ever written to sequentially. Also, since the device will -/// only read the staging resourse once, it would yield no benefit to place it in device-local +/// only read the staging resource once, it would yield no benefit to place it in device-local /// memory, in fact it would be wasteful. Therefore, it's best to put it in host-local memory: /// /// ``` @@ -425,6 +429,8 @@ impl Debug for dyn MemoryAllocator { /// # let staging_buffer: vulkano::buffer::Subbuffer = staging_buffer; /// ``` /// +/// #### Sequential writes from host, direct device access +/// /// For resources that the device accesses directly, aka a buffer/image used for anything other /// than transfers, it's probably best to put it in device-local memory: /// @@ -453,6 +459,8 @@ impl Debug for dyn MemoryAllocator { /// # let uniform_buffer: vulkano::buffer::Subbuffer = uniform_buffer; /// ``` /// +/// #### Readback to host +/// /// For readback, e.g. getting the results of a compute shader back to the host: /// /// ``` @@ -487,7 +495,9 @@ pub struct MemoryTypeFilter { } impl MemoryTypeFilter { - /// Prefers picking a memory type with the [`DEVICE_LOCAL`] flag. + /// Prefers picking a memory type with the [`DEVICE_LOCAL`] flag. **It does not affect whether + /// the memory is host-visible**. You need to combine this with either the + /// [`HOST_SEQUENTIAL_WRITE`] or [`HOST_RANDOM_ACCESS`] filter for that. /// /// Memory being device-local means that it is fastest to access for the device. However, /// for dedicated GPUs, getting memory in and out of VRAM requires to go through the PCIe bus, @@ -514,22 +524,22 @@ impl MemoryTypeFilter { /// memory types that are not `HOST_VISIBLE`, which makes sense since it's all system RAM. In /// that case you wouldn't need to do staging. /// - /// Don't use this together with [`PREFER_HOST`], that makes no sense. If you need host access, - /// make sure you combine this with either the [`HOST_SEQUENTIAL_WRITE`] or - /// [`HOST_RANDOM_ACCESS`] filter. + /// Don't use this together with [`PREFER_HOST`], that makes no sense. /// /// [`DEVICE_LOCAL`]: MemoryPropertyFlags::DEVICE_LOCAL - /// [`HOST_VISIBLE`]: MemoryPropertyFlags::HOST_VISIBLE - /// [`PREFER_HOST`]: Self::PREFER_HOST /// [`HOST_SEQUENTIAL_WRITE`]: Self::HOST_SEQUENTIAL_WRITE /// [`HOST_RANDOM_ACCESS`]: Self::HOST_RANDOM_ACCESS + /// [`HOST_VISIBLE`]: MemoryPropertyFlags::HOST_VISIBLE + /// [`PREFER_HOST`]: Self::PREFER_HOST pub const PREFER_DEVICE: Self = Self { required_flags: MemoryPropertyFlags::empty(), preferred_flags: MemoryPropertyFlags::DEVICE_LOCAL, not_preferred_flags: MemoryPropertyFlags::empty(), }; - /// Prefers picking a memory type without the [`DEVICE_LOCAL`] flag. + /// Prefers picking a memory type without the [`DEVICE_LOCAL`] flag. **It does not affect + /// whether the memory is host-visible**. You need to combine this with either the + /// [`HOST_SEQUENTIAL_WRITE`] or [`HOST_RANDOM_ACCESS`] filter for that. /// /// This option is best suited for resources that the host does access, but device doesn't /// access directly, such as staging buffers and readback buffers. @@ -544,28 +554,29 @@ impl MemoryTypeFilter { /// still prefer host-local memory if the memory is rarely used, such as for manually paging /// parts of device-local memory out in order to free up space on the device. /// - /// Don't use this together with [`PREFER_DEVICE`], that makes no sense. If you need host - /// access, make sure you combine this with either the [`HOST_SEQUENTIAL_WRITE`] or - /// [`HOST_RANDOM_ACCESS`] filter. + /// Don't use this together with [`PREFER_DEVICE`], that makes no sense. /// /// [`DEVICE_LOCAL`]: MemoryPropertyFlags::DEVICE_LOCAL - /// [`PREFER_DEVICE`]: Self::PREFER_DEVICE /// [`HOST_SEQUENTIAL_WRITE`]: Self::HOST_SEQUENTIAL_WRITE /// [`HOST_RANDOM_ACCESS`]: Self::HOST_RANDOM_ACCESS + /// [`PREFER_DEVICE`]: Self::PREFER_DEVICE pub const PREFER_HOST: Self = Self { required_flags: MemoryPropertyFlags::empty(), preferred_flags: MemoryPropertyFlags::empty(), not_preferred_flags: MemoryPropertyFlags::DEVICE_LOCAL, }; - /// This guarantees picking a memory type that has the [`HOST_VISIBLE`] flag. Using this filter - /// allows the allocator to pick a memory type that is uncached and write-combined, which is - /// ideal for sequential writes. However, this optimization might lead to poor performance for - /// anything else. What counts as a sequential write is any kind of loop that writes memory - /// locations in order, such as iterating over a slice while writing each element in order, or - /// equivalently using [`slice::copy_from_slice`]. Copying sized data also counts, as rustc - /// should write the memory locations in order. If you have a struct, make sure you write it - /// member-by-member. + /// This guarantees picking a memory type that has the [`HOST_VISIBLE`] flag. **It does not + /// affect whether the memory is device-local or host-local**. You need to combine this with + /// either the [`PREFER_DEVICE`] or [`PREFER_HOST`] filter for that. + /// + /// Using this filter allows the allocator to pick a memory type that is uncached and + /// write-combined, which is ideal for sequential writes. However, this optimization might lead + /// to poor performance for anything else. What counts as a sequential write is any kind of + /// loop that writes memory locations in order, such as iterating over a slice while writing + /// each element in order, or equivalently using [`slice::copy_from_slice`]. Copying sized data + /// also counts, as rustc should write the memory locations in order. If you have a struct, + /// make sure you write it member-by-member. /// /// Example use cases include staging buffers, as well as any other kind of buffer that you /// only write to from the host, like a uniform or vertex buffer. @@ -576,7 +587,8 @@ impl MemoryTypeFilter { /// to get the most performance out, if that's possible. /// /// [`HOST_VISIBLE`]: MemoryPropertyFlags::HOST_VISIBLE - /// [`HOST_COHERENT`]: MemoryPropertyFlags::HOST_COHERENT + /// [`PREFER_DEVICE`]: Self::PREFER_DEVICE + /// [`PREFER_HOST`]: Self::PREFER_HOST /// [`HOST_RANDOM_ACCESS`]: Self::HOST_RANDOM_ACCESS pub const HOST_SEQUENTIAL_WRITE: Self = Self { required_flags: MemoryPropertyFlags::HOST_VISIBLE, @@ -585,7 +597,9 @@ impl MemoryTypeFilter { }; /// This guarantees picking a memory type that has the [`HOST_VISIBLE`] and [`HOST_CACHED`] - /// flags, which is best suited for readback and/or random access. + /// flags, which is best suited for readback and/or random access. **It does not affect whether + /// the memory is device-local or host-local**. You need to combine this with either the + /// [`PREFER_DEVICE`] or [`PREFER_HOST`] filter for that. /// /// Example use cases include using the device for things other than rendering and getting the /// results back to the host. That might be compute shading, or image or video manipulation, or @@ -597,6 +611,8 @@ impl MemoryTypeFilter { /// /// [`HOST_VISIBLE`]: MemoryPropertyFlags::HOST_VISIBLE /// [`HOST_CACHED`]: MemoryPropertyFlags::HOST_CACHED + /// [`PREFER_DEVICE`]: Self::PREFER_DEVICE + /// [`PREFER_HOST`]: Self::PREFER_HOST /// [`HOST_SEQUENTIAL_WRITE`]: Self::HOST_SEQUENTIAL_WRITE pub const HOST_RANDOM_ACCESS: Self = Self { required_flags: MemoryPropertyFlags::HOST_VISIBLE.union(MemoryPropertyFlags::HOST_CACHED),