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.

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

VMASharp API Review #595

Open
wants to merge 5 commits into
base: feature/vmasharp
Choose a base branch
from
Open
Changes from all commits
Commits
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
305 changes: 305 additions & 0 deletions src/Vulkan/VMASharp_API_For_Review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,305 @@
```cs
namespace VMASharp.Metadata
{
public interface IBlockMetadata
{
long Size { get; }
int AllocationCount { get; }
long SumFreeSize { get; }
long UnusedRangeSizeMax { get; }
bool IsEmpty { get; }
void Validate();
Copy link
Contributor

Choose a reason for hiding this comment

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

i guess since it is void this throws if it is invalid ?

Choose a reason for hiding this comment

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

Yes, that is the design choice, but its only called in Debug Builds

void CalcAllocationStatInfo(out StatInfo outInfo);
void AddPoolStats(ref PoolStats stats);
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this method do to the stats ?

Choose a reason for hiding this comment

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

Something like this.

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);
}
}
namespace VMASharp
{
public abstract class Allocation : IDisposable
{
public long Size { get; }
public int MemoryTypeIndex { get; }
public abstract DeviceMemory DeviceMemory { get; }
public abstract long Offset { get; internal set; }
public object? UserData { get; set; }
public abstract nint MappedData { get; }
public void Dispose();
public Result BindBufferMemory(Buffer buffer);
public Result BindBufferMemory(Buffer buffer, long allocationLocalOffset, nint pNext);
public Result BindBufferMemory(Buffer buffer, long allocationLocalOffset, void* pNext = null);
public Result BindImageMemory(Image image);
public Result BindImageMemory(Image image, long allocationLocalOffset, nint pNext);
public Result BindImageMemory(Image image, long allocationLocalOffset, void* pNext = null);
public bool TouchAllocation();
public Result Flush(long offset, long size);
public Result Invalidate(long offset, long size);
public abstract nint Map();
public abstract void Unmap();
public bool TryGetMemory<T>(out Memory<T> memory) where T : unmanaged;
public bool TryGetSpan<T>(out Span<T> span) where T : unmanaged;
}
public struct AllocationBudget
{
public long BlockBytes;
public long AllocationBytes;
public long Usage;
public long Budget;
public AllocationBudget(long blockBytes, long allocationBytes, long usage, long budget);
}
public struct AllocationContext
{
public int CurrentFrame;
public int FrameInUseCount;
public long BufferImageGranularity;
public long AllocationSize;
public long AllocationAlignment;
public AllocationStrategyFlags Strategy;
public SuballocationType SuballocationType;
public bool CanMakeOtherLost;
public AllocationContext(
int currentFrame,
int framesInUse,
long bufferImageGranularity,
long allocationSize,
long allocationAlignment,
AllocationStrategyFlags strategy,
SuballocationType suballocType,
bool canMakeOtherLost);
}
[Flags]
public enum AllocationCreateFlags
{
DedicatedMemory = 1,
NeverAllocate = 2,
Mapped = 4,
CanBecomeLost = 8,
CanMakeOtherLost = 16, // 0x00000010
UpperAddress = 64, // 0x00000040
DontBind = 128, // 0x00000080
WithinBudget = 256, // 0x00000100
}
public struct AllocationCreateInfo
{
public AllocationCreateFlags Flags;
public AllocationStrategyFlags Strategy;
public MemoryUsage Usage;
public MemoryPropertyFlags? RequiredFlags;
public MemoryPropertyFlags? PreferredFlags;
public uint MemoryTypeBits;
public VulkanMemoryPool? Pool;
public object? UserData;
public AllocationCreateInfo(
AllocationCreateFlags flags = (AllocationCreateFlags) 0,
AllocationStrategyFlags strategy = (AllocationStrategyFlags) 0,
MemoryUsage usage = MemoryUsage.Unknown,
MemoryPropertyFlags? requiredFlags = 0,
MemoryPropertyFlags? preferredFlags = 0,
uint memoryTypeBits = 0,
VulkanMemoryPool? pool = null,
object? userData = null);
}
public class AllocationException : VulkanResultException
{
public AllocationException(string message);
public AllocationException(string message, Exception? innerException);
public AllocationException(Result res);
public AllocationException(string message, Result res);
}
public struct AllocationPoolCreateInfo
{
public int MemoryTypeIndex;
public PoolCreateFlags Flags;
public long BlockSize;
public int MinBlockCount;
public int MaxBlockCount;
public int FrameInUseCount;
public Func<long, IBlockMetadata>? AllocationAlgorithmCreate;
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this function do ?

Choose a reason for hiding this comment

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

Allows the user to specify what algorithm they want to use for allocating out of each memory block in the pool.

public AllocationPoolCreateInfo(
int memoryTypeIndex,
PoolCreateFlags flags = (PoolCreateFlags) 0,
long blockSize = 0,
int minBlockCount = 0,
int maxBlockCount = 0,
int frameInUseCount = 0,
Func<long, IBlockMetadata>? allocationAlgorithemCreate = null);
}
public struct AllocationRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of the AllocationRequest compared to a normal allocation, better asked why would i care for the request and not just use an allocation via AllocateMemory?

Choose a reason for hiding this comment

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

VMA has this weird internal multi-stage allocation mechanism to support a few key features. This stores state for that.

{
public const long LostAllocationCost = 1048576;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this magic number for ? Is it a limit or a threshold ?

public long Offset;
public long SumFreeSize;
public long SumItemSize;
public long ItemsToMakeLostCount;
public object Item;
public object CustomData;
public AllocationRequestType Type;
public readonly long CalcCost();
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the cost will be used internally in the allocator itself to decide something internal ?

Choose a reason for hiding this comment

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

Yes

}
public enum AllocationRequestType
{
Normal,
UpperAddress
}
[Flags]
public enum AllocationStrategyFlags
{
BestFit = 1,
WorstFit = 2,
FirstFit = 4,
MinMemory = BestFit, // 0x00000001
MinTime = FirstFit, // 0x00000004
MinFragmentation = WorstFit, // 0x00000002
}
[Flags]
public enum AllocatorCreateFlags
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this flag is used in the VulkanMemoryAllocatorCreateInfo ?

Choose a reason for hiding this comment

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

Yup

{
ExternallySyncronized = 1,
ExtMemoryBudget = 8,
AMDDeviceCoherentMemory = 16, // 0x00000010
Copy link
Contributor

Choose a reason for hiding this comment

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

Whats so special for AMD ?

Copy link
Member

Choose a reason for hiding this comment

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

That's just what the Vulkan extension is named. AMD (and some other vendors) expose a way to check whether the driver thinks it'd be better for performance if a buffer/image would get a dedicated allocation, instead of bing sub-allocated

BufferDeviceAddress = 32, // 0x00000020
}
public sealed class BlockAllocation : Allocation
{
public override DeviceMemory DeviceMemory { get; }
public override long Offset { get; internal set; }
public override nint MappedData { get; }
public override nint Map();
public override void Unmap();
}
public class MapMemoryException : VulkanResultException
{
public MapMemoryException(string message);
public MapMemoryException(Result res);
public MapMemoryException(string message, Result res);
}
public enum MemoryUsage
Copy link
Contributor

Choose a reason for hiding this comment

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

So i guess this is a combination of BufferUsageFlags, SharingMode and MemoryPropertyFlags ?

Choose a reason for hiding this comment

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

These are presets for MemoryPropertyFlags, essentially.

{
Unknown,
GPU_Only,
CPU_Only,
CPU_To_GPU,
GPU_To_CPU,
CPU_Copy,
GPU_LazilyAllocated,
}
[Flags]
public enum PoolCreateFlags
{
IgnoreBufferImageGranularity = 1,
LinearAlgorithm = 16, // 0x00000010
BuddyAlgorithm = 32, // 0x00000020
}
public struct PoolStats
{
public long Size;
public long UnusedSize;
public int AllocationCount;
public int UnusedRangeCount;
public long UnusedRangeSizeMax;
public int BlockCount;
}
public struct StatInfo
{
public int BlockCount;
public int AllocationCount;
public int UnusedRangeCount;
public long UsedBytes;
public long UnusedBytes;
public long AllocationSizeMin;
public long AllocationSizeAvg;
public long AllocationSizeMax;
public long UnusedRangeSizeMin;
public long UnusedRangeSizeAvg;
public long UnusedRangeSizeMax;
}
public class Stats
{
public readonly StatInfo[] MemoryType;
public readonly StatInfo[] MemoryHeap;
public StatInfo Total;
}
public enum SuballocationType
{
Free,
Unknown,
Buffer,
Image_Unknown,
Image_Linear,
Image_Optimal,
}
public class ValidationFailedException : ApplicationException
{
}
public sealed class VulkanMemoryAllocator : IDisposable
{
public Device Device { get; }
public VulkanMemoryAllocator(in VulkanMemoryAllocatorCreateInfo createInfo);
Copy link
Contributor

Choose a reason for hiding this comment

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

Definition of VulkanMemoryAllocatorCreateInfo is missing. Is this intentional (AllocationCreateInfo is listed)?

Copy link

@sunkin351 sunkin351 Oct 3, 2021

Choose a reason for hiding this comment

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

This could honestly be replaced with { get; init; } properties with a few required constructor arguments. I wrote this before those were a thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Depends on whether we want to support NS20 with this library or not - init doesn't work there.

Choose a reason for hiding this comment

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

Weren't you planning on being .NET 6 only?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's 3.0. VMASharp isn't triaged against any release at the moment. It could reasonably be a new feature in 2.X sustaining so the question would be do we want to support this package everywhere Silk.NET.Vulkan is supported today?

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends on whether we want to support NS20 with this library or not - init doesn't work there.

I think for clarity for users, we should support NS20, and in 3.0 look into rewriting parts to use the newer features of .NET 6(+)

public int CurrentFrameIndex { get; set; }
public void Dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the allocator dispose all memory pools too ?

Choose a reason for hiding this comment

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

I believe the current implementation throws if there's memory still allocated when you try to dispose it. But we can do whatever here as we are moving away from VMA altogether.

public ref readonly Silk.NET.Vulkan.PhysicalDeviceProperties PhysicalDeviceProperties { get; }
public ref readonly PhysicalDeviceMemoryProperties MemoryProperties { get; }
Comment on lines +253 to +254
Copy link
Contributor

Choose a reason for hiding this comment

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

should these be exposed ?

Choose a reason for hiding this comment

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

Do you see a reason they shouldn't be?

Copy link
Contributor

Choose a reason for hiding this comment

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

I just though it's more of a implementation detail, that semantically belongs more to the physical device itself (seeing it's also exposed) but i guess there's no harm in putting it out there.

Choose a reason for hiding this comment

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

Its currently setup to be a single device per Allocator. So its not the be all end all of memory allocation for an entire machine.

public MemoryPropertyFlags GetMemoryTypeProperties(int memoryTypeIndex);
public int? FindMemoryTypeIndex(uint memoryTypeBits, in AllocationCreateInfo allocInfo);
public int? FindMemoryTypeIndexForBufferInfo(
in BufferCreateInfo bufferInfo,
in AllocationCreateInfo allocInfo);
public int? FindMemoryTypeIndexForImageInfo(
in ImageCreateInfo imageInfo,
in AllocationCreateInfo allocInfo);
public Allocation AllocateMemory(
in MemoryRequirements requirements,
in AllocationCreateInfo createInfo);
public Allocation AllocateMemoryForBuffer(
Buffer buffer,
in AllocationCreateInfo createInfo,
bool BindToBuffer = false);
public Allocation AllocateMemoryForImage(
Image image,
in AllocationCreateInfo createInfo,
bool BindToImage = false);
public Result CheckCorruption(uint memoryTypeBits);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is corruption detection enabled by default ?

Choose a reason for hiding this comment

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

I haven't implemented it myself, but it was part of VMA, and we can try to provide it as a debug tool type thing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, we could for the time being remove it from the public facing api part and add it later.

public Buffer CreateBuffer(
in BufferCreateInfo bufferInfo,
in AllocationCreateInfo allocInfo,
out Allocation allocation);
public Image CreateImage(
in ImageCreateInfo imageInfo,
in AllocationCreateInfo allocInfo,
out Allocation allocation);
public void FreeMemory(Allocation allocation);
public Stats CalculateStats();
public VulkanMemoryPool CreatePool(in AllocationPoolCreateInfo createInfo);
}
public sealed class VulkanMemoryPool : IDisposable
{
public VulkanMemoryAllocator Allocator { get; }
public string Name { get; set; }
public void Dispose();
public int MakeAllocationsLost();
public Result CheckForCorruption();
public void GetPoolStats(out PoolStats stats);
}
public class VulkanResultException : ApplicationException
{
public readonly Silk.NET.Vulkan.Result? Result;
public VulkanResultException(string message);
public VulkanResultException(string message, Exception? innerException);
public VulkanResultException(Silk.NET.Vulkan.Result res);
public VulkanResultException(string message, Silk.NET.Vulkan.Result res);
}
}
```