From c43a9c9d50158924a7b33f1cbed11a07076ca4f4 Mon Sep 17 00:00:00 2001 From: marc0246 <40955683+marc0246@users.noreply.github.com> Date: Tue, 12 Sep 2023 17:25:46 +0200 Subject: [PATCH] Document the safety invariants of the (sub)allocator traits (#2321) * Document the safety invariants of the (sub)allocator traits * Oopsie * Remove alignment from the safety requirements Reason being that this requirement is already explained in the docs of `Suballocation`, along with the requirements of the other fields. --- vulkano/src/memory/allocator/mod.rs | 26 ++++++++++++++++++- vulkano/src/memory/allocator/suballocator.rs | 27 +++++++++++++++++--- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/vulkano/src/memory/allocator/mod.rs b/vulkano/src/memory/allocator/mod.rs index f94ac1b3..07dc53c8 100644 --- a/vulkano/src/memory/allocator/mod.rs +++ b/vulkano/src/memory/allocator/mod.rs @@ -257,7 +257,31 @@ const G: DeviceSize = 1024 * M; /// /// # Safety /// -/// TODO +/// - `allocate`, `allocate_from_type` and `allocate_dedicated` must return a memory block that is +/// in bounds of its device memory. +/// - `allocate` and `allocate_from_type` must return a memory block that doesn't alias any other +/// currently allocated memory blocks: +/// - Two currently allocated memory blocks must not share any memory locations, meaning that the +/// intersection of the byte ranges of the two memory blocks must be empty. +/// - Two neighboring currently allocated memory blocks must not share any [page] whose size is +/// given by the [buffer-image granularity], unless either both were allocated with +/// [`AllocationType::Linear`] or both were allocated with [`AllocationType::NonLinear`]. +/// - For all [host-visible] memory types that are not [host-coherent], all memory blocks must be +/// aligned to the [non-coherent atom size]. +/// - The size does **not** have to be padded to the alignment. That is, as long the offset is +/// aligned and the memory blocks don't share any memory locations, a memory block is not +/// considered to alias another even if the padded size shares memory locations with another +/// memory block. +/// - A memory block must stay allocated until either `deallocate` is called on it or the allocator +/// is dropped. If the allocator is cloned, it must produce the same allocator, and memory blocks +/// must stay allocated until either `deallocate` is called on the memory block using any of the +/// clones or all of the clones have been dropped. +/// +/// [page]: self#pages +/// [buffer-image granularity]: self#buffer-image-granularity +/// [host-visible]: MemoryPropertyFlags::HOST_VISIBLE +/// [host-coherent]: MemoryPropertyFlags::HOST_COHERENT +/// [non-coherent atom size]: crate::device::Properties::non_coherent_atom_size pub unsafe trait MemoryAllocator: DeviceOwned + Send + Sync + 'static { /// Finds the most suitable memory type index in `memory_type_bits` using the given `filter`. /// Returns [`None`] if the requirements are too strict and no memory type is able to satisfy diff --git a/vulkano/src/memory/allocator/suballocator.rs b/vulkano/src/memory/allocator/suballocator.rs index 4400a3a5..db2e591f 100644 --- a/vulkano/src/memory/allocator/suballocator.rs +++ b/vulkano/src/memory/allocator/suballocator.rs @@ -50,7 +50,7 @@ use std::{ /// trade-offs and are best suited to specific tasks. To account for all possible use-cases, /// Vulkano offers the ability to create *memory hierarchies*. We refer to the `DeviceMemory` as /// the root of any such hierarchy, even though technically the driver has levels that are further -/// up, because those `DeviceMemory` blocks need to be allocated from physical memory [pages] +/// up, because those `DeviceMemory` blocks need to be allocated from physical memory pages /// themselves, but since those levels are not accessible to us we don't need to consider them. You /// can create any number of levels/branches from there, bounded only by the amount of available /// memory within a `DeviceMemory` block. You can suballocate the root into regions, which are then @@ -60,12 +60,31 @@ use std::{ /// /// TODO /// -/// # Implementing the trait +/// # Safety /// -/// TODO +/// First consider using the provided implementations as there should be no reason to implement +/// this trait, but if you **must**: +/// +/// - `allocate` must return a memory block that is in bounds of the region. +/// - `allocate` must return a memory block that doesn't alias any other currently allocated +/// memory blocks: +/// - Two currently allocated memory blocks must not share any memory locations, meaning that the +/// intersection of the byte ranges of the two memory blocks must be empty. +/// - Two neighboring currently allocated memory blocks must not share any [page] whose size is +/// given by the [buffer-image granularity], unless either both were allocated with +/// [`AllocationType::Linear`] or both were allocated with [`AllocationType::NonLinear`]. +/// - The size does **not** have to be padded to the alignment. That is, as long the offset is +/// aligned and the memory blocks don't share any memory locations, a memory block is not +/// considered to alias another even if the padded size shares memory locations with another +/// memory block. +/// - A memory block must stay allocated until either `deallocate` is called on it or the allocator +/// is dropped. If the allocator is cloned, it must produce the same allocator, and memory blocks +/// must stay allocated until either `deallocate` is called on the memory block using any of the +/// clones or all of the clones have been dropped. /// /// [`DeviceMemory`]: crate::memory::DeviceMemory -/// [pages]: super#pages +/// [page]: super#pages +/// [buffer-image granularity]: super#buffer-image-granularity pub unsafe trait Suballocator { /// Whether the allocator needs [`cleanup`] to be called before memory can be released. ///