From b16fef0ca21170e00eaf39b2b21f5b4389f7a3dd Mon Sep 17 00:00:00 2001 From: Bill Hollings Date: Tue, 9 Aug 2022 16:28:22 -0400 Subject: [PATCH] Improve performance of vkResetDescriptorPool(). - MVKDescriptorPool::reset() don't waste time freeing descriptor sets that were never allocated. - If descriptor set could not be allocated, set availability bit (unrelated). - MVKBitArray add _lowestNeverClearedBitIndex to track the lowest bit index that has not been cleared since last reset. - MVKBitArray rename _minUnclearedSectionIndex to _clearedSectionCount for clarity. - MVKBitArray use _clearedSectionCount and _lowestNeverClearedBitIndex to optimize operation of setting or clearing all bits. - MVKBitArray::setBit() ensure we don't try to change a bit that is out of range. - MVKBitArray::resize() no-op if size doesn't actually change. - MVKQueue don't include object pointer in error log, so CTS log results are consistent across multiple CTS runs (unrelated). --- Docs/Whats_New.md | 1 + .../MoltenVK/GPUObjects/MVKDescriptorSet.mm | 12 +++- MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm | 2 +- MoltenVK/MoltenVK/Utility/MVKBitArray.h | 69 +++++++++++++------ 4 files changed, 59 insertions(+), 25 deletions(-) diff --git a/Docs/Whats_New.md b/Docs/Whats_New.md index b6b3f4f28..9b6287a1e 100644 --- a/Docs/Whats_New.md +++ b/Docs/Whats_New.md @@ -31,6 +31,7 @@ Released TBD - Fix query pool wait block when query is not encoded to be written to. - Fix `vkUpdateDescriptorSetWithTemplate()` for inline block descriptors. - Fix retrieval of accurate `vkGetRefreshCycleDurationGOOGLE()` across multiple display screens. +- Improve performance of `vkResetDescriptorPool()`. - Report appropriate values of `VkDebugUtilsMessageTypeFlagsEXT` for debug util messages generated within MoltenVK. - Update _macOS Cube_ demo to demonstrate optimizing the swapchain across multiple display screens. - Update `VK_MVK_MOLTENVK_SPEC_VERSION` to version `35`. diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm index 1a303581f..156572be8 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm @@ -525,8 +525,10 @@ *pVKDS = (VkDescriptorSet)mvkDS; } return false; + } else { + _descriptorSetAvailablility.setBit(dsIdx); // We didn't consume this one after all, so it's still available + return true; } - return true; }); return rslt; } @@ -555,9 +557,13 @@ } } -// Free all descriptor sets and reset descriptor pools +// Free allocated descriptor sets and reset descriptor pools. +// Don't waste time freeing desc sets that were never allocated. VkResult MVKDescriptorPool::reset(VkDescriptorPoolResetFlags flags) { - for (auto& mvkDS : _descriptorSets) { freeDescriptorSet(&mvkDS, true); } + size_t dsCnt = _descriptorSetAvailablility.getLowestNeverClearedBitIndex(); + for (uint32_t dsIdx = 0; dsIdx < dsCnt; dsIdx++) { + freeDescriptorSet(&_descriptorSets[dsIdx], true); + } _descriptorSetAvailablility.setAllBits(); _uniformBufferDescriptors.reset(); diff --git a/MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm b/MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm index 1908f379f..4db74af8a 100644 --- a/MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm +++ b/MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm @@ -377,7 +377,7 @@ // the physical device, always mark both the device and physical device as lost. // If the error is local to this command buffer, optionally mark the device (but not the // physical device) as lost, depending on the value of MVKConfiguration::resumeLostDevice. - getVulkanAPIObject()->reportError(VK_ERROR_DEVICE_LOST, "Command buffer %p \"%s\" execution failed (code %li): %s", mtlCB, mtlCB.label ? mtlCB.label.UTF8String : "", mtlCB.error.code, mtlCB.error.localizedDescription.UTF8String); + getVulkanAPIObject()->reportError(VK_ERROR_DEVICE_LOST, "MTLCommandBuffer \"%s\" execution failed (code %li): %s", mtlCB.label ? mtlCB.label.UTF8String : "", mtlCB.error.code, mtlCB.error.localizedDescription.UTF8String); switch (mtlCB.error.code) { case MTLCommandBufferErrorBlacklisted: case MTLCommandBufferErrorNotPermitted: // May also be used for command buffers executed in the background without the right entitlement. diff --git a/MoltenVK/MoltenVK/Utility/MVKBitArray.h b/MoltenVK/MoltenVK/Utility/MVKBitArray.h index 1172e7390..351c2985f 100755 --- a/MoltenVK/MoltenVK/Utility/MVKBitArray.h +++ b/MoltenVK/MoltenVK/Utility/MVKBitArray.h @@ -47,13 +47,16 @@ class MVKBitArray { /** Sets the value of the bit to the val (or to 1 by default). */ void setBit(size_t bitIndex, bool val = true) { + if (bitIndex >= _bitCount) { return; } + size_t secIdx = getIndexOfSection(bitIndex); if (val) { mvkEnableFlags(getSection(secIdx), getSectionSetMask(bitIndex)); - if (secIdx < _minUnclearedSectionIndex) { _minUnclearedSectionIndex = secIdx; } + if (secIdx < _clearedSectionCount) { _clearedSectionCount = secIdx; } } else { mvkDisableFlags(getSection(secIdx), getSectionSetMask(bitIndex)); - if (secIdx == _minUnclearedSectionIndex && !getSection(secIdx)) { _minUnclearedSectionIndex++; } + if (secIdx == _clearedSectionCount && !getSection(secIdx)) { _clearedSectionCount++; } + _lowestNeverClearedBitIndex = std::max(_lowestNeverClearedBitIndex, bitIndex + 1); } } @@ -61,26 +64,42 @@ class MVKBitArray { void clearBit(size_t bitIndex) { setBit(bitIndex, false); } /** Sets all bits in the array to 1. */ - void setAllBits() { setAllSections(~0); } + void setAllBits() { + // Nothing to do if no bits have been cleared (also ensure _lowestNeverClearedBitIndex doesn't go negative) + if (_lowestNeverClearedBitIndex) { + size_t endSecIdx = getIndexOfSection(_lowestNeverClearedBitIndex - 1); + for (size_t secIdx = 0; secIdx <= endSecIdx; secIdx++) { + getSection(secIdx) = ~0; + } + } + _clearedSectionCount = 0; + _lowestNeverClearedBitIndex = 0; + } /** Clears all bits in the array to 0. */ - void clearAllBits() { setAllSections(0); } + void clearAllBits() { + size_t secCnt = getSectionCount(); + while (_clearedSectionCount < secCnt) { + getSection(_clearedSectionCount++) = 0; + } + _lowestNeverClearedBitIndex = _bitCount; + } /** * Returns the index of the first bit that is set, at or after the specified index, * and optionally clears that bit. If no bits are set, returns the size() of this bit array. */ size_t getIndexOfFirstSetBit(size_t startIndex, bool shouldClear) { - size_t startSecIdx = std::max(getIndexOfSection(startIndex), _minUnclearedSectionIndex); + size_t startSecIdx = std::max(getIndexOfSection(startIndex), _clearedSectionCount); size_t bitIdx = startSecIdx << SectionMaskSize; size_t secCnt = getSectionCount(); for (size_t secIdx = startSecIdx; secIdx < secCnt; secIdx++) { size_t lclBitIdx = getIndexOfFirstSetBitInSection(getSection(secIdx), getBitIndexInSection(startIndex)); bitIdx += lclBitIdx; if (lclBitIdx < SectionBitCount) { - if (startSecIdx == _minUnclearedSectionIndex && !getSection(startSecIdx)) { _minUnclearedSectionIndex = secIdx; } + if (startSecIdx == _clearedSectionCount && !getSection(startSecIdx)) { _clearedSectionCount = secIdx; } if (shouldClear) { clearBit(bitIdx); } - return bitIdx; + return std::min(bitIdx, _bitCount); } } return std::min(bitIdx, _bitCount); @@ -102,6 +121,12 @@ class MVKBitArray { return getIndexOfFirstSetBit(0, shouldClear); } + /** + * Returns the index of the lowest bit that has never been cleared since the last time all the bits were set or cleared. + * In other words, this bit, and all above it, have never been cleared since the last time they were all set or cleared. + */ + size_t getLowestNeverClearedBitIndex() { return _lowestNeverClearedBitIndex; } + /** * Returns the index of the first bit that is set. * If no bits are set, returns the size() of this bit array. @@ -149,6 +174,8 @@ class MVKBitArray { * unless the size is set to zero. */ void resize(size_t size, bool val = false) { + if (size == _bitCount) { return; } + size_t oldBitCnt = _bitCount; size_t oldSecCnt = getSectionCount(); size_t oldEndBitCnt = oldSecCnt << SectionMaskSize; @@ -165,7 +192,8 @@ class MVKBitArray { // Clear out the existing data if (oldSecCnt > 1) { free(pOldData); } _data = 0; - _minUnclearedSectionIndex = 0; + _clearedSectionCount = 0; + _lowestNeverClearedBitIndex = 0; } else if (newSecCnt == oldSecCnt) { // Keep the existing data, but fill any bits in the last section // that were beyond the old bit count with the new initial value. @@ -185,10 +213,13 @@ class MVKBitArray { memcpy(pNewData, pOldData, oldByteCnt); for (size_t bitIdx = oldBitCnt; bitIdx < oldEndBitCnt; bitIdx++) { setBit(bitIdx, val); } if (oldSecCnt > 1) { free(pOldData); } + if (!val) { _lowestNeverClearedBitIndex = _bitCount; } // Cover additional sections // If the entire old array and the new array are cleared, move the uncleared indicator to the new end. - if (_minUnclearedSectionIndex == oldSecCnt && !val) { _minUnclearedSectionIndex = newSecCnt; } + if (_clearedSectionCount == oldSecCnt && !val) { _clearedSectionCount = newSecCnt; } } + // If we shrank, ensure this value still fits + if (_lowestNeverClearedBitIndex > _bitCount) { _lowestNeverClearedBitIndex = _bitCount; } } /** Constructs an instance for the specified number of bits, and sets the initial value of all the bits. */ @@ -197,12 +228,16 @@ class MVKBitArray { MVKBitArray(const MVKBitArray& other) { resize(other._bitCount); memcpy(getData(), other.getData(), getSectionCount() * SectionByteCount); + _clearedSectionCount = other._clearedSectionCount; + _lowestNeverClearedBitIndex = other._lowestNeverClearedBitIndex; } MVKBitArray& operator=(const MVKBitArray& other) { - resize(0); + resize(0); // Clear out the old memory resize(other._bitCount); memcpy(getData(), other.getData(), getSectionCount() * SectionByteCount); + _clearedSectionCount = other._clearedSectionCount; + _lowestNeverClearedBitIndex = other._lowestNeverClearedBitIndex; return *this; } @@ -252,16 +287,8 @@ class MVKBitArray { return section ? __builtin_clzll(section) : SectionBitCount; } - // Sets the content of all sections to the value - void setAllSections(uint64_t sectionValue) { - size_t secCnt = getSectionCount(); - for (size_t secIdx = 0; secIdx < secCnt; secIdx++) { - getSection(secIdx) = sectionValue; - } - _minUnclearedSectionIndex = sectionValue ? 0 : secCnt; - } - - uint64_t* _data = 0; + uint64_t* _data = nullptr; size_t _bitCount = 0; - size_t _minUnclearedSectionIndex = 0; // Tracks where to start looking for bits that are set + size_t _clearedSectionCount = 0; // Tracks where to start looking for bits that are set + size_t _lowestNeverClearedBitIndex = 0; // Tracks the lowest bit that has never been cleared };