diff --git a/llvm/include/llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h b/llvm/include/llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h --- a/llvm/include/llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h @@ -70,7 +70,7 @@ ExecutionSession &ES; using OwnedDebugObject = std::unique_ptr; - std::map PendingObjs; + std::map PendingObjs; std::map> RegisteredObjs; std::mutex PendingObjsLock; diff --git a/llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp b/llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp --- a/llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp +++ b/llvm/lib/ExecutionEngine/Orc/DebugObjectManagerPlugin.cpp @@ -9,6 +9,7 @@ #include "llvm/ExecutionEngine/Orc/DebugObjectManagerPlugin.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/BinaryFormat/ELF.h" @@ -407,17 +408,15 @@ void DebugObjectManagerPlugin::notifyMaterializing( MaterializationResponsibility &MR, LinkGraph &G, JITLinkContext &Ctx, MemoryBufferRef ObjBuffer) { - assert(PendingObjs.count(getResourceKey(MR)) == 0 && + std::lock_guard Lock(PendingObjsLock); + assert(PendingObjs.count(&MR) == 0 && "Cannot have more than one pending debug object per " "MaterializationResponsibility"); - std::lock_guard Lock(PendingObjsLock); if (auto DebugObj = createDebugObjectFromBuffer(ES, G, Ctx, ObjBuffer)) { // Not all link artifacts allow debugging. - if (*DebugObj != nullptr) { - ResourceKey Key = getResourceKey(MR); - PendingObjs[Key] = std::move(*DebugObj); - } + if (*DebugObj != nullptr) + PendingObjs[&MR] = std::move(*DebugObj); } else { ES.reportError(DebugObj.takeError()); } @@ -428,7 +427,7 @@ PassConfiguration &PassConfig) { // Not all link artifacts have associated debug objects. std::lock_guard Lock(PendingObjsLock); - auto It = PendingObjs.find(getResourceKey(MR)); + auto It = PendingObjs.find(&MR); if (It == PendingObjs.end()) return; @@ -446,10 +445,8 @@ Error DebugObjectManagerPlugin::notifyEmitted( MaterializationResponsibility &MR) { - ResourceKey Key = getResourceKey(MR); - std::lock_guard Lock(PendingObjsLock); - auto It = PendingObjs.find(Key); + auto It = PendingObjs.find(&MR); if (It == PendingObjs.end()) return Error::success(); @@ -467,7 +464,7 @@ // the raw pointer in the continuation function, which re-owns it immediately. if (UnownedDebugObj) UnownedDebugObj->finalizeAsync( - [this, Key, UnownedDebugObj, + [this, UnownedDebugObj, &MR, &FinalizePromise](Expected TargetMem) { std::unique_ptr ReownedDebugObj(UnownedDebugObj); if (!TargetMem) { @@ -483,6 +480,7 @@ // materialization can finish. FinalizePromise.set_value(Error::success()); + ResourceKey Key = getResourceKey(MR); std::lock_guard Lock(RegisteredObjsLock); RegisteredObjs[Key].push_back(std::move(ReownedDebugObj)); }); @@ -493,33 +491,22 @@ Error DebugObjectManagerPlugin::notifyFailed( MaterializationResponsibility &MR) { std::lock_guard Lock(PendingObjsLock); - PendingObjs.erase(getResourceKey(MR)); + PendingObjs.erase(&MR); return Error::success(); } void DebugObjectManagerPlugin::notifyTransferringResources(ResourceKey DstKey, ResourceKey SrcKey) { - { - std::lock_guard Lock(RegisteredObjsLock); - auto SrcIt = RegisteredObjs.find(SrcKey); - if (SrcIt != RegisteredObjs.end()) { - // Resources from distinct MaterializationResponsibilitys can get merged - // after emission, so we can have multiple debug objects per resource key. - for (std::unique_ptr &DebugObj : SrcIt->second) - RegisteredObjs[DstKey].push_back(std::move(DebugObj)); - RegisteredObjs.erase(SrcIt); - } - } - { - std::lock_guard Lock(PendingObjsLock); - auto SrcIt = PendingObjs.find(SrcKey); - if (SrcIt != PendingObjs.end()) { - assert(PendingObjs.count(DstKey) == 0 && - "Cannot have more than one pending debug object per " - "MaterializationResponsibility"); - PendingObjs[DstKey] = std::move(SrcIt->second); - PendingObjs.erase(SrcIt); - } + // Debug objects are stored by ResourceKey only after registration. + // Thus, pending objects don't need to be updated here. + std::lock_guard Lock(RegisteredObjsLock); + auto SrcIt = RegisteredObjs.find(SrcKey); + if (SrcIt != RegisteredObjs.end()) { + // Resources from distinct MaterializationResponsibilitys can get merged + // after emission, so we can have multiple debug objects per resource key. + for (std::unique_ptr &DebugObj : SrcIt->second) + RegisteredObjs[DstKey].push_back(std::move(DebugObj)); + RegisteredObjs.erase(SrcIt); } } @@ -529,8 +516,25 @@ RegisteredObjs.erase(K); // TODO: Implement unregister notifications. } - std::lock_guard Lock(PendingObjsLock); - PendingObjs.erase(K); + { + // Removing a resource also removes any MaterializationResponsibility that + // is still in-flight. If we have pending debug objects for such MRs, we + // will remove them here. + // FIXME: Getting the resource key from a MaterializationResponsibility is + // quite expensive. We lock the session mutex in each iteration of the first + // loop. For the moment this seems acceptable, because the number of pending + // debug objects will usually be small (i.e. zero) and resource removal + // tends to be on cold paths. + SmallVector PendingMRsForKey; + for (const auto &KV : PendingObjs) { + if (K == getResourceKey(*KV.first)) + PendingMRsForKey.push_back(KV.first); + } + + std::lock_guard Lock(PendingObjsLock); + for (MaterializationResponsibility *MR : PendingMRsForKey) + PendingObjs.erase(MR); + } return Error::success(); }