Skip to content

Commit 33e09f9

Browse files
artemcmartemcm
authored
Merge pull request #70211 from artemcm/ParallelScanFixes
[Dependency Scanning] Remove/move mutable state from parallel scanner workers
2 parents 406d533 + 674dfb3 commit 33e09f9

File tree

13 files changed

+48
-53
lines changed

13 files changed

+48
-53
lines changed

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -915,10 +915,6 @@ class SwiftDependencyScanningService {
915915
return Mapper->mapToString(Path);
916916
}
917917

918-
/// Wrap the filesystem on the specified `CompilerInstance` with a
919-
/// caching `DependencyScanningWorkerFilesystem`
920-
void overlaySharedFilesystemCacheForCompilation(CompilerInstance &Instance);
921-
922918
/// Setup caching service.
923919
bool setupCachingDependencyScanningService(CompilerInstance &Instance);
924920
private:

‎include/swift/AST/ModuleLoader.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ class ModuleLoader {
341341
/// Retrieve the dependencies for the given, named module, or \c None
342342
/// if no such module exists.
343343
virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
344-
getModuleDependencies(StringRef moduleName,
344+
getModuleDependencies(Identifier moduleName,
345345
StringRef moduleOutputPath,
346346
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
347347
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,

‎include/swift/ClangImporter/ClangImporter.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ class ClangImporter final : public ClangModuleLoader {
440440
StringRef moduleOutputPath, RemapPathCallback remapPath = nullptr);
441441

442442
llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
443-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
443+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
444444
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
445445
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
446446
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎include/swift/DependencyScan/ModuleDependencyScanner.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "swift/AST/ASTContext.h"
15+
#include "swift/AST/Identifier.h"
1516
#include "swift/AST/ModuleDependencies.h"
1617
#include "swift/Frontend/ModuleInterfaceLoader.h"
1718
#include "swift/Serialization/SerializedModuleLoader.h"
@@ -36,18 +37,18 @@ class ModuleDependencyScanningWorker {
3637
private:
3738
/// Retrieve the module dependencies for the module with the given name.
3839
ModuleDependencyVector
39-
scanFilesystemForModuleDependency(StringRef moduleName,
40+
scanFilesystemForModuleDependency(Identifier moduleName,
4041
const ModuleDependenciesCache &cache,
4142
bool isTestableImport = false);
4243

4344
/// Retrieve the module dependencies for the Clang module with the given name.
4445
ModuleDependencyVector
45-
scanFilesystemForClangModuleDependency(StringRef moduleName,
46+
scanFilesystemForClangModuleDependency(Identifier moduleName,
4647
const ModuleDependenciesCache &cache);
4748

4849
/// Retrieve the module dependencies for the Swift module with the given name.
4950
ModuleDependencyVector
50-
scanFilesystemForSwiftModuleDependency(StringRef moduleName,
51+
scanFilesystemForSwiftModuleDependency(Identifier moduleName,
5152
const ModuleDependenciesCache &cache);
5253

5354
// An AST delegate for interface scanning.
@@ -135,6 +136,8 @@ class ModuleDependencyScanner {
135136
template <typename Function, typename... Args>
136137
auto withDependencyScanningWorker(Function &&F, Args &&...ArgList);
137138

139+
Identifier getModuleImportIdentifier(StringRef moduleName);
140+
138141
private:
139142
const CompilerInvocation &ScanCompilerInvocation;
140143
ASTContext &ScanASTContext;

‎include/swift/Sema/SourceLoader.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class SourceLoader : public ModuleLoader {
9898
}
9999

100100
llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
101-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
101+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
102102
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
103103
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
104104
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎include/swift/Serialization/SerializedModuleLoader.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ class SerializedModuleLoaderBase : public ModuleLoader {
250250
virtual void verifyAllModules() override;
251251

252252
virtual llvm::SmallVector<std::pair<ModuleDependencyID, ModuleDependencyInfo>, 1>
253-
getModuleDependencies(StringRef moduleName, StringRef moduleOutputPath,
253+
getModuleDependencies(Identifier moduleName, StringRef moduleOutputPath,
254254
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
255255
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
256256
clang::tooling::dependencies::DependencyScanningTool &clangScanningTool,

‎lib/AST/ModuleDependencies.cpp‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -480,17 +480,6 @@ SwiftDependencyTracker::createTreeFromDependencies() {
480480
});
481481
}
482482

483-
void SwiftDependencyScanningService::overlaySharedFilesystemCacheForCompilation(
484-
CompilerInstance &Instance) {
485-
auto existingFS = Instance.getSourceMgr().getFileSystem();
486-
llvm::IntrusiveRefCntPtr<
487-
clang::tooling::dependencies::DependencyScanningWorkerFilesystem>
488-
depFS =
489-
new clang::tooling::dependencies::DependencyScanningWorkerFilesystem(
490-
getSharedFilesystemCache(), existingFS);
491-
Instance.getSourceMgr().setFileSystem(depFS);
492-
}
493-
494483
bool SwiftDependencyScanningService::setupCachingDependencyScanningService(
495484
CompilerInstance &Instance) {
496485
if (!Instance.getInvocation().getFrontendOptions().EnableCaching)

‎lib/AST/SearchPathOptions.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,12 @@ ModuleSearchPathLookup::searchPathsContainingFile(
130130
llvm::SmallSet<std::pair<ModuleSearchPathKind, unsigned>, 4> ResultIds;
131131

132132
for (auto &Filename : Filenames) {
133-
for (auto &Entry : LookupTable.lookup(Filename)) {
134-
if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
135-
.second) {
136-
Result.push_back(Entry.get());
133+
if (LookupTable.contains(Filename)) {
134+
for (auto &Entry : LookupTable.at(Filename)) {
135+
if (ResultIds.insert(std::make_pair(Entry->getKind(), Entry->getIndex()))
136+
.second) {
137+
Result.push_back(Entry.get());
138+
}
137139
}
138140
}
139141
}

‎lib/ClangImporter/ClangModuleDependencyScanner.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ computeClangWorkingDirectory(const std::vector<std::string> &commandLineArgs,
407407
}
408408

409409
ModuleDependencyVector
410-
ClangImporter::getModuleDependencies(StringRef moduleName,
410+
ClangImporter::getModuleDependencies(Identifier moduleName,
411411
StringRef moduleOutputPath,
412412
llvm::IntrusiveRefCntPtr<llvm::cas::CachingOnDiskFileSystem> CacheFS,
413413
const llvm::DenseSet<clang::tooling::dependencies::ModuleID> &alreadySeenClangModules,
@@ -435,14 +435,14 @@ ClangImporter::getModuleDependencies(StringRef moduleName,
435435

436436
auto clangModuleDependencies =
437437
clangScanningTool.getModuleDependencies(
438-
moduleName, commandLineArgs, workingDir,
438+
moduleName.str(), commandLineArgs, workingDir,
439439
alreadySeenClangModules, lookupModuleOutput);
440440
if (!clangModuleDependencies) {
441441
auto errorStr = toString(clangModuleDependencies.takeError());
442442
// We ignore the "module 'foo' not found" error, the Swift dependency
443443
// scanner will report such an error only if all of the module loaders
444444
// fail as well.
445-
if (errorStr.find("fatal error: module '" + moduleName.str() +
445+
if (errorStr.find("fatal error: module '" + moduleName.str().str() +
446446
"' not found") == std::string::npos)
447447
ctx.Diags.diagnose(SourceLoc(), diag::clang_dependency_scan_error,
448448
errorStr);

‎lib/DependencyScan/ModuleDependencyScanner.cpp‎

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ ModuleDependencyScanningWorker::ModuleDependencyScanningWorker(
179179

180180
ModuleDependencyVector
181181
ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
182-
StringRef moduleName, const ModuleDependenciesCache &cache,
182+
Identifier moduleName, const ModuleDependenciesCache &cache,
183183
bool isTestableImport) {
184184
// First query a Swift module, otherwise lookup a Clang module
185185
ModuleDependencyVector moduleDependencies =
@@ -203,7 +203,7 @@ ModuleDependencyScanningWorker::scanFilesystemForModuleDependency(
203203

204204
ModuleDependencyVector
205205
ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency(
206-
StringRef moduleName, const ModuleDependenciesCache &cache) {
206+
Identifier moduleName, const ModuleDependenciesCache &cache) {
207207
return swiftScannerModuleLoader->getModuleDependencies(
208208
moduleName, cache.getModuleOutputPath(),
209209
cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
@@ -213,7 +213,7 @@ ModuleDependencyScanningWorker::scanFilesystemForSwiftModuleDependency(
213213

214214
ModuleDependencyVector
215215
ModuleDependencyScanningWorker::scanFilesystemForClangModuleDependency(
216-
StringRef moduleName, const ModuleDependenciesCache &cache) {
216+
Identifier moduleName, const ModuleDependenciesCache &cache) {
217217
return clangScannerModuleLoader->getModuleDependencies(
218218
moduleName, cache.getModuleOutputPath(),
219219
cache.getScanService().getCachingFS(), cache.getAlreadySeenClangModules(),
@@ -252,6 +252,10 @@ auto ModuleDependencyScanner::withDependencyScanningWorker(Function &&F,
252252
return result;
253253
}
254254

255+
Identifier ModuleDependencyScanner::getModuleImportIdentifier(StringRef moduleName) {
256+
return ScanASTContext.getIdentifier(moduleName);
257+
}
258+
255259
ModuleDependencyScanner::ModuleDependencyScanner(
256260
SwiftDependencyScanningService &ScanningService,
257261
const CompilerInvocation &ScanCompilerInvocation,
@@ -443,10 +447,11 @@ ModuleDependencyScanner::getNamedClangModuleDependencyInfo(
443447
return found;
444448

445449
// Otherwise perform filesystem scan
450+
auto moduleIdentifier = getModuleImportIdentifier(moduleName);
446451
auto moduleDependencies = withDependencyScanningWorker(
447-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
452+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
448453
return ScanningWorker->scanFilesystemForClangModuleDependency(
449-
moduleName, cache);
454+
moduleIdentifier, cache);
450455
});
451456
if (moduleDependencies.empty())
452457
return llvm::None;
@@ -474,10 +479,11 @@ ModuleDependencyScanner::getNamedSwiftModuleDependencyInfo(
474479
return found;
475480

476481
// Otherwise perform filesystem scan
482+
auto moduleIdentifier = getModuleImportIdentifier(moduleName);
477483
auto moduleDependencies = withDependencyScanningWorker(
478-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
484+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
479485
return ScanningWorker->scanFilesystemForSwiftModuleDependency(
480-
moduleName, cache);
486+
moduleIdentifier, cache);
481487
});
482488
if (moduleDependencies.empty())
483489
return llvm::None;
@@ -552,8 +558,9 @@ void ModuleDependencyScanner::resolveImportDependencies(
552558
// A scanning task to query a module by-name. If the module already exists
553559
// in the cache, do nothing and return.
554560
auto scanForModuleDependency = [this, &cache, &moduleLookupResult](
555-
StringRef moduleName, bool onlyClangModule,
561+
Identifier moduleIdentifier, bool onlyClangModule,
556562
bool isTestable) {
563+
auto moduleName = moduleIdentifier.str();
557564
// If this is already in the cache, no work to do here
558565
if (onlyClangModule) {
559566
if (cache.hasDependency(moduleName, ModuleDependencyKind::Clang))
@@ -564,13 +571,13 @@ void ModuleDependencyScanner::resolveImportDependencies(
564571
}
565572

566573
auto moduleDependencies = withDependencyScanningWorker(
567-
[&cache, moduleName, onlyClangModule,
574+
[&cache, moduleIdentifier, onlyClangModule,
568575
isTestable](ModuleDependencyScanningWorker *ScanningWorker) {
569576
return onlyClangModule
570577
? ScanningWorker->scanFilesystemForClangModuleDependency(
571-
moduleName, cache)
578+
moduleIdentifier, cache)
572579
: ScanningWorker->scanFilesystemForModuleDependency(
573-
moduleName, cache, isTestable);
580+
moduleIdentifier, cache, isTestable);
574581
});
575582
moduleLookupResult.insert_or_assign(moduleName, moduleDependencies);
576583
};
@@ -579,14 +586,14 @@ void ModuleDependencyScanner::resolveImportDependencies(
579586
for (const auto &dependsOn : moduleDependencyInfo->getModuleImports()) {
580587
bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
581588
bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
582-
ScanningThreadPool.async(scanForModuleDependency, dependsOn,
589+
ScanningThreadPool.async(scanForModuleDependency, getModuleImportIdentifier(dependsOn),
583590
underlyingClangModuleLookup, isTestable);
584591
}
585592
for (const auto &dependsOn :
586593
moduleDependencyInfo->getOptionalModuleImports()) {
587594
bool underlyingClangModuleLookup = moduleID.ModuleName == dependsOn;
588595
bool isTestable = moduleDependencyInfo->isTestableImport(dependsOn);
589-
ScanningThreadPool.async(scanForModuleDependency, dependsOn,
596+
ScanningThreadPool.async(scanForModuleDependency, getModuleImportIdentifier(dependsOn),
590597
underlyingClangModuleLookup, isTestable);
591598
}
592599
ScanningThreadPool.wait();
@@ -726,23 +733,24 @@ void ModuleDependencyScanner::resolveSwiftOverlayDependencies(
726733
// A scanning task to query a Swift module by-name. If the module already
727734
// exists in the cache, do nothing and return.
728735
auto scanForSwiftDependency = [this, &cache, &swiftOverlayLookupResult](
729-
StringRef moduleName) {
736+
Identifier moduleIdentifier) {
737+
auto moduleName = moduleIdentifier.str();
730738
if (cache.hasDependency(moduleName, ModuleDependencyKind::SwiftInterface) ||
731739
cache.hasDependency(moduleName, ModuleDependencyKind::SwiftBinary) ||
732740
cache.hasDependency(moduleName, ModuleDependencyKind::SwiftPlaceholder))
733741
return;
734742

735743
auto moduleDependencies = withDependencyScanningWorker(
736-
[&cache, moduleName](ModuleDependencyScanningWorker *ScanningWorker) {
737-
return ScanningWorker->scanFilesystemForSwiftModuleDependency(moduleName,
744+
[&cache, moduleIdentifier](ModuleDependencyScanningWorker *ScanningWorker) {
745+
return ScanningWorker->scanFilesystemForSwiftModuleDependency(moduleIdentifier,
738746
cache);
739747
});
740748
swiftOverlayLookupResult.insert_or_assign(moduleName, moduleDependencies);
741749
};
742750

743751
// Enque asynchronous lookup tasks
744752
for (const auto &clangDep : clangDependencies)
745-
ScanningThreadPool.async(scanForSwiftDependency, clangDep);
753+
ScanningThreadPool.async(scanForSwiftDependency, getModuleImportIdentifier(clangDep));
746754
ScanningThreadPool.wait();
747755

748756
// Aggregate both previously-cached and freshly-scanned module results

0 commit comments

Comments
 (0)