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

WIP: Proposal to base VkDeviceMemory on MTLHeap always #2309

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
6 changes: 6 additions & 0 deletions MoltenVK/MoltenVK/API/mvk_datatypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,12 @@ MTLCPUCacheMode mvkMTLCPUCacheModeFromVkMemoryPropertyFlags(VkMemoryPropertyFlag
/** Returns the Metal resource option flags corresponding to the Metal storage mode and cache mode. */
MTLResourceOptions mvkMTLResourceOptions(MTLStorageMode mtlStorageMode, MTLCPUCacheMode mtlCPUCacheMode);

/** Resturn the Metal storage mode from the Metal resource options */
MTLStorageMode mvkMTLStorageMode(MTLResourceOptions options);

/** Resturn the Metal cache mode from the Metal resource options */
MTLCPUCacheMode mvkMTLCPUCacheMode(MTLResourceOptions options);

Copy link
Contributor

@billhollings billhollings Aug 15, 2024

Choose a reason for hiding this comment

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

These are for internal use and don't need to be in mvk_datatypes.h. I suggest you move them (along with mvkMTLResourceOptions()) to a support functions section at the bottom of MVKDeviceMemory.h/mm. See other support function areas at the bottom of other files for reference.

The ancient original intention of mvk_datatypes.h was to provide apps access to convenience functions for mapping Vulkan to Metal types. TBH...I'm sure no-one ever uses it, and eventually I'd like to remove it and just use internal non-public files instead.

Also Returns is misspelled in the comment.


#ifdef __cplusplus
}
Expand Down
122 changes: 67 additions & 55 deletions MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,46 +33,45 @@ static const VkExternalMemoryHandleTypeFlagBits VK_EXTERNAL_MEMORY_HANDLE_TYPE_M

#pragma mark MVKDeviceMemory

typedef struct MVKMappedMemoryRange {
VkDeviceSize offset = 0;
VkDeviceSize size = 0;
} MVKMappedMemoryRange;

typedef struct MVKMemoryRange {
VkDeviceSize offset = 0u;
VkDeviceSize size = 0u;
} MVKMemoryRange;

/** Represents a Vulkan device-space memory allocation. */
class MVKDeviceMemory : public MVKVulkanAPIDeviceObject {

public:

/** Returns the Vulkan type of this object. */
VkObjectType getVkObjectType() override { return VK_OBJECT_TYPE_DEVICE_MEMORY; }

/** Returns the debug report object type of this object. */
VkDebugReportObjectTypeEXT getVkDebugReportObjectType() override { return VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_MEMORY_EXT; }

/** Returns whether the memory is accessible from the host. */
inline bool isMemoryHostAccessible() {
inline bool isMemoryHostAccessible() {
MTLStorageMode storageMode = getMTLStorageMode();
#if MVK_APPLE_SILICON
if (_mtlStorageMode == MTLStorageModeMemoryless)
return false;
if (storageMode == MTLStorageModeMemoryless)
return false;
#endif
return (_mtlStorageMode != MTLStorageModePrivate);
}
return (storageMode != MTLStorageModePrivate);
}

/** Returns whether the memory is automatically coherent between device and host. */
inline bool isMemoryHostCoherent() { return (_mtlStorageMode == MTLStorageModeShared); }
inline bool isMemoryHostCoherent() { return getMTLStorageMode() == MTLStorageModeShared; }

/** Returns whether this is a dedicated allocation. */
inline bool isDedicatedAllocation() { return _isDedicated; }
/** Returns whether this is a dedicated allocation. */
inline bool isDedicatedAllocation() { return _dedicatedResourceType != DedicatedResourceType::NONE; }

/** Returns the memory already committed by this instance. */
inline VkDeviceSize getDeviceMemoryCommitment() { return _allocationSize; }
/** Returns the memory already committed by this instance. */
inline VkDeviceSize getDeviceMemoryCommitment() { return _size; }

/**
* Returns the host memory address of this memory, or NULL if the memory has not been
* mapped yet, or is marked as device-only and cannot be mapped to a host address.
*/
inline void* getHostMemoryAddress() { return _pMemory; }
inline void* getHostMemoryAddress() { return _map; }

/**
* Maps the memory address at the specified offset from the start of this memory allocation,
Expand All @@ -83,15 +82,8 @@ class MVKDeviceMemory : public MVKVulkanAPIDeviceObject {
/** Unmaps a previously mapped memory range. */
VkResult unmap(const VkMemoryUnmapInfoKHR* unmapInfo);

/**
* If this device memory is currently mapped to host memory, returns the range within
* this device memory that is currently mapped to host memory, or returns {0,0} if
* this device memory is not currently mapped to host memory.
*/
inline const MVKMappedMemoryRange& getMappedRange() { return _mappedRange; }

/** Returns whether this device memory is currently mapped to host memory. */
bool isMapped() { return _mappedRange.size > 0; }
bool isMapped() { return _map; }

/** If this memory is host-visible, the specified memory range is flushed to the device. */
VkResult flushToDevice(VkDeviceSize offset, VkDeviceSize size);
Expand Down Expand Up @@ -120,13 +112,13 @@ class MVKDeviceMemory : public MVKVulkanAPIDeviceObject {
inline id<MTLHeap> getMTLHeap() { return _mtlHeap; }

/** Returns the Metal storage mode used by this memory allocation. */
inline MTLStorageMode getMTLStorageMode() { return _mtlStorageMode; }
inline MTLStorageMode getMTLStorageMode() { return mvkMTLStorageMode(_options); }

/** Returns the Metal CPU cache mode used by this memory allocation. */
inline MTLCPUCacheMode getMTLCPUCacheMode() { return _mtlCPUCacheMode; }
inline MTLCPUCacheMode getMTLCPUCacheMode() { return mvkMTLCPUCacheMode(_options); }

/** Returns the Metal resource options used by this memory allocation. */
inline MTLResourceOptions getMTLResourceOptions() { return mvkMTLResourceOptions(_mtlStorageMode, _mtlCPUCacheMode); }
inline MTLResourceOptions getMTLResourceOptions() { return _options; }


#pragma mark Construction
Expand All @@ -136,41 +128,61 @@ class MVKDeviceMemory : public MVKVulkanAPIDeviceObject {
const VkMemoryAllocateInfo* pAllocateInfo,
const VkAllocationCallbacks* pAllocator);

~MVKDeviceMemory() override;
~MVKDeviceMemory() override;

protected:
friend class MVKBuffer;
friend class MVKImageMemoryBinding;
friend class MVKImagePlane;
friend class MVKImageMemoryBinding;
friend class MVKImagePlane;

void propagateDebugName() override;
VkDeviceSize adjustMemorySize(VkDeviceSize size, VkDeviceSize offset);
VkResult addBuffer(MVKBuffer* mvkBuff);
void removeBuffer(MVKBuffer* mvkBuff);
VkResult addImageMemoryBinding(MVKImageMemoryBinding* mvkImg);
void removeImageMemoryBinding(MVKImageMemoryBinding* mvkImg);
bool ensureMTLHeap();
bool ensureMTLBuffer();
bool ensureHostMemory();
void freeHostMemory();
MVKResource* getDedicatedResource();
void initExternalMemory(VkExternalMemoryHandleTypeFlags handleTypes);

MVKSmallVector<MVKBuffer*, 4> _buffers;
MVKSmallVector<MVKImageMemoryBinding*, 4> _imageMemoryBindings;
std::mutex _rezLock;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've removed this since my understanding is they are used to read/write to the resources that use this memory device objects. Allocated resources now will be placed in heaps and a buffer that spans the whole heap is provided to accomplish this. The only exception are imported resources, these are handled slightly different. The explanation is in code comments, but in short, textures require to be dedicated for export/import by VK_EXT_metal_objects, so we can create buffers when importing to account for host mapping due to Metal not providing anything; and buffers will just not have a heap if they are not backed by it at import.

VkDeviceSize _allocationSize = 0;
MVKMappedMemoryRange _mappedRange;
id<MTLBuffer> _mtlBuffer = nil;
void checkExternalMemoryRequirements(VkExternalMemoryHandleTypeFlags handleTypes);

// Backing memory of VkDeviceMemory. This will not be allocated if memory was imported.
// Imported memory will directly be backed by MTLBuffer/MTLTexture since there's no way
// to create a MTLHeap with existing memory in Metal for now.
id<MTLHeap> _mtlHeap = nil;
void* _pMemory = nullptr;
void* _pHostMemory = nullptr;

// This MTLBuffer can have 3 usages:
// 1. When a heap is allocated, the buffer will extend the whole heap to be able to map and flush memory.
// 2. When there's no heap, the buffer will be the backing memory of VkDeviceMemory.
// 3. When a texture is imported, the GPU memory will be held by MTLTexture. However, if said texture is
// host accessible, we need to provide some memory for the mapping since Metal provides nothing. In this
// case, the buffer will hold the host memory that will later be copied to the texture once flushed.
id<MTLBuffer> _mtlBuffer = nil;

// If the user is importing a texture that is not backed by MTLHeap nor MTLBuffer, Metal does not expose
// anything to be able to access the texture data such as MTLBuffer::contents. This leads us to having to
// use the MTLTexture as the main GPU resource for the memory. If the texture is also host accessible,
// a buffer with host visible memory will be allocated as pointed out in point 3 before.
id<MTLTexture> _mtlTexture = nil;

// Mapped memory.
void* _map = nullptr;
MVKMemoryRange _mapRange = { 0u, 0u };

// Allocation size.
VkDeviceSize _size = 0u;
// Metal resource options.
MTLResourceOptions _options = 0u;

// When the allocation is dedicated, it will belong to one specific resource.
union {
MVKBuffer* _dedicatedBufferOwner = nullptr;
MVKImage* _dedicatedImageOwner;
};
enum class DedicatedResourceType : uint8_t {
NONE = 0,
BUFFER,
IMAGE
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nit. MoltenVK traditional style is to avoid all-caps for names (except in some old macro definitions). So it would be more consistent to call these None, Buffer, and Image.

};
DedicatedResourceType _dedicatedResourceType = DedicatedResourceType::NONE;

VkMemoryPropertyFlags _vkMemPropFlags;
VkMemoryAllocateFlags _vkMemAllocFlags;
MTLStorageMode _mtlStorageMode;
MTLCPUCacheMode _mtlCPUCacheMode;
bool _isDedicated = false;
bool _isHostMemImported = false;

// Tracks if we need to flush from MTLBuffer to MTLTexture. Used only when memory is an imported texture
// that had no backing MTLBuffer nor MTLHeap
bool _requiresFlushingBufferToTexture = false;
};

Loading
Loading