Remove exposure to leakable instances of NSString, NSArray & NSDictionary.

Reduce creation of autoreleased instances of NSString, NSArray & NSDictionary.
Ensure remaining are covered by deliberate autorelease pools.
This commit is contained in:
Bill Hollings 2019-07-15 18:59:52 -04:00
parent 53b6ced8b3
commit d1a6b5a40d
11 changed files with 123 additions and 93 deletions

View File

@ -76,10 +76,12 @@ void mvkDispatchToMainAndWait(dispatch_block_t block) {
#pragma mark Process environment #pragma mark Process environment
string mvkGetEnvVar(string varName, bool* pWasFound) { string mvkGetEnvVar(string varName, bool* pWasFound) {
NSDictionary* env = [[NSProcessInfo processInfo] environment]; @autoreleasepool {
NSString* envStr = env[@(varName.c_str())]; NSDictionary*nsEnv = [[NSProcessInfo processInfo] environment];
NSString* envStr = nsEnv[@(varName.c_str())];
if (pWasFound) { *pWasFound = envStr != nil; } if (pWasFound) { *pWasFound = envStr != nil; }
return envStr ? envStr.UTF8String : ""; return envStr ? envStr.UTF8String : "";
}
} }
int64_t mvkGetEnvVarInt64(string varName, bool* pWasFound) { int64_t mvkGetEnvVarInt64(string varName, bool* pWasFound) {

View File

@ -31,6 +31,9 @@ Released TBD
- Fix pipeline cache lookups. - Fix pipeline cache lookups.
- Fix race condition between swapchain image destruction and presentation completion callback. - Fix race condition between swapchain image destruction and presentation completion callback.
- Set Metal texture usage to allow texture copy via view. - Set Metal texture usage to allow texture copy via view.
- Fix memory leak in debug marker and debug utils labelling.
- Reduce use of autoreleased Obj-C objects, and ensure those remaining are
covered by deliberate autorelease pools.
- `vkCmdCopyImage()` support copying between compressed and uncompressed formats - `vkCmdCopyImage()` support copying between compressed and uncompressed formats
and validate that formats are compatible for copying. and validate that formats are compatible for copying.
- `vkCmdBufferImageCopy()` fix crash when setting bytes per image in non-arrayed images. - `vkCmdBufferImageCopy()` fix crash when setting bytes per image in non-arrayed images.

View File

@ -399,7 +399,9 @@ id<MTLComputePipelineState> MVKCommandResourceFactory::newCmdCopyQueryPoolResult
id<MTLFunction> MVKCommandResourceFactory::getFunctionNamed(const char* funcName) { id<MTLFunction> MVKCommandResourceFactory::getFunctionNamed(const char* funcName) {
uint64_t startTime = _device->getPerformanceTimestamp(); uint64_t startTime = _device->getPerformanceTimestamp();
id<MTLFunction> mtlFunc = [[_mtlLibrary newFunctionWithName: @(funcName)] autorelease]; NSString* nsFuncName = [[NSString alloc] initWithUTF8String: funcName]; // temp retained
id<MTLFunction> mtlFunc = [[_mtlLibrary newFunctionWithName: nsFuncName] autorelease];
[nsFuncName release]; // release temp NSStr
_device->addActivityPerformance(_device->_performanceStatistics.shaderCompilation.functionRetrieval, startTime); _device->addActivityPerformance(_device->_performanceStatistics.shaderCompilation.functionRetrieval, startTime);
return mtlFunc; return mtlFunc;
} }

View File

@ -372,6 +372,7 @@ VkResult MVKImage::useIOSurface(IOSurfaceRef ioSurface) {
} else { } else {
#pragma clang diagnostic push #pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations" #pragma clang diagnostic ignored "-Wdeprecated-declarations"
@autoreleasepool {
_ioSurface = IOSurfaceCreate((CFDictionaryRef)@{ _ioSurface = IOSurfaceCreate((CFDictionaryRef)@{
(id)kIOSurfaceWidth: @(_extent.width), (id)kIOSurfaceWidth: @(_extent.width),
(id)kIOSurfaceHeight: @(_extent.height), (id)kIOSurfaceHeight: @(_extent.height),
@ -380,6 +381,7 @@ VkResult MVKImage::useIOSurface(IOSurfaceRef ioSurface) {
(id)kIOSurfaceElementHeight: @(mvkMTLPixelFormatBlockTexelSize(_mtlPixelFormat).height), (id)kIOSurfaceElementHeight: @(mvkMTLPixelFormatBlockTexelSize(_mtlPixelFormat).height),
(id)kIOSurfaceIsGlobal: @(true), // Deprecated but needed for interprocess transfers (id)kIOSurfaceIsGlobal: @(true), // Deprecated but needed for interprocess transfers
}); });
}
#pragma clang diagnostic pop #pragma clang diagnostic pop
} }

View File

@ -252,28 +252,28 @@ VkDebugUtilsMessageSeverityFlagBitsEXT MVKInstance::getVkDebugUtilsMessageSeveri
#pragma mark Object Creation #pragma mark Object Creation
// Returns an autoreleased array containing the MTLDevices available on this system, // Returns a new array containing the MTLDevices available on this system, sorted according to power,
// sorted according to power, with higher power GPU's at the front of the array. // with higher power GPU's at the front of the array. This ensures that a lazy app that simply
// This ensures that a lazy app that simply grabs the first GPU will get a high-power // grabs the first GPU will get a high-power one by default. If the MVK_CONFIG_FORCE_LOW_POWER_GPU
// one by default. If the MVK_CONFIG_FORCE_LOW_POWER_GPU env var or build setting is set, // env var or build setting is set, the returned array will only include low-power devices.
// the returned array will only include low-power devices. // It is the caller's responsibility to release the array when not required anymore.
// If Metal is not supported, ensure we return an empty array. // If Metal is not supported, returns an empty array.
static NSArray<id<MTLDevice>>* getAvailableMTLDevices() { static NSArray<id<MTLDevice>>* newAvailableMTLDevicesArray() {
#if MVK_MACOS NSMutableArray* mtlDevs = [NSMutableArray new];
NSArray* mtlDevs = [MTLCopyAllDevices() autorelease];
if ( !mtlDevs ) { return @[]; }
#if MVK_MACOS
NSArray* rawMTLDevs = MTLCopyAllDevices(); // temp retain
if (rawMTLDevs) {
bool forceLowPower = MVK_CONFIG_FORCE_LOW_POWER_GPU; bool forceLowPower = MVK_CONFIG_FORCE_LOW_POWER_GPU;
MVK_SET_FROM_ENV_OR_BUILD_BOOL(forceLowPower, MVK_CONFIG_FORCE_LOW_POWER_GPU); MVK_SET_FROM_ENV_OR_BUILD_BOOL(forceLowPower, MVK_CONFIG_FORCE_LOW_POWER_GPU);
if (forceLowPower) { // Populate the array of appropriate MTLDevices
NSMutableArray* lpDevs = [[NSMutableArray new] autorelease]; for (id<MTLDevice> md in rawMTLDevs) {
for (id<MTLDevice> md in mtlDevs) { if ( !forceLowPower || md.isLowPower ) { [mtlDevs addObject: md]; }
if (md.isLowPower) { [lpDevs addObject: md]; }
} }
return lpDevs;
} else { // Sort by power
return [mtlDevs sortedArrayUsingComparator: ^(id<MTLDevice> md1, id<MTLDevice> md2) { [mtlDevs sortUsingComparator: ^(id<MTLDevice> md1, id<MTLDevice> md2) {
BOOL md1IsLP = md1.isLowPower; BOOL md1IsLP = md1.isLowPower;
BOOL md2IsLP = md2.isLowPower; BOOL md2IsLP = md2.isLowPower;
@ -290,14 +290,18 @@ static NSArray<id<MTLDevice>>* getAvailableMTLDevices() {
return md2IsLP ? NSOrderedAscending : NSOrderedDescending; return md2IsLP ? NSOrderedAscending : NSOrderedDescending;
}]; }];
}
}
[rawMTLDevs release]; // release temp
#endif // MVK_MACOS #endif // MVK_MACOS
#if MVK_IOS #if MVK_IOS
id<MTLDevice> mtlDev = MTLCreateSystemDefaultDevice(); id<MTLDevice> md = MTLCreateSystemDefaultDevice();
return mtlDev ? [NSArray arrayWithObject: mtlDev] : @[]; if (md) { [mtlDevs addObject: md]; }
[md release];
#endif // MVK_IOS #endif // MVK_IOS
return mtlDevs; // retained
} }
MVKInstance::MVKInstance(const VkInstanceCreateInfo* pCreateInfo) : _enabledExtensions(this) { MVKInstance::MVKInstance(const VkInstanceCreateInfo* pCreateInfo) : _enabledExtensions(this) {
@ -326,11 +330,13 @@ MVKInstance::MVKInstance(const VkInstanceCreateInfo* pCreateInfo) : _enabledExte
} }
// Populate the array of physical GPU devices // Populate the array of physical GPU devices
NSArray<id<MTLDevice>>* mtlDevices = getAvailableMTLDevices(); NSArray<id<MTLDevice>>* mtlDevices = newAvailableMTLDevicesArray(); // temp retain
_physicalDevices.reserve(mtlDevices.count); _physicalDevices.reserve(mtlDevices.count);
for (id<MTLDevice> mtlDev in mtlDevices) { for (id<MTLDevice> mtlDev in mtlDevices) {
_physicalDevices.push_back(new MVKPhysicalDevice(this, mtlDev)); _physicalDevices.push_back(new MVKPhysicalDevice(this, mtlDev));
} }
[mtlDevices release]; // release temp
if (_physicalDevices.empty()) { if (_physicalDevices.empty()) {
setConfigurationResult(reportError(VK_ERROR_INCOMPATIBLE_DRIVER, "Vulkan is not supported on this device. MoltenVK requires Metal, which is not available on this device.")); setConfigurationResult(reportError(VK_ERROR_INCOMPATIBLE_DRIVER, "Vulkan is not supported on this device. MoltenVK requires Metal, which is not available on this device."));
} }

View File

@ -49,10 +49,12 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
if ( !_mtlLibrary ) { return MVKMTLFunctionNull; } if ( !_mtlLibrary ) { return MVKMTLFunctionNull; }
id<MTLFunction> mtlFunc = nil;
@autoreleasepool {
NSString* mtlFuncName = @(_shaderConversionResults.entryPoint.mtlFunctionName.c_str()); NSString* mtlFuncName = @(_shaderConversionResults.entryPoint.mtlFunctionName.c_str());
MVKDevice* mvkDev = _owner->getDevice(); MVKDevice* mvkDev = _owner->getDevice();
uint64_t startTime = mvkDev->getPerformanceTimestamp(); uint64_t startTime = mvkDev->getPerformanceTimestamp();
id<MTLFunction> mtlFunc = [[_mtlLibrary newFunctionWithName: mtlFuncName] autorelease]; mtlFunc = [_mtlLibrary newFunctionWithName: mtlFuncName]; // retained
mvkDev->addActivityPerformance(mvkDev->_performanceStatistics.shaderCompilation.functionRetrieval, startTime); mvkDev->addActivityPerformance(mvkDev->_performanceStatistics.shaderCompilation.functionRetrieval, startTime);
if (mtlFunc) { if (mtlFunc) {
@ -64,7 +66,7 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
if (mtlFCs.count) { if (mtlFCs.count) {
// The Metal shader contains function constants and expects to be specialized // The Metal shader contains function constants and expects to be specialized
// Populate the Metal function constant values from the Vulkan specialization info. // Populate the Metal function constant values from the Vulkan specialization info.
MTLFunctionConstantValues* mtlFCVals = [[MTLFunctionConstantValues new] autorelease]; MTLFunctionConstantValues* mtlFCVals = [MTLFunctionConstantValues new]; // temp retain
if (pSpecializationInfo) { if (pSpecializationInfo) {
// Iterate through the provided Vulkan specialization entries, and populate the // Iterate through the provided Vulkan specialization entries, and populate the
// Metal function constant value that matches the Vulkan specialization constantID. // Metal function constant value that matches the Vulkan specialization constantID.
@ -82,8 +84,9 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
// Compile the specialized Metal function, and use it instead of the unspecialized Metal function. // Compile the specialized Metal function, and use it instead of the unspecialized Metal function.
MVKFunctionSpecializer* fs = new MVKFunctionSpecializer(_owner); MVKFunctionSpecializer* fs = new MVKFunctionSpecializer(_owner);
mtlFunc = [fs->newMTLFunction(_mtlLibrary, mtlFuncName, mtlFCVals) autorelease]; mtlFunc = fs->newMTLFunction(_mtlLibrary, mtlFuncName, mtlFCVals); // retained
fs->destroy(); fs->destroy();
[mtlFCVals release]; // release temp
} }
} }
} else { } else {
@ -94,10 +97,10 @@ MVKMTLFunction MVKShaderLibrary::getMTLFunction(const VkSpecializationInfo* pSpe
NSString* dbName = shaderModule-> getDebugName(); NSString* dbName = shaderModule-> getDebugName();
if ( !dbName ) { dbName = _owner-> getDebugName(); } if ( !dbName ) { dbName = _owner-> getDebugName(); }
setLabelIfNotNil(mtlFunc, dbName); setLabelIfNotNil(mtlFunc, dbName);
}
auto& wgSize = _shaderConversionResults.entryPoint.workgroupSize; auto& wgSize = _shaderConversionResults.entryPoint.workgroupSize;
return { mtlFunc, _shaderConversionResults, MTLSizeMake(getWorkgroupDimensionSize(wgSize.width, pSpecializationInfo), return { [mtlFunc autorelease], _shaderConversionResults, MTLSizeMake(getWorkgroupDimensionSize(wgSize.width, pSpecializationInfo),
getWorkgroupDimensionSize(wgSize.height, pSpecializationInfo), getWorkgroupDimensionSize(wgSize.height, pSpecializationInfo),
getWorkgroupDimensionSize(wgSize.depth, pSpecializationInfo))}; getWorkgroupDimensionSize(wgSize.depth, pSpecializationInfo))};
} }
@ -124,7 +127,11 @@ MVKShaderLibrary::MVKShaderLibrary(MVKVulkanAPIDeviceObject* owner,
const string& mslSourceCode, const string& mslSourceCode,
const SPIRVToMSLConversionResults& shaderConversionResults) : _owner(owner) { const SPIRVToMSLConversionResults& shaderConversionResults) : _owner(owner) {
MVKShaderLibraryCompiler* slc = new MVKShaderLibraryCompiler(_owner); MVKShaderLibraryCompiler* slc = new MVKShaderLibraryCompiler(_owner);
_mtlLibrary = slc->newMTLLibrary(@(mslSourceCode.c_str())); // retained
NSString* nsSrc = [[NSString alloc] initWithUTF8String: mslSourceCode.c_str()]; // temp retained
_mtlLibrary = slc->newMTLLibrary(nsSrc); // retained
[nsSrc release]; // release temp string
slc->destroy(); slc->destroy();
_shaderConversionResults = shaderConversionResults; _shaderConversionResults = shaderConversionResults;

View File

@ -38,7 +38,9 @@ void MVKSwapchain::propogateDebugName() {
if (_debugName) { if (_debugName) {
size_t imgCnt = _surfaceImages.size(); size_t imgCnt = _surfaceImages.size();
for (size_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) { for (size_t imgIdx = 0; imgIdx < imgCnt; imgIdx++) {
_surfaceImages[imgIdx]->setDebugName([NSString stringWithFormat: @"%@(%lu)", _debugName, imgIdx].UTF8String); NSString* nsName = [[NSString alloc] initWithFormat: @"%@(%lu)", _debugName, imgIdx]; // temp retain
_surfaceImages[imgIdx]->setDebugName(nsName.UTF8String);
[nsName release]; // release temp string
} }
} }
} }

View File

@ -215,9 +215,11 @@ void MVKMetalCompiler::compile(unique_lock<mutex>& lock, dispatch_block_t block)
_blocker.wait_for(lock, nanoTimeout, [this]{ return _isCompileDone; }); _blocker.wait_for(lock, nanoTimeout, [this]{ return _isCompileDone; });
if ( !_isCompileDone ) { if ( !_isCompileDone ) {
@autoreleasepool {
NSString* errDesc = [NSString stringWithFormat: @"Timeout after %.3f milliseconds. Likely internal Metal compiler error", (double)nanoTimeout.count() / 1e6]; NSString* errDesc = [NSString stringWithFormat: @"Timeout after %.3f milliseconds. Likely internal Metal compiler error", (double)nanoTimeout.count() / 1e6];
_compileError = [[NSError alloc] initWithDomain: @"MoltenVK" code: 1 userInfo: @{NSLocalizedDescriptionKey : errDesc}]; // retained _compileError = [[NSError alloc] initWithDomain: @"MoltenVK" code: 1 userInfo: @{NSLocalizedDescriptionKey : errDesc}]; // retained
} }
}
if (_compileError) { handleError(); } if (_compileError) { handleError(); }

View File

@ -39,7 +39,7 @@ void MVKVulkanAPIObject::destroy() {
VkResult MVKVulkanAPIObject::setDebugName(const char* pObjectName) { VkResult MVKVulkanAPIObject::setDebugName(const char* pObjectName) {
if (pObjectName) { if (pObjectName) {
[_debugName release]; [_debugName release];
_debugName = [[NSString stringWithUTF8String: pObjectName] retain]; // retained _debugName = [[NSString alloc] initWithUTF8String: pObjectName]; // retained
propogateDebugName(); propogateDebugName();
} }
return VK_SUCCESS; return VK_SUCCESS;

View File

@ -59,8 +59,10 @@ void MVKGPUCaptureScope::makeDefault() {
MVKGPUCaptureScope::MVKGPUCaptureScope(MVKQueue* mvkQueue, const char* purpose) : _queue(mvkQueue) { MVKGPUCaptureScope::MVKGPUCaptureScope(MVKQueue* mvkQueue, const char* purpose) : _queue(mvkQueue) {
_mtlQueue = [_queue->getMTLCommandQueue() retain]; // retained _mtlQueue = [_queue->getMTLCommandQueue() retain]; // retained
if (mvkOSVersion() >= kMinOSVersionMTLCaptureScope) { if (mvkOSVersion() >= kMinOSVersionMTLCaptureScope) {
NSString* nsQLbl = [[NSString alloc] initWithUTF8String: (_queue->getName() + "-" + purpose).c_str()]; // temp retained
_mtlCaptureScope = [[MTLCaptureManager sharedCaptureManager] newCaptureScopeWithCommandQueue: _mtlQueue]; // retained _mtlCaptureScope = [[MTLCaptureManager sharedCaptureManager] newCaptureScopeWithCommandQueue: _mtlQueue]; // retained
_mtlCaptureScope.label = @((_queue->getName() + "-" + purpose).c_str()); _mtlCaptureScope.label = nsQLbl;
[nsQLbl release]; // release temp
} }
} }

View File

@ -304,9 +304,11 @@ void MVKWatermark::initTexture(unsigned char* textureContent,
// Initialize the shader functions for rendering the watermark // Initialize the shader functions for rendering the watermark
void MVKWatermark::initShaders(const char* mslSourceCode) { void MVKWatermark::initShaders(const char* mslSourceCode) {
NSError* err = nil; NSError* err = nil;
id<MTLLibrary> mtlLib = [[_mtlDevice newLibraryWithSource: @(mslSourceCode) NSString* nsSrc = [[NSString alloc] initWithUTF8String: mslSourceCode]; // temp retained
id<MTLLibrary> mtlLib = [[_mtlDevice newLibraryWithSource: nsSrc
options: nil options: nil
error: &err] autorelease]; error: &err] autorelease];
[nsSrc release]; // release temp string
MVKAssert( !err, "Could not compile watermark shaders (Error code %li):\n%s", (long)err.code, err.localizedDescription.UTF8String); MVKAssert( !err, "Could not compile watermark shaders (Error code %li):\n%s", (long)err.code, err.localizedDescription.UTF8String);
_mtlFunctionVertex = [mtlLib newFunctionWithName: @"watermarkVertex"]; // retained _mtlFunctionVertex = [mtlLib newFunctionWithName: @"watermarkVertex"]; // retained