Fix intermittent concurrent shader specialization race condition.

MVKShaderLibrary::getMTLFunction() synchronize and refactor release of Metal objects.
Make use of existing autorelease pool instead of discrete retain/release.
Wrap entire specialization operation in @synchronized() to guard against
Metal internals not coping with multiple simultaneous specializations.
This commit is contained in:
Bill Hollings 2020-07-22 17:48:19 -04:00
parent c0103fd008
commit 1f68d5fc2a
2 changed files with 49 additions and 59 deletions

View File

@ -32,6 +32,7 @@ Released TBD
- Fix issue where mapped host-coherent device memory not updated from image contents on *macOS*.
- Fix small memory leak when setting swapchain color space.
- Fix new and unexpected App Store failure on newly deprecated color space values.
- Fix intermittent concurrent shader specialization race condition.
- Include vertex attribute size when testing whether attribute offset exceeds stride.
- Add `MVKPhysicalDeviceMetalFeatures::vertexStrideAlignment` to track Metal vertex binding stride alignment.
- Add `MVKPhysicalDeviceMetalFeatures::indirectTessellationDrawing` to track if indirect tessellation drawing is supported.

View File

@ -64,45 +64,44 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
if ( !_mtlLibrary ) { return MVKMTLFunctionNull; }
id<MTLFunction> mtlFunc = nil;
@synchronized (_mtlLibrary) {
@autoreleasepool {
NSString* mtlFuncName = @(_shaderConversionResults.entryPoint.mtlFunctionName.c_str());
MVKDevice* mvkDev = _owner->getDevice();
uint64_t startTime = mvkDev->getPerformanceTimestamp();
mtlFunc = [_mtlLibrary newFunctionWithName: mtlFuncName]; // temp retain
id<MTLFunction> mtlFunc = [[_mtlLibrary newFunctionWithName: mtlFuncName] autorelease];
mvkDev->addActivityPerformance(mvkDev->_performanceStatistics.shaderCompilation.functionRetrieval, startTime);
if (mtlFunc) {
// If the Metal device supports shader specialization, and the Metal function expects to be
// specialized, populate Metal function constant values from the Vulkan specialization info,
// and compiled a specialized Metal function, otherwise simply use the unspecialized Metal function.
// If the Metal device supports shader specialization, and the Metal function expects to be specialized,
// populate Metal function constant values from the Vulkan specialization info, and compile a specialized
// Metal function, otherwise simply use the unspecialized Metal function.
if (mvkDev->_pMetalFeatures->shaderSpecialization) {
NSArray<MTLFunctionConstant*>* mtlFCs = mtlFunc.functionConstantsDictionary.allValues;
if (mtlFCs.count) {
// The Metal shader contains function constants and expects to be specialized
if (mtlFCs.count > 0) {
// The Metal shader contains function constants and expects to be specialized.
// Populate the Metal function constant values from the Vulkan specialization info.
MTLFunctionConstantValues* mtlFCVals = [MTLFunctionConstantValues new]; // temp retain
MTLFunctionConstantValues* mtlFCVals = [[MTLFunctionConstantValues new] autorelease];
if (pSpecializationInfo) {
// Iterate through the provided Vulkan specialization entries, and populate the
// Metal function constant value that matches the Vulkan specialization constantID.
for (uint32_t specIdx = 0; specIdx < pSpecializationInfo->mapEntryCount; specIdx++) {
const VkSpecializationMapEntry* pMapEntry = &pSpecializationInfo->pMapEntries[specIdx];
NSUInteger mtlFCIndex = pMapEntry->constantID;
MTLFunctionConstant* mtlFC = getFunctionConstant(mtlFCs, mtlFCIndex);
if (mtlFC) {
[mtlFCVals setConstantValue: &(((char*)pSpecializationInfo->pData)[pMapEntry->offset])
type: mtlFC.type
atIndex: mtlFCIndex];
for (MTLFunctionConstant* mfc in mtlFCs) {
if (mfc.index == pMapEntry->constantID) {
[mtlFCVals setConstantValue: ((char*)pSpecializationInfo->pData + pMapEntry->offset)
type: mfc.type
atIndex: mfc.index];
break;
}
}
}
}
// Compile the specialized Metal function, and use it instead of the unspecialized Metal function.
MVKFunctionSpecializer* fs = new MVKFunctionSpecializer(_owner);
[mtlFunc release]; // temp release
mtlFunc = fs->newMTLFunction(_mtlLibrary, mtlFuncName, mtlFCVals); // temp retain
fs->destroy();
[mtlFCVals release]; // temp release
MVKFunctionSpecializer fs(_owner);
mtlFunc = [fs.newMTLFunction(_mtlLibrary, mtlFuncName, mtlFCVals) autorelease];
}
}
} else {
@ -113,23 +112,13 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
NSString* dbName = shaderModule-> getDebugName();
if ( !dbName ) { dbName = _owner-> getDebugName(); }
setLabelIfNotNil(mtlFunc, dbName);
}
auto& wgSize = _shaderConversionResults.entryPoint.workgroupSize;
MVKMTLFunction mvkMTLFunc(mtlFunc, _shaderConversionResults, MTLSizeMake(getWorkgroupDimensionSize(wgSize.width, pSpecializationInfo),
return MVKMTLFunction(mtlFunc, _shaderConversionResults, MTLSizeMake(getWorkgroupDimensionSize(wgSize.width, pSpecializationInfo),
getWorkgroupDimensionSize(wgSize.height, pSpecializationInfo),
getWorkgroupDimensionSize(wgSize.depth, pSpecializationInfo)));
[mtlFunc release]; // temp release
return mvkMTLFunc;
}
// Returns the MTLFunctionConstant with the specified ID from the specified array of function constants.
// The specified ID is the index value contained within the function constant.
MTLFunctionConstant* MVKShaderLibrary::getFunctionConstant(NSArray<MTLFunctionConstant*>* mtlFCs, NSUInteger mtlFCID) {
for (MTLFunctionConstant* mfc in mtlFCs) { if (mfc.index == mtlFCID) { return mfc; } }
return nil;
}
}
}
void MVKShaderLibrary::setEntryPointName(string& funcName) {