From 642d8931be779d048630bd90e8714ea1661520d5 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Fri, 24 Mar 2023 13:15:18 -0700 Subject: [PATCH] [Dependency Scanning] Isolate shared dependency scanner state Using mutual exclusion, ensuring that multiple threads executing dependency scans do not encounter data races on shared mutable state. There are two layers with shared state where we need to be careful: - `DependencyScanningTool`, as the main entity that scanning clients interact with. This tool instantiates compiler instances for individual scans, computing a scanning invocation hash. It needs to remember those instances for future use, and when creating instances it needs to reset LLVM argument processor's global state, meaning all uses of argument processing must be in a critical section. - `SwiftDependencyScanningService`, as the main cache where dependency scanning results are stored. Each individual scan instantiates a `ModuleDependenciesCache`, which uses the scanning service as the underlying storage. The services' storage is segmented to storing dependencies discovered in a scan with a given context hash, which means two different scanning invocations running at the same time will be accessing different locations in its storage, thus not requiring synchronization. But the service still has some shared state that must be protected, such as the collection of discovered source modules, and the map used to query context-hash-specific underlying cache storage. --- include/swift/AST/ModuleDependencies.h | 43 +++++---- .../DependencyScan/DependencyScanningTool.h | 3 + lib/AST/ModuleDependencies.cpp | 92 ++++++++++--------- lib/DependencyScan/DependencyScanningTool.cpp | 11 +++ .../ModuleDependencyCacheSerialization.cpp | 22 +++-- 5 files changed, 103 insertions(+), 68 deletions(-) diff --git a/include/swift/AST/ModuleDependencies.h b/include/swift/AST/ModuleDependencies.h index 66fbe9938c164..554d8ec758ac2 100644 --- a/include/swift/AST/ModuleDependencies.h +++ b/include/swift/AST/ModuleDependencies.h @@ -25,6 +25,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/StringSet.h" +#include "llvm/Support/Mutex.h" #include #include #include @@ -580,18 +581,20 @@ class SwiftDependencyScanningService { llvm::StringMap> ContextSpecificCacheMap; - /// The current context hash configuration - Optional CurrentContextHash; - /// The context hashes used by scanners using this cache, in the order in /// which they were used std::vector AllContextHashes; + /// Shared state mutual-exclusivity lock + llvm::sys::SmartMutex ScanningServiceGlobalLock; + /// Retrieve the dependencies map that corresponds to the given dependency /// kind. - ModuleNameToDependencyMap &getDependenciesMap(ModuleDependencyKind kind); + ModuleNameToDependencyMap &getDependenciesMap(ModuleDependencyKind kind, + StringRef scanContextHash); const ModuleNameToDependencyMap & - getDependenciesMap(ModuleDependencyKind kind) const; + getDependenciesMap(ModuleDependencyKind kind, + StringRef scanContextHash) const; public: SwiftDependencyScanningService(); @@ -613,8 +616,8 @@ class SwiftDependencyScanningService { return *SharedFilesystemCache; } - llvm::StringSet<>& getAlreadySeenClangModules() { - return getCurrentCache()->alreadySeenClangModules; + llvm::StringSet<>& getAlreadySeenClangModules(StringRef scanningContextHash) { + return getCacheForScanningContextHash(scanningContextHash)->alreadySeenClangModules; } /// Wrap the filesystem on the specified `CompilerInstance` with a @@ -630,7 +633,7 @@ class SwiftDependencyScanningService { /// Configure the current state of the cache to respond to queries /// for the specified scanning context hash. - void configureForContextHash(std::string scanningContextHash); + void configureForContextHash(StringRef scanningContextHash); /// Return context hashes of all scanner invocations that have used /// this cache instance. @@ -640,15 +643,12 @@ class SwiftDependencyScanningService { /// Whether we have cached dependency information for the given module. bool hasDependency(StringRef moduleName, - Optional kind) const; - - /// Return a pointer to the context-specific cache state of the current - /// scanning action. - ContextSpecificGlobalCacheState *getCurrentCache() const; + Optional kind, + StringRef scanContextHash) const; /// Return a pointer to the cache state of the specified context hash. ContextSpecificGlobalCacheState * - getCacheForScanningContextHash(StringRef scanningContextHash) const; + getCacheForScanningContextHash(StringRef scanContextHash) const; /// Look for source-based module dependency details Optional @@ -659,15 +659,22 @@ class SwiftDependencyScanningService { /// \returns the cached result, or \c None if there is no cached entry. Optional findDependency(StringRef moduleName, - Optional kind) const; + Optional kind, + StringRef scanContextHash) const; /// Record dependencies for the given module. const ModuleDependencyInfo *recordDependency(StringRef moduleName, - ModuleDependencyInfo dependencies); + ModuleDependencyInfo dependencies, + StringRef scanContextHash); + + /// Record source-module dependencies for the given module. + const ModuleDependencyInfo *recordSourceDependency(StringRef moduleName, + ModuleDependencyInfo dependencies); /// Update stored dependencies for the given module. const ModuleDependencyInfo *updateDependency(ModuleDependencyID moduleID, - ModuleDependencyInfo dependencies); + ModuleDependencyInfo dependencies, + StringRef scanContextHash); /// Reference the list of all module dependencies that are not source-based /// modules (i.e. interface dependencies, binary dependencies, clang @@ -727,7 +734,7 @@ class ModuleDependenciesCache { return clangScanningTool; } llvm::StringSet<>& getAlreadySeenClangModules() { - return globalScanningService.getAlreadySeenClangModules(); + return globalScanningService.getAlreadySeenClangModules(scannerContextHash); } /// Look for module dependencies for a module with the given name diff --git a/include/swift/DependencyScan/DependencyScanningTool.h b/include/swift/DependencyScan/DependencyScanningTool.h index 6792cde8b3ccc..2eef60866e72c 100644 --- a/include/swift/DependencyScan/DependencyScanningTool.h +++ b/include/swift/DependencyScan/DependencyScanningTool.h @@ -115,6 +115,9 @@ class DependencyScanningTool { /// command-line options specified in the batch scan input entry. std::unique_ptr VersionedPCMInstanceCacheCache; + /// Shared state mutual-exclusivity lock + llvm::sys::SmartMutex DependencyScanningToolStateLock; + /// A shared consumer that accumulates encountered diagnostics. DependencyScannerDiagnosticCollectingConsumer CDC; llvm::BumpPtrAllocator Alloc; diff --git a/lib/AST/ModuleDependencies.cpp b/lib/AST/ModuleDependencies.cpp index 9b6b8aaf9798f..dfcb0d44d098a 100644 --- a/lib/AST/ModuleDependencies.cpp +++ b/lib/AST/ModuleDependencies.cpp @@ -251,13 +251,6 @@ void SwiftDependencyScanningService::overlaySharedFilesystemCacheForCompilation( Instance.getSourceMgr().setFileSystem(depFS); } -SwiftDependencyScanningService::ContextSpecificGlobalCacheState * -SwiftDependencyScanningService::getCurrentCache() const { - assert(CurrentContextHash.has_value() && - "Global Module Dependencies Cache not configured with Triple."); - return getCacheForScanningContextHash(CurrentContextHash.value()); -} - SwiftDependencyScanningService::ContextSpecificGlobalCacheState * SwiftDependencyScanningService::getCacheForScanningContextHash(StringRef scanningContextHash) const { auto contextSpecificCache = ContextSpecificCacheMap.find(scanningContextHash); @@ -269,8 +262,8 @@ SwiftDependencyScanningService::getCacheForScanningContextHash(StringRef scannin const ModuleNameToDependencyMap & SwiftDependencyScanningService::getDependenciesMap( - ModuleDependencyKind kind) const { - auto contextSpecificCache = getCurrentCache(); + ModuleDependencyKind kind, StringRef scanContextHash) const { + auto contextSpecificCache = getCacheForScanningContextHash(scanContextHash); auto it = contextSpecificCache->ModuleDependenciesMap.find(kind); assert(it != contextSpecificCache->ModuleDependenciesMap.end() && "invalid dependency kind"); @@ -279,40 +272,38 @@ SwiftDependencyScanningService::getDependenciesMap( ModuleNameToDependencyMap & SwiftDependencyScanningService::getDependenciesMap( - ModuleDependencyKind kind) { - auto contextSpecificCache = getCurrentCache(); + ModuleDependencyKind kind, StringRef scanContextHash) { + llvm::sys::SmartScopedLock Lock(ScanningServiceGlobalLock); + auto contextSpecificCache = getCacheForScanningContextHash(scanContextHash); auto it = contextSpecificCache->ModuleDependenciesMap.find(kind); assert(it != contextSpecificCache->ModuleDependenciesMap.end() && "invalid dependency kind"); return it->second; } -void SwiftDependencyScanningService::configureForContextHash(std::string scanningContextHash) { +void SwiftDependencyScanningService::configureForContextHash(StringRef scanningContextHash) { auto knownContext = ContextSpecificCacheMap.find(scanningContextHash); - if (knownContext != ContextSpecificCacheMap.end()) { - // Set the current context and leave the rest as-is - CurrentContextHash = scanningContextHash; - } else { - // First time scanning with this triple, initialize target-specific state. + if (knownContext == ContextSpecificCacheMap.end()) { + // First time scanning with this context, initialize context-specific state. std::unique_ptr contextSpecificCache = std::make_unique(); for (auto kind = ModuleDependencyKind::FirstKind; kind != ModuleDependencyKind::LastKind; ++kind) { contextSpecificCache->ModuleDependenciesMap.insert({kind, ModuleNameToDependencyMap()}); } - - ContextSpecificCacheMap.insert({scanningContextHash, std::move(contextSpecificCache)}); - CurrentContextHash = scanningContextHash; - AllContextHashes.push_back(scanningContextHash); + llvm::sys::SmartScopedLock Lock(ScanningServiceGlobalLock); + ContextSpecificCacheMap.insert({scanningContextHash.str(), std::move(contextSpecificCache)}); + AllContextHashes.push_back(scanningContextHash.str()); } } Optional SwiftDependencyScanningService::findDependency( - StringRef moduleName, Optional kind) const { + StringRef moduleName, Optional kind, + StringRef scanningContextHash) const { if (!kind) { for (auto kind = ModuleDependencyKind::FirstKind; kind != ModuleDependencyKind::LastKind; ++kind) { - auto dep = findDependency(moduleName, kind); + auto dep = findDependency(moduleName, kind, scanningContextHash); if (dep.has_value()) return dep.value(); } @@ -324,7 +315,7 @@ Optional SwiftDependencyScanningService::findDepend return findSourceModuleDependency(moduleName); } - const auto &map = getDependenciesMap(kind.value()); + const auto &map = getDependenciesMap(kind.value(), scanningContextHash); auto known = map.find(moduleName); if (known != map.end()) return &(known->second); @@ -343,36 +334,47 @@ SwiftDependencyScanningService::findSourceModuleDependency( } bool SwiftDependencyScanningService::hasDependency( - StringRef moduleName, Optional kind) const { - return findDependency(moduleName, kind).has_value(); + StringRef moduleName, Optional kind, + StringRef scanContextHash) const { + return findDependency(moduleName, kind, scanContextHash).has_value(); } const ModuleDependencyInfo *SwiftDependencyScanningService::recordDependency( - StringRef moduleName, ModuleDependencyInfo dependencies) { + StringRef moduleName, ModuleDependencyInfo dependencies, + StringRef scanContextHash) { auto kind = dependencies.getKind(); // Source-based dependencies are recorded independently of the invocation's // target triple. - if (kind == swift::ModuleDependencyKind::SwiftSource) { - assert(SwiftSourceModuleDependenciesMap.count(moduleName) == 0 && - "Attempting to record duplicate SwiftSource dependency."); - SwiftSourceModuleDependenciesMap.insert( - {moduleName, std::move(dependencies)}); - AllSourceModules.push_back({moduleName.str(), kind}); - return &(SwiftSourceModuleDependenciesMap.find(moduleName)->second); - } + if (kind == swift::ModuleDependencyKind::SwiftSource) + return recordSourceDependency(moduleName, std::move(dependencies)); // All other dependencies are recorded according to the target triple of the // scanning invocation that discovers them. - auto &map = getDependenciesMap(kind); + auto &map = getDependenciesMap(kind, scanContextHash); map.insert({moduleName, dependencies}); return &(map[moduleName]); } +const ModuleDependencyInfo *SwiftDependencyScanningService::recordSourceDependency( + StringRef moduleName, ModuleDependencyInfo dependencies) { + llvm::sys::SmartScopedLock Lock(ScanningServiceGlobalLock); + auto kind = dependencies.getKind(); + assert(kind == swift::ModuleDependencyKind::SwiftSource && "Expected source module dependncy info"); + assert(SwiftSourceModuleDependenciesMap.count(moduleName) == 0 && + "Attempting to record duplicate SwiftSource dependency."); + SwiftSourceModuleDependenciesMap.insert( + {moduleName, std::move(dependencies)}); + AllSourceModules.push_back({moduleName.str(), kind}); + return &(SwiftSourceModuleDependenciesMap.find(moduleName)->second); +} + const ModuleDependencyInfo *SwiftDependencyScanningService::updateDependency( - ModuleDependencyID moduleID, ModuleDependencyInfo dependencies) { + ModuleDependencyID moduleID, ModuleDependencyInfo dependencies, + StringRef scanningContextHash) { auto kind = dependencies.getKind(); // Source-based dependencies if (kind == swift::ModuleDependencyKind::SwiftSource) { + llvm::sys::SmartScopedLock Lock(ScanningServiceGlobalLock); assert(SwiftSourceModuleDependenciesMap.count(moduleID.first) == 1 && "Attempting to update non-existing Swift Source dependency."); auto known = SwiftSourceModuleDependenciesMap.find(moduleID.first); @@ -380,7 +382,7 @@ const ModuleDependencyInfo *SwiftDependencyScanningService::updateDependency( return &(known->second); } - auto &map = getDependenciesMap(moduleID.second); + auto &map = getDependenciesMap(moduleID.second, scanningContextHash); auto known = map.find(moduleID.first); assert(known != map.end() && "Not yet added to map"); known->second = std::move(dependencies); @@ -422,9 +424,10 @@ ModuleDependenciesCache::ModuleDependenciesCache( Optional ModuleDependenciesCache::findDependency( StringRef moduleName, Optional kind) const { - auto optionalDep = globalScanningService.findDependency(moduleName, kind); - // During a scan, only produce the cached source module info for the current module - // under scan. + auto optionalDep = globalScanningService.findDependency(moduleName, kind, + scannerContextHash); + // During a scan, only produce the cached source module info for the current + // module under scan. if (optionalDep.hasValue()) { auto dep = optionalDep.getValue(); if (dep->getAsSwiftSourceModule() && @@ -446,7 +449,8 @@ void ModuleDependenciesCache::recordDependency( StringRef moduleName, ModuleDependencyInfo dependencies) { auto dependenciesKind = dependencies.getKind(); const ModuleDependencyInfo *recordedDependencies = - globalScanningService.recordDependency(moduleName, dependencies); + globalScanningService.recordDependency(moduleName, dependencies, + scannerContextHash); auto &map = getDependencyReferencesMap(dependenciesKind); assert(map.count(moduleName) == 0 && "Already added to map"); @@ -455,7 +459,9 @@ void ModuleDependenciesCache::recordDependency( void ModuleDependenciesCache::updateDependency( ModuleDependencyID moduleID, ModuleDependencyInfo dependencyInfo) { - const ModuleDependencyInfo *updatedDependencies = globalScanningService.updateDependency(moduleID, dependencyInfo); + const ModuleDependencyInfo *updatedDependencies = + globalScanningService.updateDependency(moduleID, dependencyInfo, + scannerContextHash); auto &map = getDependencyReferencesMap(moduleID.second); auto known = map.find(moduleID.first); if (known != map.end()) diff --git a/lib/DependencyScan/DependencyScanningTool.cpp b/lib/DependencyScan/DependencyScanningTool.cpp index dd9d4a3b752a1..cc812e4c3a954 100644 --- a/lib/DependencyScan/DependencyScanningTool.cpp +++ b/lib/DependencyScan/DependencyScanningTool.cpp @@ -26,8 +26,13 @@ namespace swift { namespace dependencies { +// Global mutex for target info queries since they are executed separately . +llvm::sys::SmartMutex TargetInfoMutex; + llvm::ErrorOr getTargetInfo(ArrayRef Command, const char *main_executable_path) { + llvm::sys::SmartScopedLock Lock(TargetInfoMutex); + // We must reset option occurrences because we are handling an unrelated // command-line to those possibly parsed before using the same tool. // We must do so because LLVM options parsing is done using a managed @@ -178,6 +183,7 @@ DependencyScanningTool::getDependencies( } void DependencyScanningTool::serializeCache(llvm::StringRef path) { + llvm::sys::SmartScopedLock Lock(DependencyScanningToolStateLock); SourceManager SM; DiagnosticEngine Diags(SM); Diags.addConsumer(CDC); @@ -186,6 +192,7 @@ void DependencyScanningTool::serializeCache(llvm::StringRef path) { } bool DependencyScanningTool::loadCache(llvm::StringRef path) { + llvm::sys::SmartScopedLock Lock(DependencyScanningToolStateLock); SourceManager SM; DiagnosticEngine Diags(SM); Diags.addConsumer(CDC); @@ -210,6 +217,10 @@ void DependencyScanningTool::resetDiagnostics() { llvm::ErrorOr> DependencyScanningTool::initScannerForAction( ArrayRef Command) { + // The remainder of this method operates on shared state in the + // scanning service and global LLVM state with: + // llvm::cl::ResetAllOptionOccurrences + llvm::sys::SmartScopedLock Lock(DependencyScanningToolStateLock); auto instanceOrErr = initCompilerInstanceForScan(Command); if (instanceOrErr.getError()) return instanceOrErr; diff --git a/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp b/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp index f9e749c04a4f7..84bdf8d64f57a 100644 --- a/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp +++ b/lib/DependencyScan/ModuleDependencyCacheSerialization.cpp @@ -319,7 +319,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi for (const auto &mod : *bridgingModuleDeps) moduleDep.addBridgingModuleDependency(mod, alreadyAdded); - cache.recordDependency(currentModuleName, std::move(moduleDep)); + cache.recordDependency(currentModuleName, std::move(moduleDep), + getContextHash()); hasCurrentModule = false; break; } @@ -385,7 +386,7 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi for (const auto &mod : *bridgingModuleDeps) moduleDep.addBridgingModuleDependency(mod, alreadyAdded); - cache.recordDependency(currentModuleName, std::move(moduleDep)); + cache.recordSourceDependency(currentModuleName, std::move(moduleDep)); hasCurrentModule = false; break; } @@ -419,7 +420,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi for (const auto &moduleName : *currentModuleImports) moduleDep.addModuleImport(moduleName); - cache.recordDependency(currentModuleName, std::move(moduleDep)); + cache.recordDependency(currentModuleName, std::move(moduleDep), + getContextHash()); hasCurrentModule = false; break; } @@ -451,7 +453,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi for (const auto &moduleName : *currentModuleImports) moduleDep.addModuleImport(moduleName); - cache.recordDependency(currentModuleName, std::move(moduleDep)); + cache.recordDependency(currentModuleName, std::move(moduleDep), + getContextHash()); hasCurrentModule = false; break; } @@ -494,7 +497,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi for (const auto &moduleName : *currentModuleImports) moduleDep.addModuleImport(moduleName); - cache.recordDependency(currentModuleName, std::move(moduleDep)); + cache.recordDependency(currentModuleName, std::move(moduleDep), + getContextHash()); hasCurrentModule = false; break; } @@ -1018,7 +1022,9 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays( for (auto &contextHash : cache.getAllContextHashes()) { addIdentifier(contextHash); for (auto &moduleID : cache.getAllNonSourceModules(contextHash)) { - auto optionalDependencyInfo = cache.findDependency(moduleID.first, moduleID.second); + auto optionalDependencyInfo = cache.findDependency(moduleID.first, + moduleID.second, + contextHash); assert(optionalDependencyInfo.has_value() && "Expected dependency info."); auto dependencyInfo = optionalDependencyInfo.value(); // Add the module's name @@ -1140,7 +1146,9 @@ void ModuleDependenciesCacheSerializer::writeInterModuleDependenciesCache( // has been used with for (auto &contextHash : cache.getAllContextHashes()) { for (auto &moduleID : cache.getAllNonSourceModules(contextHash)) { - auto dependencyInfo = cache.findDependency(moduleID.first, moduleID.second); + auto dependencyInfo = cache.findDependency(moduleID.first, + moduleID.second, + contextHash); assert(dependencyInfo.has_value() && "Expected dependency info."); writeModuleInfo(moduleID, contextHash, **dependencyInfo); }