From 13687d1a571f57f32345180e3dc458583ba700fd Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Wed, 31 Jan 2018 13:43:19 -0500 Subject: [PATCH] Updates to Metal resource locking. MVKBufferView add lock when creating MTLTexture. MVKDeviceMemory add lock when creating MTLBuffer during memory mapping. MVKMTLBufferAllocator does not need to be threadsafe. Cleanup syntax on other lock handling to add consistency. --- .../MoltenVK/Commands/MVKMTLBufferAllocation.h | 4 +++- .../Commands/MVKMTLBufferAllocation.mm | 8 +++++--- MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h | 1 + MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm | 12 +++++++----- MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h | 1 + .../MoltenVK/GPUObjects/MVKDeviceMemory.mm | 13 +++++++++---- MoltenVK/MoltenVK/GPUObjects/MVKImage.mm | 18 ++++++++++-------- 7 files changed, 36 insertions(+), 21 deletions(-) diff --git a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h index 4c1c216c..5fa0a4c3 100644 --- a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h +++ b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.h @@ -130,14 +130,16 @@ public: * maximum size. Because MVKMTLBufferRegions are created with a power-of-two size, * the largest size of a MVKMTLBufferAllocation dispensed by this instance will be the * next power-of-two value that is at least as big as the specified maximum size. + * If makeThreadSafe is true, a lock will be applied when an allocation is acquired. */ - MVKMTLBufferAllocator(MVKDevice* device, NSUInteger maxRegionLength); + MVKMTLBufferAllocator(MVKDevice* device, NSUInteger maxRegionLength, bool makeThreadSafe = false); ~MVKMTLBufferAllocator() override; protected: std::vector _regionPools; NSUInteger _maxAllocationLength; + bool _makeThreadSafe; }; diff --git a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm index e8d09b4a..804e8a47 100644 --- a/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm +++ b/MoltenVK/MoltenVK/Commands/MVKMTLBufferAllocation.mm @@ -80,11 +80,13 @@ const MVKMTLBufferAllocation* MVKMTLBufferAllocator::acquireMTLBufferRegion(NSUI // Convert max length to the next power-of-two exponent to use as a lookup uint32_t p2Exp = mvkPowerOfTwoExponent(length); - return _regionPools[p2Exp]->acquireObjectSafely(); + MVKMTLBufferAllocationPool* pRP = _regionPools[p2Exp]; + return _makeThreadSafe ? pRP->acquireObjectSafely() : pRP->acquireObject(); } -MVKMTLBufferAllocator::MVKMTLBufferAllocator(MVKDevice* device, NSUInteger maxAllocationLength) : MVKBaseDeviceObject(device) { - _maxAllocationLength = maxAllocationLength; +MVKMTLBufferAllocator::MVKMTLBufferAllocator(MVKDevice* device, NSUInteger maxRegionLength, bool makeThreadSafe) : MVKBaseDeviceObject(device) { + _maxAllocationLength = maxRegionLength; + _makeThreadSafe = makeThreadSafe; // Convert max length to the next power-of-two exponent uint32_t maxP2Exp = mvkPowerOfTwoExponent(_maxAllocationLength); diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h b/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h index 8becc7a3..87f542d1 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.h @@ -122,5 +122,6 @@ protected: id _mtlTexture; VkDeviceSize _byteCount; VkExtent2D _textureSize; + std::mutex _lock; }; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm b/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm index 1168e670..9d105a87 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKBuffer.mm @@ -157,12 +157,9 @@ id MVKBuffer::getMTLBuffer() { id devMemMTLBuff = _deviceMemory->getMTLBuffer(); if (devMemMTLBuff) { return devMemMTLBuff; } + // Lock and check again in case another thread has created the buffer. lock_guard lock(_lock); - - // Check again in case another thread has created the buffer. - if (_mtlBuffer) { - return _mtlBuffer; - } + if (_mtlBuffer) { return _mtlBuffer; } NSUInteger mtlBuffLen = mvkAlignByteOffset(_byteCount, _byteAlignment); _mtlBuffer = [getMTLDevice() newBufferWithLength: mtlBuffLen @@ -203,6 +200,11 @@ MVKBuffer::~MVKBuffer() { id MVKBufferView::getMTLTexture() { if ( !_mtlTexture && _mtlPixelFormat && _device->_pMetalFeatures->texelBuffers) { + + // Lock and check again in case another thread has created the texture. + lock_guard lock(_lock); + if (_mtlTexture) { return _mtlTexture; } + VkDeviceSize byteAlign = _device->_pProperties->limits.minTexelBufferOffsetAlignment; NSUInteger mtlByteCnt = mvkAlignByteOffset(_byteCount, byteAlign); MTLTextureDescriptor* mtlTexDesc = [MTLTextureDescriptor texture2DDescriptorWithPixelFormat: _mtlPixelFormat diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h index 17aad053..5ad05986 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.h @@ -120,6 +120,7 @@ protected: VkDeviceSize _mapOffset; VkDeviceSize _mapSize; id _mtlBuffer; + std::mutex _lock; MTLResourceOptions _mtlResourceOptions; MTLStorageMode _mtlStorageMode; MTLCPUCacheMode _mtlCPUCacheMode; diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm index 0d08d801..83f034ad 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDeviceMemory.mm @@ -62,10 +62,15 @@ VkResult MVKDeviceMemory::map(VkDeviceSize offset, VkDeviceSize size, VkMemoryMa if ( !mapToSingleResource(offset, mapSize) ) { if (isMemoryHostCoherent()) { if ( !_mtlBuffer ) { - NSUInteger mtlBuffLen = mvkAlignByteOffset(_allocationSize, _device->_pMetalFeatures->mtlBufferAlignment); - _mtlBuffer = [getMTLDevice() newBufferWithLength: mtlBuffLen options: _mtlResourceOptions]; // retained -// MVKLogDebug("Allocating host mapped memory %p with offset %d and size %d via underlying coherent MTLBuffer %p of size %d.", this, offset, mapSize, _mtlBuffer , _mtlBuffer.length); - } + + // Lock and check again in case another thread has created the buffer. + lock_guard lock(_lock); + if ( !_mtlBuffer ) { + NSUInteger mtlBuffLen = mvkAlignByteOffset(_allocationSize, _device->_pMetalFeatures->mtlBufferAlignment); + _mtlBuffer = [getMTLDevice() newBufferWithLength: mtlBuffLen options: _mtlResourceOptions]; // retained +// MVKLogDebug("Allocating host mapped memory %p with offset %d and size %d via underlying coherent MTLBuffer %p of size %d.", this, offset, mapSize, _mtlBuffer , _mtlBuffer.length); + } + } _pLogicalMappedMemory = _mtlBuffer.contents; _pMappedMemory = (void*)((uintptr_t)_pLogicalMappedMemory + offset); } else { diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm index 7c19c1e1..8aea412a 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKImage.mm @@ -216,11 +216,12 @@ void* MVKImage::map(VkDeviceSize offset, VkDeviceSize size) { id MVKImage::getMTLTexture() { if ( !_mtlTexture && _mtlPixelFormat ) { + + // Lock and check again in case another thread has created the texture. lock_guard lock(_lock); - // Check again in case another thread has created the texture. - if ( !_mtlTexture ) { - _mtlTexture = newMTLTexture(); // retained - } + if (_mtlTexture) { return _mtlTexture; } + + _mtlTexture = newMTLTexture(); // retained } return _mtlTexture; } @@ -544,11 +545,12 @@ id MVKImageView::getMTLTexture() { // If we can use a Metal texture view, lazily create it, otherwise use the image texture directly. if (_useMTLTextureView) { if ( !_mtlTexture && _mtlPixelFormat ) { + + // Lock and check again in case another thread created the texture view lock_guard lock(_lock); - // Check again in case another thread created the texture view - if ( !_mtlTexture ) { - _mtlTexture = newMTLTexture(); // retained - } + if (_mtlTexture) { return _mtlTexture; } + + _mtlTexture = newMTLTexture(); // retained } return _mtlTexture; } else {