From bf3cdf286cbaed792f1dab2786692bd759a26a27 Mon Sep 17 00:00:00 2001 From: Jason Macnak Date: Wed, 11 Dec 2024 11:19:41 -0800 Subject: [PATCH] Update VkDecoderSnapshot locking Replace tryLock / unlock with regular scoped lock now that the "extra handles" have been moved out of VkReconstruction and into the VkSnapshotApiCallInfo. Switch to regular std::mutex and std::lock_guard. Annotate mReconstruction with GUARDED_BY to start to get more thread safety analysis. Reviewed-by: Marcin Radomski Part-of: --- .../codegen/scripts/cereal/decodersnapshot.py | 38 +++++++------------ .../codegen/scripts/cerealgenerator.py | 6 ++- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py index acc9f9ffe63..36c7016b840 100644 --- a/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py +++ b/src/gfxstream/codegen/scripts/cereal/decodersnapshot.py @@ -63,29 +63,30 @@ public: Impl() { } void save(android::base::Stream* stream) { + std::lock_guard lock(mReconstructionMutex); mReconstruction.save(stream); } - void load(android::base::Stream* stream, GfxApiLogger& gfx_logger, - HealthMonitor<>* healthMonitor) { + void load(android::base::Stream* stream, GfxApiLogger& gfx_logger, HealthMonitor<>* healthMonitor) { + std::lock_guard lock(mReconstructionMutex); mReconstruction.load(stream, gfx_logger, healthMonitor); } VkSnapshotApiCallInfo* createApiCallInfo() { - android::base::AutoLock lock(mLock); + std::lock_guard lock(mReconstructionMutex); return mReconstruction.createApiCallInfo(); } void destroyApiCallInfoIfUnused(VkSnapshotApiCallInfo* info) { - android::base::AutoLock lock(mLock); + std::lock_guard lock(mReconstructionMutex); return mReconstruction.destroyApiCallInfoIfUnused(info); } """ decoder_snapshot_impl_postamble = """ private: - android::base::Lock mLock; - VkReconstruction mReconstruction; + std::mutex mReconstructionMutex; + VkReconstruction mReconstruction GUARDED_BY(mReconstructionMutex); }; VkDecoderSnapshot::VkDecoderSnapshot() : @@ -95,8 +96,7 @@ void VkDecoderSnapshot::save(android::base::Stream* stream) { mImpl->save(stream); } -void VkDecoderSnapshot::load(android::base::Stream* stream, GfxApiLogger& gfx_logger, - HealthMonitor<>* healthMonitor) { +void VkDecoderSnapshot::load(android::base::Stream* stream, GfxApiLogger& gfx_logger, HealthMonitor<>* healthMonitor) { mImpl->load(stream, gfx_logger, healthMonitor); } @@ -230,11 +230,6 @@ apiSequences = { "vkAllocateMemory" : ["vkAllocateMemory", "vkMapMemoryIntoAddressSpaceGOOGLE"] } -apiCrreateExtraHandles = [ - "vkCreateDevice", - "vkCreateDescriptorPool", -] - @dataclass(frozen=True) class VkObjectState: vk_object : str @@ -251,7 +246,7 @@ def api_special_implementation_vkBindImageMemory2(api, cgen): parentType = "VkDeviceMemory" childObj = "boxed_%s" % childType parentObj = "boxed_%s" % parentType - cgen.stmt("android::base::AutoLock lock(mLock)") + cgen.stmt("std::lock_guard lock(mReconstructionMutex)") cgen.beginFor("uint32_t i = 0", "i < bindInfoCount", "++i") cgen.stmt("%s boxed_%s = unboxed_to_boxed_non_dispatchable_%s(pBindInfos[i].image)" % (childType, childType, childType)) @@ -368,11 +363,8 @@ def emit_impl(typeInfo, api, cgen): boxed_access = "&boxed_%s" % p.typeName if p.pointerIndirectionLevels > 0: cgen.stmt("if (!%s) return" % access) - isCreateExtraHandleApi = api.name in apiCrreateExtraHandles - if isCreateExtraHandleApi: - cgen.stmt("mLock.tryLock()"); - else: - cgen.stmt("android::base::AutoLock lock(mLock)") + + cgen.stmt("std::lock_guard lock(mReconstructionMutex)") cgen.line("// %s create" % p.paramName) if p.isCreatedBy(api): cgen.stmt("mReconstruction.addHandles((const uint64_t*)%s, %s)" % (boxed_access, lenExpr)); @@ -394,11 +386,9 @@ def emit_impl(typeInfo, api, cgen): cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiCallHandle, (const uint64_t*)%s, %s)" % (boxed_access, lenExpr)) if lenAccessGuard is not None: cgen.endIf() - if isCreateExtraHandleApi: - cgen.stmt("mLock.unlock()") if p.isDestroyedBy(api): - cgen.stmt("android::base::AutoLock lock(mLock)") + cgen.stmt("std::lock_guard lock(mReconstructionMutex)") cgen.line("// %s destroy" % p.paramName) if lenAccessGuard is not None: cgen.beginIf(lenAccessGuard) @@ -408,7 +398,7 @@ def emit_impl(typeInfo, api, cgen): cgen.endIf() if is_action_operation(api, p): - cgen.stmt("android::base::AutoLock lock(mLock)") + cgen.stmt("std::lock_guard lock(mReconstructionMutex)") cgen.line("// %s action" % p.paramName) cgen.stmt("VkDecoderGlobalState* m_state = VkDecoderGlobalState::get()") cgen.beginIf("m_state->batchedDescriptorSetUpdateEnabled()") @@ -424,7 +414,7 @@ def emit_impl(typeInfo, api, cgen): cgen.stmt("mReconstruction.setCreatedHandlesForApi(apiCallHandle, (const uint64_t*)(&handle), 1)") elif is_modify_operation(api, p) or is_clear_modifier_operation(api, p): - cgen.stmt("android::base::AutoLock lock(mLock)") + cgen.stmt("std::lock_guard lock(mReconstructionMutex)") cgen.line("// %s modify" % p.paramName) cgen.stmt("auto apiCallHandle = apiCallInfo->handle") cgen.stmt("mReconstruction.setApiTrace(apiCallInfo, apiCallPacket, apiCallPacketSize)") diff --git a/src/gfxstream/codegen/scripts/cerealgenerator.py b/src/gfxstream/codegen/scripts/cerealgenerator.py index 697318ebc75..bf2febef681 100644 --- a/src/gfxstream/codegen/scripts/cerealgenerator.py +++ b/src/gfxstream/codegen/scripts/cerealgenerator.py @@ -485,17 +485,19 @@ using DlSymFunc = void* (void*, const char*); decoderSnapshotHeaderIncludes = f""" #include + #include "VkSnapshotApiCall.h" #include "{self.utilsHeaderDirPrefix}/GfxApiLogger.h" #include "{self.baseLibDirPrefix}/HealthMonitor.h" #include "goldfish_vk_private_defs.h" """ decoderSnapshotImplIncludes = f""" +#include + #include "VulkanHandleMapping.h" #include "VkDecoderGlobalState.h" #include "VkReconstruction.h" - -#include "{self.baseLibDirPrefix}/synchronization/Lock.h" +#include "{self.baseLibDirPrefix}/ThreadAnnotations.h" """ decoderHeaderIncludes = f"""