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

Improve performance of vkResetDescriptorPool(). #1676

Merged
Merged
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
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
};