Silk.NET
Silk.NET copied to clipboard
dotnet
VMASharp API Review
This will serve as the place where we'll review the VMASharp API.
The public API has been summarised in a single file (in markdown format so we can add words/discussions/conclusions/action items if we want to), which will be the file for reviewing.
Internal APIs have been excluded.
Ok, so, much of the internal logic is semi-documented in VMA, the native library I ported this from. No, I didn't bother to document a lot of this stuff, at the time it was a passion project. I may have done a lot more work on this if I were actively using Vulkan, but alas, my time had other places it needed to be spent. I'm saying this for your benefit and for public record.
Edit: I'll do active feedback on your review tomorrow, as I'm spent today.
@sunkin351 This was not meant as a critique and i hope you don't take it personally. The comments i did were purely on this document with the help of some parts of the VMA documentation (when i could find the equivalent). I don't claim to be an expert on this topic and tried to understand the connections of the classes purely by the name of them. Ignore my comments if they sound stupid because i do them from a beginners/users point of view and just try to get the discussion started so we hopefully can all have a good memory allocator as part of Silk.
@sunkin351 This was not meant as a critique and i hope you don't take it personally.
Fear not, my fellow developer. I didn't take any of this personally, though I did write that reply while under heavy fatigue, I hope you can forgive me. My only desire is to help turn this into a usable tool that every developer can enjoy.
Also, am I reading some of this right in that there are data structure definitions missing?
I haven't included anything internal, should just be the public (and hopefully protected) APIs in this document.
As well as defragmentation which is being excluded altogether for the time being.
VulkanMemoryAllocatorCreateInfo struct would be nice to have in the document so we can discuss which stuff is actually needed to create one.
well i tried to create some kind of basic overview diagram to highlight the public relations of the classes (plantuml source in the spoiler at the end). So basically it all boils down to the Allocation class which owns a part of the memory. But i am not sure how the IBlockMetadata plays a role in all this (except that it's part of the multi-stage request). Also how the VulkanMemoryPool relates to the allocation (apart from the AllocationCreateInfo where you can specify a Pool) is the Pool after creation not relevant anymore ?
plantuml source
@startuml
interface IBlockMetadata { long Size { get; } int AllocationCount { get; } long SumFreeSize { get; } long UnusedRangeSizeMax { get; } bool IsEmpty { get; } void Validate(); void CalcAllocationStatInfo(out StatInfo outInfo); void AddPoolStats(ref PoolStats stats); bool TryCreateAllocationRequest(in AllocationContext context, out AllocationRequest request); bool MakeRequestedAllocationsLost( int currentFrame, int frameInUseCount, ref AllocationRequest request); int MakeAllocationsLost(int currentFrame, int frameInUseCount); void CheckCorruption(nuint blockDataPointer); void Alloc( in AllocationRequest request,SuballocationType type, long allocSize, BlockAllocation allocation); void Free(BlockAllocation allocation); void FreeAtOffset(long offset); } class Allocation << (A, grey) >> { long Size { get; } int MemoryTypeIndex { get; } abstract DeviceMemory DeviceMemory { get; } abstract long Offset { get; internal set; } object? UserData { get; set; } abstract IntPtr MappedData { get; } void Dispose(); Result BindBufferMemory(Buffer buffer); Result BindBufferMemory(Buffer buffer, long allocationLocalOffset, IntPtr pNext); Result BindBufferMemory(Buffer buffer, long allocationLocalOffset, void* pNext = null); Result BindImageMemory(Image image); Result BindImageMemory(Image image, long allocationLocalOffset, IntPtr pNext); Result BindImageMemory(Image image, long allocationLocalOffset, void* pNext = null); bool TouchAllocation(); Result Flush(long offset, long size); Result Invalidate(long offset, long size); abstract IntPtr Map(); abstract void Unmap(); bool TryGetMemory<T>(out Memory<T> memory) where T : unmanaged; bool TryGetSpan<T>(out Span<T> span) where T : unmanaged; }
class AllocationBudget << (S, gold) >> { long BlockBytes; long AllocationBytes; long Usage; long Budget; }
class AllocationContext << (S, gold) >> { int CurrentFrame; int FrameInUseCount; long BufferImageGranularity; long AllocationSize; long AllocationAlignment; AllocationStrategyFlags Strategy; SuballocationType SuballocationType; bool CanMakeOtherLost; } class AllocationCreateInfo << (S, gold) >> { AllocationCreateFlags Flags; AllocationStrategyFlags Strategy; MemoryUsage Usage; MemoryPropertyFlags? RequiredFlags; MemoryPropertyFlags? PreferredFlags; uint MemoryTypeBits; VulkanMemoryPool? Pool; object? UserData; } class AllocationPoolCreateInfo << (S, gold) >> { int MemoryTypeIndex; PoolCreateFlags Flags; long BlockSize; int MinBlockCount; int MaxBlockCount; int FrameInUseCount; Func<long, IBlockMetadata>? AllocationAlgorithmCreate; }
class AllocationRequest << (S, gold) >> { long Offset; long SumFreeSize; long SumItemSize; long ItemsToMakeLostCount; object Item; object CustomData; AllocationRequestType Type; readonly long CalcCost(); }
class PoolStats<< (S, gold) >> { long Size; long UnusedSize; int AllocationCount; int UnusedRangeCount; long UnusedRangeSizeMax; int BlockCount; }
class StatInfo<< (S, gold) >> { int BlockCount; int AllocationCount; int UnusedRangeCount; long UsedBytes; long UnusedBytes; long AllocationSizeMin; long AllocationSizeAvg; long AllocationSizeMax; long UnusedRangeSizeMin; long UnusedRangeSizeAvg; long UnusedRangeSizeMax; } class Stats { readonly StatInfo[] MemoryType; readonly StatInfo[] MemoryHeap; StatInfo Total; } class BlockAllocation { }
class VulkanMemoryAllocator { Device Device { get; } VulkanMemoryAllocator(in VulkanMemoryAllocatorCreateInfo createInfo); int CurrentFrameIndex { get; set; } void Dispose(); ref readonly Silk.NET.Vulkan.PhysicalDeviceProperties PhysicalDeviceProperties { get; } ref readonly PhysicalDeviceMemoryProperties MemoryProperties { get; } MemoryPropertyFlags GetMemoryTypeProperties(int memoryTypeIndex); int? FindMemoryTypeIndex(uint memoryTypeBits, in AllocationCreateInfo allocInfo); int? FindMemoryTypeIndexForBufferInfo( in BufferCreateInfo bufferInfo, in AllocationCreateInfo allocInfo); int? FindMemoryTypeIndexForImageInfo( in ImageCreateInfo imageInfo, in AllocationCreateInfo allocInfo); Allocation AllocateMemory( in MemoryRequirements requirements, in AllocationCreateInfo createInfo); Allocation AllocateMemoryForBuffer( Buffer buffer, in AllocationCreateInfo createInfo, bool BindToBuffer = false); Allocation AllocateMemoryForImage( Image image, in AllocationCreateInfo createInfo, bool BindToImage = false); Result CheckCorruption(uint memoryTypeBits); Buffer CreateBuffer( in BufferCreateInfo bufferInfo, in AllocationCreateInfo allocInfo, out Allocation allocation); Image CreateImage( in ImageCreateInfo imageInfo, in AllocationCreateInfo allocInfo, out Allocation allocation); void FreeMemory(Allocation allocation); Stats CalculateStats(); VulkanMemoryPool CreatePool(in AllocationPoolCreateInfo createInfo); }
class VulkanMemoryPool { VulkanMemoryAllocator Allocator { get; } string Name { get; set; } void Dispose(); int MakeAllocationsLost(); Result CheckForCorruption(); void GetPoolStats(out PoolStats stats); }
enum SuballocationType {} enum AllocationRequestType{} enum AllocationStrategyFlags {} enum AllocatorCreateFlags {} enum MemoryUsage {} enum AllocationCreateFlags {} enum PoolCreateFlags {}
AllocationRequest <-- AllocationRequestType AllocationPoolCreateInfo <-- PoolCreateFlags
AllocationCreateInfo <-- MemoryUsage AllocationCreateInfo <-- MemoryPropertyFlags AllocationCreateInfo <-- AllocationStrategyFlags AllocationCreateInfo <-- AllocationCreateFlags AllocationCreateInfo --> VulkanMemoryPool : belongs optional to
AllocationContext <-- AllocationStrategyFlags AllocationContext <-- SuballocationType AllocationContext --> IBlockMetadata
Allocation --> Memory : occupies a chunk Allocation --> Buffer : gets bound to Allocation --> Image : gets bound to
VulkanMemoryAllocator <-- AllocationCreateInfo : for Allocations VulkanMemoryAllocator <-- AllocationPoolCreateInfo : for AllocationPool VulkanMemoryAllocator <-- VulkanMemoryAllocatorCreateInfo VulkanMemoryAllocator --> Allocation : create 'VulkanMemoryAllocator --> Buffer : creates 'VulkanMemoryAllocator --> Image : creates VulkanMemoryAllocator --> VulkanMemoryPool : creates
IBlockMetadata --> StatInfo : calculates IBlockMetadata --> PoolStats : calculates IBlockMetadata --> AllocationRequest : creates IBlockMetadata --> BlockAllocation : alloc/free Stats <--StatInfo
BlockAllocation --> Allocation
hide empty members remove enum remove AllocationPoolCreateInfo remove AllocationCreateInfo remove PoolCreateFlags remove AllocationRequestType remove AllocatorCreateFlags remove SuballocationType remove AllocationStrategyFlags remove MemoryUsage remove AllocationCreateFlags remove MemoryPropertyFlags remove VulkanMemoryAllocatorCreateInfo remove Stats remove StatInfo remove PoolStats @enduml
The IBlockMetadata interface is responsible for book keeping for a singular block of Device Memory. It keeps track of what memory in said block is allocated and what memory is free to be allocated. That's its reason for existing.
The
IBlockMetadatainterface is responsible for book keeping for a singular block of Device Memory. It keeps track of what memory in said block is allocated and what memory is free to be allocated. That's its reason for existing.
Ok i some time to look through the source and the interface is basically the extension point of this library via the AllocationPoolCreateInfo struct where you could specify custom pool allocation strategy. So we can't reduce the surface without sacrificing this feature (IBlockMetadata, allocation request/context would then be internal).
Otherwise i think the api is ok. When it gets in before the big 3.0 it can be battle tested and if there are some api changes that would be beneficial but breaking, they could be postponed to 3.0.
It looks like there isn't a lot of movement on this, and 2.X isn't really where the Silk.NET team themselves are focusing at this time.
We (I) still want this in mainline Silk.NET, though, and we own the code now (#599) so we can always revisit this later down the line!
For now @sunkin351 did you want the original VMASharp to join the Silk.NET community program? Hopefully this will make the community more willing to use VMASharp in the interim period.
I'm closing this as I won't be coming back to this, and with all maintainers being dedicated to 3.0 I'm not sure if anyone else would either. I've protected the branch, so it should stay put should anyone else want to pick this up.
Re-opening since there's active work on 2.x again, and a plan for more often API review meetings and more coffee & code streams, once #1253 is in, ill PR to rebase this branch
Assigning @Beyley for API review given we don't really have any Vulkaneers around atm.
Theres quite a few cases of IntPtr being used, which doesnt fall in line with Silk classes like SilkMarshal which use nint, and the Vulkan bindings in general which seem to like nint and void* and have no uses of IntPtr (posted normally so i dont create 6 review comments for one thing)
Bear in mind, this was written in a time before nint and nuint were introduced and likewise has not been updated to reflect the newest standards.
The original design of this API revolved around the ability to free "lost" allocations should they not be accessed in X (user defined) frames. If we don't wanna support that, we might be able to simplify the IBlockMetadata interface with a singular bool TryAllocate(...) method.
Edit: I'm probably forgetting some of the other things the convoluted design allowed...
