Skip to content

Commit

Permalink
Improve performance of vkResetDescriptorPool().
Browse files Browse the repository at this point in the history
- 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).
  • Loading branch information
billhollings committed Aug 9, 2022
1 parent af88bb9 commit b16fef0
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 25 deletions.
1 change: 1 addition & 0 deletions Docs/Whats_New.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
12 changes: 9 additions & 3 deletions MoltenVK/MoltenVK/GPUObjects/MVKDescriptorSet.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion MoltenVK/MoltenVK/GPUObjects/MVKQueue.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
69 changes: 48 additions & 21 deletions MoltenVK/MoltenVK/Utility/MVKBitArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,40 +47,59 @@ 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);
}
}

/** Sets the value of the bit to 0. */
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);
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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. */
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
};

0 comments on commit b16fef0

Please sign in to comment.