Skip to content

Commit 359a23c

Browse files
artemcmartemcm
committed
[Dependency Scanning] Do not track source module dependencies separately in SwiftDependencyScanningService
Instead, treat them like any other module that is specific to the scanning context hash of the scan it originates from. Otherwise we may actually have simultaneous scans happening for the same source module but with different context hashes, and the current scheme leads to collisions.
1 parent 9f14e24 commit 359a23c

File tree

3 files changed

+21
-115
lines changed

3 files changed

+21
-115
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -567,14 +567,6 @@ class SwiftDependencyScanningService {
567567
clang::tooling::dependencies::DependencyScanningFilesystemSharedCache>
568568
SharedFilesystemCache;
569569

570-
/// All cached Swift source module dependencies, in the order in which they
571-
/// were encountered
572-
std::vector<ModuleDependencyID> AllSourceModules;
573-
574-
/// Dependencies for all Swift source-based modules discovered. Each one is
575-
/// the main module of a prior invocation of the scanner.
576-
ModuleNameToDependencyMap SwiftSourceModuleDependenciesMap;
577-
578570
/// A map from a String representing the target triple of a scanner invocation
579571
/// to the corresponding cached dependencies discovered so far when using this
580572
/// triple.
@@ -650,10 +642,6 @@ class SwiftDependencyScanningService {
650642
ContextSpecificGlobalCacheState *
651643
getCacheForScanningContextHash(StringRef scanContextHash) const;
652644

653-
/// Look for source-based module dependency details
654-
Optional<const ModuleDependencyInfo*>
655-
findSourceModuleDependency(StringRef moduleName) const;
656-
657645
/// Look for module dependencies for a module with the given name
658646
///
659647
/// \returns the cached result, or \c None if there is no cached entry.
@@ -667,29 +655,18 @@ class SwiftDependencyScanningService {
667655
ModuleDependencyInfo dependencies,
668656
StringRef scanContextHash);
669657

670-
/// Record source-module dependencies for the given module.
671-
const ModuleDependencyInfo *recordSourceDependency(StringRef moduleName,
672-
ModuleDependencyInfo dependencies);
673-
674658
/// Update stored dependencies for the given module.
675659
const ModuleDependencyInfo *updateDependency(ModuleDependencyID moduleID,
676660
ModuleDependencyInfo dependencies,
677661
StringRef scanContextHash);
678662

679-
/// Reference the list of all module dependencies that are not source-based
680-
/// modules (i.e. interface dependencies, binary dependencies, clang
681-
/// dependencies).
663+
/// Reference the list of all module dependency infos for a given scanning context
682664
const std::vector<ModuleDependencyID> &
683-
getAllNonSourceModules(StringRef scanningContextHash) const {
665+
getAllModules(StringRef scanningContextHash) const {
684666
auto contextSpecificCache =
685667
getCacheForScanningContextHash(scanningContextHash);
686668
return contextSpecificCache->AllModules;
687669
}
688-
689-
/// Return the list of all source-based modules discovered by this cache
690-
const std::vector<ModuleDependencyID> &getAllSourceModules() const {
691-
return AllSourceModules;
692-
}
693670
};
694671

695672
// MARK: ModuleDependenciesCache
@@ -756,10 +733,6 @@ class ModuleDependenciesCache {
756733
/// to a kind-qualified set of module IDs.
757734
void resolveDependencyImports(ModuleDependencyID moduleID,
758735
const std::vector<ModuleDependencyID> &dependencyIDs);
759-
760-
const std::vector<ModuleDependencyID> &getAllSourceModules() const {
761-
return globalScanningService.getAllSourceModules();
762-
}
763736

764737
StringRef getMainModuleName() const {
765738
return mainScanModuleName;

‎lib/AST/ModuleDependencies.cpp

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -311,10 +311,6 @@ Optional<const ModuleDependencyInfo*> SwiftDependencyScanningService::findDepend
311311
}
312312

313313
assert(kind.has_value() && "Expected dependencies kind for lookup.");
314-
if (kind.value() == swift::ModuleDependencyKind::SwiftSource) {
315-
return findSourceModuleDependency(moduleName);
316-
}
317-
318314
const auto &map = getDependenciesMap(kind.value(), scanningContextHash);
319315
auto known = map.find(moduleName);
320316
if (known != map.end())
@@ -323,16 +319,6 @@ Optional<const ModuleDependencyInfo*> SwiftDependencyScanningService::findDepend
323319
return None;
324320
}
325321

326-
Optional<const ModuleDependencyInfo*>
327-
SwiftDependencyScanningService::findSourceModuleDependency(
328-
StringRef moduleName) const {
329-
auto known = SwiftSourceModuleDependenciesMap.find(moduleName);
330-
if (known != SwiftSourceModuleDependenciesMap.end())
331-
return &(known->second);
332-
else
333-
return None;
334-
}
335-
336322
bool SwiftDependencyScanningService::hasDependency(
337323
StringRef moduleName, Optional<ModuleDependencyKind> kind,
338324
StringRef scanContextHash) const {
@@ -343,45 +329,14 @@ const ModuleDependencyInfo *SwiftDependencyScanningService::recordDependency(
343329
StringRef moduleName, ModuleDependencyInfo dependencies,
344330
StringRef scanContextHash) {
345331
auto kind = dependencies.getKind();
346-
// Source-based dependencies are recorded independently of the invocation's
347-
// target triple.
348-
if (kind == swift::ModuleDependencyKind::SwiftSource)
349-
return recordSourceDependency(moduleName, std::move(dependencies));
350-
351-
// All other dependencies are recorded according to the target triple of the
352-
// scanning invocation that discovers them.
353332
auto &map = getDependenciesMap(kind, scanContextHash);
354333
map.insert({moduleName, dependencies});
355334
return &(map[moduleName]);
356335
}
357336

358-
const ModuleDependencyInfo *SwiftDependencyScanningService::recordSourceDependency(
359-
StringRef moduleName, ModuleDependencyInfo dependencies) {
360-
llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
361-
auto kind = dependencies.getKind();
362-
assert(kind == swift::ModuleDependencyKind::SwiftSource && "Expected source module dependncy info");
363-
assert(SwiftSourceModuleDependenciesMap.count(moduleName) == 0 &&
364-
"Attempting to record duplicate SwiftSource dependency.");
365-
SwiftSourceModuleDependenciesMap.insert(
366-
{moduleName, std::move(dependencies)});
367-
AllSourceModules.push_back({moduleName.str(), kind});
368-
return &(SwiftSourceModuleDependenciesMap.find(moduleName)->second);
369-
}
370-
371337
const ModuleDependencyInfo *SwiftDependencyScanningService::updateDependency(
372338
ModuleDependencyID moduleID, ModuleDependencyInfo dependencies,
373339
StringRef scanningContextHash) {
374-
auto kind = dependencies.getKind();
375-
// Source-based dependencies
376-
if (kind == swift::ModuleDependencyKind::SwiftSource) {
377-
llvm::sys::SmartScopedLock<true> Lock(ScanningServiceGlobalLock);
378-
assert(SwiftSourceModuleDependenciesMap.count(moduleID.first) == 1 &&
379-
"Attempting to update non-existing Swift Source dependency.");
380-
auto known = SwiftSourceModuleDependenciesMap.find(moduleID.first);
381-
known->second = std::move(dependencies);
382-
return &(known->second);
383-
}
384-
385340
auto &map = getDependenciesMap(moduleID.second, scanningContextHash);
386341
auto known = map.find(moduleID.first);
387342
assert(known != map.end() && "Not yet added to map");
@@ -451,7 +406,6 @@ void ModuleDependenciesCache::recordDependency(
451406
const ModuleDependencyInfo *recordedDependencies =
452407
globalScanningService.recordDependency(moduleName, dependencies,
453408
scannerContextHash);
454-
455409
auto &map = getDependencyReferencesMap(dependenciesKind);
456410
assert(map.count(moduleName) == 0 && "Already added to map");
457411
map.insert({moduleName, recordedDependencies});

‎lib/DependencyScan/ModuleDependencyCacheSerialization.cpp

Lines changed: 19 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,8 @@ bool ModuleDependenciesCacheDeserializer::readGraph(SwiftDependencyScanningServi
386386
for (const auto &mod : *bridgingModuleDeps)
387387
moduleDep.addBridgingModuleDependency(mod, alreadyAdded);
388388

389-
cache.recordSourceDependency(currentModuleName, std::move(moduleDep));
389+
cache.recordDependency(currentModuleName, std::move(moduleDep),
390+
getContextHash());
390391
hasCurrentModule = false;
391392
break;
392393
}
@@ -991,37 +992,9 @@ unsigned ModuleDependenciesCacheSerializer::getArrayID(ModuleDependencyID module
991992

992993
void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
993994
const SwiftDependencyScanningService &cache) {
994-
for (auto &moduleID : cache.getAllSourceModules()) {
995-
assert(moduleID.second == ModuleDependencyKind::SwiftSource &&
996-
"Expected source-based dependency");
997-
auto optionalDependencyInfo =
998-
cache.findSourceModuleDependency(moduleID.first);
999-
assert(optionalDependencyInfo.has_value() && "Expected dependency info.");
1000-
auto dependencyInfo = optionalDependencyInfo.value();
1001-
// Add the module's name
1002-
addIdentifier(moduleID.first);
1003-
// Add the module's dependencies
1004-
addStringArray(moduleID, ModuleIdentifierArrayKind::DependencyImports,
1005-
dependencyInfo->getModuleImports());
1006-
addDependencyIDArray(moduleID, ModuleIdentifierArrayKind::QualifiedModuleDependencyIDs,
1007-
dependencyInfo->getModuleDependencies());
1008-
auto swiftSourceDeps = dependencyInfo->getAsSwiftSourceModule();
1009-
assert(swiftSourceDeps);
1010-
addStringArray(moduleID, ModuleIdentifierArrayKind::ExtraPCMArgs,
1011-
swiftSourceDeps->textualModuleDetails.extraPCMArgs);
1012-
if (swiftSourceDeps->textualModuleDetails.bridgingHeaderFile.has_value())
1013-
addIdentifier(swiftSourceDeps->textualModuleDetails.bridgingHeaderFile.value());
1014-
addStringArray(moduleID, ModuleIdentifierArrayKind::SourceFiles,
1015-
swiftSourceDeps->sourceFiles);
1016-
addStringArray(moduleID, ModuleIdentifierArrayKind::BridgingSourceFiles,
1017-
swiftSourceDeps->textualModuleDetails.bridgingSourceFiles);
1018-
addStringArray(moduleID, ModuleIdentifierArrayKind::BridgingModuleDependencies,
1019-
swiftSourceDeps->textualModuleDetails.bridgingModuleDependencies);
1020-
}
1021-
1022995
for (auto &contextHash : cache.getAllContextHashes()) {
1023996
addIdentifier(contextHash);
1024-
for (auto &moduleID : cache.getAllNonSourceModules(contextHash)) {
997+
for (auto &moduleID : cache.getAllModules(contextHash)) {
1025998
auto optionalDependencyInfo = cache.findDependency(moduleID.first,
1026999
moduleID.second,
10271000
contextHash);
@@ -1078,6 +1051,21 @@ void ModuleDependenciesCacheSerializer::collectStringsAndArrays(
10781051
addIdentifier(swiftPHDeps->sourceInfoPath);
10791052
break;
10801053
}
1054+
case swift::ModuleDependencyKind::SwiftSource: {
1055+
auto swiftSourceDeps = dependencyInfo->getAsSwiftSourceModule();
1056+
assert(swiftSourceDeps);
1057+
addStringArray(moduleID, ModuleIdentifierArrayKind::ExtraPCMArgs,
1058+
swiftSourceDeps->textualModuleDetails.extraPCMArgs);
1059+
if (swiftSourceDeps->textualModuleDetails.bridgingHeaderFile.has_value())
1060+
addIdentifier(swiftSourceDeps->textualModuleDetails.bridgingHeaderFile.value());
1061+
addStringArray(moduleID, ModuleIdentifierArrayKind::SourceFiles,
1062+
swiftSourceDeps->sourceFiles);
1063+
addStringArray(moduleID, ModuleIdentifierArrayKind::BridgingSourceFiles,
1064+
swiftSourceDeps->textualModuleDetails.bridgingSourceFiles);
1065+
addStringArray(moduleID, ModuleIdentifierArrayKind::BridgingModuleDependencies,
1066+
swiftSourceDeps->textualModuleDetails.bridgingModuleDependencies);
1067+
break;
1068+
}
10811069
case swift::ModuleDependencyKind::Clang: {
10821070
auto clangDeps = dependencyInfo->getAsClangModule();
10831071
assert(clangDeps);
@@ -1135,17 +1123,8 @@ void ModuleDependenciesCacheSerializer::writeInterModuleDependenciesCache(
11351123
writeArraysOfIdentifiers();
11361124

11371125
// Write the core graph
1138-
// First, write the source modules we've encountered
1139-
for (auto &moduleID : cache.getAllSourceModules()) {
1140-
auto dependencyInfo = cache.findSourceModuleDependency(moduleID.first);
1141-
assert(dependencyInfo.has_value() && "Expected dependency info.");
1142-
writeModuleInfo(moduleID, llvm::Optional<std::string>(), **dependencyInfo);
1143-
}
1144-
1145-
// Write all non-source modules, for each of the context hashes this scanner
1146-
// has been used with
11471126
for (auto &contextHash : cache.getAllContextHashes()) {
1148-
for (auto &moduleID : cache.getAllNonSourceModules(contextHash)) {
1127+
for (auto &moduleID : cache.getAllModules(contextHash)) {
11491128
auto dependencyInfo = cache.findDependency(moduleID.first,
11501129
moduleID.second,
11511130
contextHash);

0 commit comments

Comments
 (0)