buffer up the memory flushes whenever possible, also factor out the mapped memory align function out of IUtilities because its useful

Author: devsh

include/nbl/video/IDeviceMemoryAllocation.h

@@ -91,6 +91,7 @@ class IDeviceMemoryAllocation : public virtual core::IReferenceCounted
         //! Utility function, tell us if writes by the CPU or GPU need extra visibility operations to become visible for reading on the other processor
         /** Only execute flushes or invalidations if the allocation requires them, and batch them (flush one combined range instead of two or more)
         for greater efficiency. To execute a flush or invalidation, use IDriver::flushMappedAllocationRanges and IDriver::invalidateMappedAllocationRanges respectively. */
+        // TODO: Visible is a misnomer, collides with Vulkan memory model nomenclature where visibility only concerns reads, where as this is both read and write (visibility and availability)
         inline bool haveToMakeVisible() const
         {
             return !m_memoryPropertyFlags.hasFlags(EMPF_HOST_COHERENT_BIT);
@@ -102,6 +103,9 @@ class IDeviceMemoryAllocation : public virtual core::IReferenceCounted
             size_t offset = 0ull;
             size_t length = 0ull;
         };
+        // makes sure offset and length are aligned to the `SPhysicalDeviceLimits::nonCoherentAtomSize` but also not outside the memory allocation
+        MemoryRange alignNonCoherentRange(MemoryRange range) const;
+        //
         inline void* map(const MemoryRange& range, const core::bitflag<E_MAPPING_CPU_ACCESS_FLAGS> accessHint=IDeviceMemoryAllocation::EMCAF_READ_AND_WRITE)
         {
             if (isCurrentlyMapped())

include/nbl/video/ILogicalDevice.h

@@ -202,8 +202,12 @@ class NBL_API2 ILogicalDevice : public core::IReferenceCounted, public IDeviceMe
         //! Similar to VkMappedMemoryRange but no pNext
         struct MappedMemoryRange
         {
+            struct align_non_coherent_tag_t {};
+            constexpr static inline align_non_coherent_tag_t align_non_coherent_tag = {};
+
             MappedMemoryRange() : memory(nullptr), range{} {}
-            MappedMemoryRange(IDeviceMemoryAllocation* mem, const size_t& off, const size_t& len) : memory(mem), range{off,len} {}
+            MappedMemoryRange(IDeviceMemoryAllocation* mem, const size_t off, const size_t len) : memory(mem), range{off,len} {}
+            MappedMemoryRange(IDeviceMemoryAllocation* mem, const size_t off, const size_t len, const align_non_coherent_tag_t) : memory(mem), range(mem->alignNonCoherentRange({off,len})) {}
 
             inline bool valid() const
             {

include/nbl/video/utilities/IUtilities.h

@@ -354,6 +354,12 @@ class NBL_API2 IUtilities : public core::IReferenceCounted
             const uint32_t optimalTransferAtom = core::min<uint32_t>(limits.maxResidentInvocations*OptimalCoalescedInvocationXferSize,m_defaultUploadBuffer->get_total_size()/4);
             const auto minBlockSize = m_defaultUploadBuffer->getAddressAllocator().min_size();
 
+            core::vector<ILogicalDevice::MappedMemoryRange> flushRanges;
+            const bool manualFlush = m_defaultUploadBuffer.get()->needsManualFlushOrInvalidate();
+            if (manualFlush)
+                flushRanges.reserve((bufferRange.size-1)/m_defaultUploadBuffer.get()->max_size()+1);
+
+            auto* uploadBuffer = m_defaultUploadBuffer.get()->getBuffer();
             // no pipeline barriers necessary because write and optional flush happens before submit, and memory allocation is reclaimed after fence signal
             for (size_t uploadedSize=0ull; uploadedSize<bufferRange.size;)
             {
@@ -386,6 +392,11 @@ class NBL_API2 IUtilities : public core::IReferenceCounted
                 }
                 else
                 {
+                    if (!flushRanges.empty())
+                    {
+                        m_device->flushMappedMemoryRanges(flushRanges);
+                        flushRanges.clear();
+                    }
                     const auto completed = nextSubmit.getFutureScratchSemaphore();
                     nextSubmit.overflowSubmit(scratch);
                     // overflowSubmit no longer blocks for the last submit to have completed, so we must do it ourselves here
@@ -396,21 +407,20 @@ class NBL_API2 IUtilities : public core::IReferenceCounted
                     continue; // keep trying again
                 }
                 // some platforms expose non-coherent host-visible GPU memory, so writes need to be flushed explicitly
-                if (m_defaultUploadBuffer.get()->needsManualFlushOrInvalidate())
-                {
-                    auto flushRange = AlignedMappedMemoryRange(m_defaultUploadBuffer.get()->getBuffer()->getBoundMemory().memory,localOffset,subSize,limits.nonCoherentAtomSize);
-                    m_device->flushMappedMemoryRanges(1u,&flushRange);
-                }
+                if (manualFlush)
+                    flushRanges.emplace_back(uploadBuffer->getBoundMemory().memory,localOffset,subSize,ILogicalDevice::MappedMemoryRange::align_non_coherent_tag);
                 // after we make sure writes are in GPU memory (visible to GPU) and not still in a cache, we can copy using the GPU to device-only memory
                 IGPUCommandBuffer::SBufferCopy copy;
                 copy.srcOffset = localOffset;
                 copy.dstOffset = bufferRange.offset+uploadedSize;
                 copy.size = subSize;
-                scratch->cmdbuf->copyBuffer(m_defaultUploadBuffer.get()->getBuffer(), bufferRange.buffer.get(), 1u, &copy);
+                scratch->cmdbuf->copyBuffer(uploadBuffer, bufferRange.buffer.get(), 1u, &copy);
                 // this doesn't actually free the memory, the memory is queued up to be freed only after the `scratchSemaphore` reaches a value a future submit will signal
                 m_defaultUploadBuffer.get()->multi_deallocate(1u,&localOffset,&allocationSize,nextSubmit.getFutureScratchSemaphore(),&scratch->cmdbuf);
                 uploadedSize += subSize;
             }
+            if (!flushRanges.empty())
+                m_device->flushMappedMemoryRanges(flushRanges);
             return true;
         }
         // overload to make invokers not care about l-value or r-value
@@ -530,11 +540,12 @@ class NBL_API2 IUtilities : public core::IReferenceCounted
                 ~CDownstreamingDataConsumer()
                 {
                     assert(m_downstreamingBuffer);
-                    auto device = const_cast<ILogicalDevice*>(m_downstreamingBuffer->getBuffer()->getOriginDevice());
+                    auto* downstreamingBuffer = m_downstreamingBuffer->getBuffer();
+                    auto device = const_cast<ILogicalDevice*>(downstreamingBuffer->getOriginDevice());
                     if (m_downstreamingBuffer->needsManualFlushOrInvalidate())
                     {
                         const auto nonCoherentAtomSize = device->getPhysicalDevice()->getLimits().nonCoherentAtomSize;
-                        auto flushRange = AlignedMappedMemoryRange(m_downstreamingBuffer->getBuffer()->getBoundMemory().memory,m_copyRange.offset,m_copyRange.length,nonCoherentAtomSize);
+                        auto flushRange = ILogicalDevice::MappedMemoryRange(downstreamingBuffer->getBoundMemory().memory,m_copyRange.offset,m_copyRange.length,ILogicalDevice::MappedMemoryRange::align_non_coherent_tag);
                         device->invalidateMappedMemoryRanges(1u,&flushRange);
                     }
                     // Call the function
@@ -744,17 +755,6 @@ class NBL_API2 IUtilities : public core::IReferenceCounted
             return retval;
         }
 
-        // The application must round down the start of the range to the nearest multiple of VkPhysicalDeviceLimits::nonCoherentAtomSize,
-        // and round the end of the range up to the nearest multiple of VkPhysicalDeviceLimits::nonCoherentAtomSize.
-        static ILogicalDevice::MappedMemoryRange AlignedMappedMemoryRange(IDeviceMemoryAllocation* mem, const size_t& off, const size_t& len, size_t nonCoherentAtomSize)
-        {
-            ILogicalDevice::MappedMemoryRange range = {};
-            range.memory = mem;
-            range.offset = core::alignDown(off, nonCoherentAtomSize);
-            range.length = core::min(core::alignUp(len, nonCoherentAtomSize), mem->getAllocationSize());
-            return range;
-        }
-
 
         core::smart_refctd_ptr<ILogicalDevice> m_device;
 

src/nbl/video/IDeviceMemoryAllocation.cpp

@@ -10,4 +10,12 @@ E_API_TYPE IDeviceMemoryAllocation::getAPIType() const
     return m_originDevice->getAPIType();
 }
 
+IDeviceMemoryAllocation::MemoryRange IDeviceMemoryAllocation::alignNonCoherentRange(MemoryRange range) const
+{
+    const auto alignment = m_originDevice->getPhysicalDevice()->getLimits().nonCoherentAtomSize;
+    range.offset = core::alignDown(range.offset,alignment);
+    range.length = core::min(core::alignUp(range.length,alignment),m_allocationSize);
+    return range;
+}
+
 }

src/nbl/video/utilities/IUtilities.cpp

@@ -73,6 +73,12 @@ bool IUtilities::updateImageViaStagingBuffer(
 
     regionsToCopy.reserve(maxIterations);
 
+    core::vector<ILogicalDevice::MappedMemoryRange> flushRanges;
+    const bool manualFlush = m_defaultUploadBuffer.get()->needsManualFlushOrInvalidate();
+    if (manualFlush)
+        flushRanges.reserve(maxIterations);
+
+    auto* uploadBuffer = m_defaultUploadBuffer.get()->getBuffer();
     while (!regionIterator.isFinished())
     {
         size_t memoryNeededForRemainingRegions = regionIterator.getMemoryNeededForRemainingRegions();
@@ -95,6 +101,11 @@ bool IUtilities::updateImageViaStagingBuffer(
         // keep trying again
         if (failedAllocation)
         {
+            if (!flushRanges.empty())
+            {
+                m_device->flushMappedMemoryRanges(flushRanges);
+                flushRanges.clear();
+            }
             const auto completed = intendedNextSubmit.getFutureScratchSemaphore();
             intendedNextSubmit.overflowSubmit(scratch);
             // overflowSubmit no longer blocks for the last submit to have completed, so we must do it ourselves here
@@ -123,22 +134,23 @@ bool IUtilities::updateImageViaStagingBuffer(
             }
 
             if (!regionsToCopy.empty())
-                scratch->cmdbuf->copyBufferToImage(m_defaultUploadBuffer.get()->getBuffer(), dstImage, currentDstImageLayout, regionsToCopy.size(), regionsToCopy.data());
+                scratch->cmdbuf->copyBufferToImage(uploadBuffer, dstImage, currentDstImageLayout, regionsToCopy.size(), regionsToCopy.data());
 
             assert(!regionsToCopy.empty() && "allocationSize is not enough to support the smallest possible transferable units to image, may be caused if your queueFam's minImageTransferGranularity is large or equal to <0,0,0>.");
 
             // some platforms expose non-coherent host-visible GPU memory, so writes need to be flushed explicitly
-            if (m_defaultUploadBuffer.get()->needsManualFlushOrInvalidate())
+            if (manualFlush)
             {
                 const auto consumedMemory = allocationSize - availableUploadBufferMemory;
-                auto flushRange = AlignedMappedMemoryRange(m_defaultUploadBuffer.get()->getBuffer()->getBoundMemory().memory, localOffset, consumedMemory, limits.nonCoherentAtomSize);
-                m_device->flushMappedMemoryRanges(1u, &flushRange);
+                flushRanges.emplace_back(uploadBuffer->getBoundMemory().memory, localOffset, consumedMemory, ILogicalDevice::MappedMemoryRange::align_non_coherent_tag);
             }
         }
 
         // this doesn't actually free the memory, the memory is queued up to be freed only after the GPU fence/event is signalled
         m_defaultUploadBuffer.get()->multi_deallocate(1u,&localOffset,&allocationSize,intendedNextSubmit.getFutureScratchSemaphore()); // can queue with a reset but not yet pending fence, just fine
     }
+    if (!flushRanges.empty())
+        m_device->flushMappedMemoryRanges(flushRanges);
     return true;
 }