Skip to content

Commit 5334af0

Browse files
committed
[Dependency Scanning] Refactor the scanner to resolve unqualified module imports
This changes the scanner's behavior to "resolve" a discovered module's dependencies to a set of Module IDs: module name + module kind (swift textual, swift binary, clang, etc.). The 'ModuleDependencyInfo' objects that are stored in the dependency scanner's cache now carry a set of kind-qualified ModuleIDs for their dependencies, in addition to unqualified imported module names of their dependencies. Previously, the scanner's internal state would cache a module dependnecy as having its own set of dependencies which were stored as names of imported modules. This led to a design where any time we needed to process the dependency downstream from its discovery (e.g. cycle detection, graph construction), we had to query the ASTContext to resolve this dependency's imports, which shouldn't be necessary. Now, upon discovery, we "resolve" a discovered dependency by executing a lookup for each of its imported module names (this operation happens regardless of this patch) and store a fully-resolved set of dependencies in the dependency module info. Moreover, looking up a given module dependency by name (via `ASTContext`'s `getModuleDependencies`) would result in iterating over the scanner's module "loaders" and querying each for the module name. The corresponding modules would then check the scanner's cache for a respective discovered module, and if no such module is found the "loader" would search the filesystem. This meant that in practice, we searched the filesystem on many occasions where we actually had cached the required dependency, as follows: Suppose we had previously discovered a Clang module "foo" and cached its dependency info. -> ASTContext.getModuleDependencies("foo") --> (1) Swift Module "Loader" checks caches for a Swift module "foo" and doesn't find one, so it searches the filesystem for "foo" and fails to find one. --> (2) Clang Module "Loader" checks caches for a Clang module "foo", finds one and returns it to the client. This means that we were always searching the filesystem in (1) even if we knew that to be futile. With this change, queries to `ASTContext`'s `getModuleDependencies` will always check all the caches first, and only delegate to the scanner "loaders" if no cached dependency is found. The loaders are then no longer in the business of checking the cached contents. To handle cases in the scanner where we must only lookup either a Swift-only module or a Clang-only module, this patch splits 'getModuleDependencies' into an alrady-existing 'getSwiftModuleDependencies' and a newly-added 'getClangModuleDependencies'.
1 parent a59db26 commit 5334af0

17 files changed

+634
-626
lines changed

include/swift/AST/ASTContext.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -991,16 +991,18 @@ class ASTContext final {
991991

992992
/// Retrieve the module dependencies for the module with the given name.
993993
///
994-
/// \param isUnderlyingClangModule When true, only look for a Clang module
995-
/// with the given name, ignoring any Swift modules.
996994
Optional<ModuleDependencyInfo> getModuleDependencies(
997995
StringRef moduleName,
998-
bool isUnderlyingClangModule,
999996
ModuleDependenciesCache &cache,
1000997
InterfaceSubContextDelegate &delegate,
1001-
bool cacheOnly = false,
1002998
llvm::Optional<std::pair<std::string, swift::ModuleDependencyKind>> dependencyOf = None);
1003999

1000+
/// Retrieve the module dependencies for the Clang module with the given name.
1001+
Optional<ModuleDependencyInfo> getClangModuleDependencies(
1002+
StringRef moduleName,
1003+
ModuleDependenciesCache &cache,
1004+
InterfaceSubContextDelegate &delegate);
1005+
10041006
/// Retrieve the module dependencies for the Swift module with the given name.
10051007
Optional<ModuleDependencyInfo> getSwiftModuleDependencies(
10061008
StringRef moduleName,

include/swift/AST/ModuleDependencies.h

Lines changed: 87 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,44 @@ enum class ModuleDependencyKind : int8_t {
7474
LastKind = SwiftPlaceholder + 1
7575
};
7676

77+
using ModuleDependencyID = std::pair<std::string, ModuleDependencyKind>;
78+
using ModuleDependencyIDSetVector =
79+
llvm::SetVector<ModuleDependencyID, std::vector<ModuleDependencyID>,
80+
std::set<ModuleDependencyID>>;
81+
7782
struct ModuleDependencyKindHash {
7883
std::size_t operator()(ModuleDependencyKind k) const {
7984
using UnderlyingType = std::underlying_type<ModuleDependencyKind>::type;
8085
return std::hash<UnderlyingType>{}(static_cast<UnderlyingType>(k));
8186
}
8287
};
8388

89+
namespace dependencies {
90+
std::string createEncodedModuleKindAndName(ModuleDependencyID id);
91+
}
92+
8493
/// Base class for the variant storage of ModuleDependencyInfo.
8594
///
8695
/// This class is mostly an implementation detail for \c ModuleDependencyInfo.
8796
class ModuleDependencyInfoStorageBase {
8897
public:
8998
const ModuleDependencyKind dependencyKind;
9099

91-
ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind)
92-
: dependencyKind(dependencyKind) { }
100+
ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind,
101+
bool resolved = false)
102+
: dependencyKind(dependencyKind), resolved(resolved) { }
93103

94104
virtual ModuleDependencyInfoStorageBase *clone() const = 0;
95105

96106
virtual ~ModuleDependencyInfoStorageBase();
97107

98108
/// The set of modules on which this module depends.
99-
std::vector<std::string> moduleDependencies;
109+
std::vector<std::string> moduleImports;
110+
111+
/// The set of modules on which this module depends, resolved
112+
/// to Module IDs, qualified by module kind: Swift, Clang, etc.
113+
ModuleDependencyIDSetVector resolvedModuleDependencies;
114+
bool resolved;
100115
};
101116

102117
struct CommonSwiftTextualModuleDependencyDetails {
@@ -400,9 +415,41 @@ class ModuleDependencyInfo {
400415
compiledModulePath, moduleDocPath, sourceInfoPath));
401416
}
402417

403-
/// Retrieve the module-level dependencies.
404-
ArrayRef<std::string> getModuleDependencies() const {
405-
return storage->moduleDependencies;
418+
/// Retrieve the module-level imports.
419+
ArrayRef<std::string> getModuleImports() const {
420+
return storage->moduleImports;
421+
}
422+
423+
/// Retreive the module-level dependencies.
424+
const ModuleDependencyIDSetVector &getModuleDependencies() const {
425+
assert(storage->resolved);
426+
return storage->resolvedModuleDependencies;
427+
}
428+
429+
/// Resolve a dependency's set of `imports` with qualified Module IDs
430+
void resolveDependencies(ModuleDependencyIDSetVector dependencyIDs) {
431+
#ifndef NDEBUG
432+
// Sanity check against the identified imports for this module
433+
for (auto importedModuleName : storage->moduleImports) {
434+
auto matchingModuleIDIt =
435+
std::find_if(dependencyIDs.begin(), dependencyIDs.end(),
436+
[&](ModuleDependencyID dependencyID) {
437+
return dependencyID.first == importedModuleName;
438+
});
439+
assert(matchingModuleIDIt != dependencyIDs.end() &&
440+
"Imported module not found in resolved module dependency set");
441+
}
442+
#endif
443+
assert(!storage->resolved && "Resolving an already-resolved dependency");
444+
storage->resolved = true;
445+
storage->resolvedModuleDependencies = dependencyIDs;
446+
}
447+
448+
bool isResolved() const {
449+
return storage->resolved;
450+
}
451+
void setIsResolved(bool isResolved) {
452+
storage->resolved = isResolved;
406453
}
407454

408455
/// Whether the dependencies are for a Swift module: either Textual, Source, Binary, or Placeholder.
@@ -444,19 +491,22 @@ class ModuleDependencyInfo {
444491
getAsPlaceholderDependencyModule() const;
445492

446493
/// Add a dependency on the given module, if it was not already in the set.
447-
void addModuleDependency(StringRef module,
448-
llvm::StringSet<> *alreadyAddedModules = nullptr);
494+
void addModuleImport(StringRef module,
495+
llvm::StringSet<> *alreadyAddedModules = nullptr);
449496

450497
/// Add a dependency on the given module, if it was not already in the set.
451-
void addModuleDependency(ImportPath::Module module,
452-
llvm::StringSet<> *alreadyAddedModules = nullptr) {
453-
addModuleDependency(module.front().Item.str(), alreadyAddedModules);
498+
void addModuleImport(ImportPath::Module module,
499+
llvm::StringSet<> *alreadyAddedModules = nullptr) {
500+
addModuleImport(module.front().Item.str(), alreadyAddedModules);
454501
}
455502

456-
/// Add all of the module dependencies for the imports in the given source
457-
/// file to the set of module dependencies.
458-
void addModuleDependencies(const SourceFile &sf,
459-
llvm::StringSet<> &alreadyAddedModules);
503+
/// Add all of the module imports in the given source
504+
/// file to the set of module imports.
505+
void addModuleImport(const SourceFile &sf,
506+
llvm::StringSet<> &alreadyAddedModules);
507+
/// Add a kind-qualified module dependency ID to the set of
508+
/// module dependencies.
509+
void addModuleDependency(ModuleDependencyID dependencyID);
460510

461511
/// Get the bridging header.
462512
Optional<std::string> getBridgingHeader() const;
@@ -480,8 +530,6 @@ class ModuleDependencyInfo {
480530
collectCrossImportOverlayNames(ASTContext &ctx, StringRef moduleName);
481531
};
482532

483-
using ModuleDependencyID = std::pair<std::string, ModuleDependencyKind>;
484-
using ModuleDependenciesVector = llvm::SmallVector<ModuleDependencyInfo, 1>;
485533
using ModuleNameToDependencyMap = llvm::StringMap<ModuleDependencyInfo>;
486534
using ModuleDependenciesKindMap =
487535
std::unordered_map<ModuleDependencyKind,
@@ -587,8 +635,8 @@ class SwiftDependencyScanningService {
587635
}
588636

589637
/// Whether we have cached dependency information for the given module.
590-
bool hasDependencies(StringRef moduleName,
591-
Optional<ModuleDependencyKind> kind) const;
638+
bool hasDependency(StringRef moduleName,
639+
Optional<ModuleDependencyKind> kind) const;
592640

593641
/// Return a pointer to the context-specific cache state of the current
594642
/// scanning action.
@@ -606,15 +654,15 @@ class SwiftDependencyScanningService {
606654
///
607655
/// \returns the cached result, or \c None if there is no cached entry.
608656
Optional<ModuleDependencyInfo>
609-
findDependencies(StringRef moduleName,
610-
Optional<ModuleDependencyKind> kind) const;
657+
findDependency(StringRef moduleName,
658+
Optional<ModuleDependencyKind> kind) const;
611659

612660
/// Record dependencies for the given module.
613-
const ModuleDependencyInfo *recordDependencies(StringRef moduleName,
661+
const ModuleDependencyInfo *recordDependency(StringRef moduleName,
614662
ModuleDependencyInfo dependencies);
615663

616664
/// Update stored dependencies for the given module.
617-
const ModuleDependencyInfo *updateDependencies(ModuleDependencyID moduleID,
665+
const ModuleDependencyInfo *updateDependency(ModuleDependencyID moduleID,
618666
ModuleDependencyInfo dependencies);
619667

620668
/// Reference the list of all module dependencies that are not source-based
@@ -658,7 +706,7 @@ class ModuleDependenciesCache {
658706
/// The Clang dependency scanner tool
659707
clang::tooling::dependencies::DependencyScanningTool clangScanningTool;
660708
/// Discovered Clang modules are only cached locally.
661-
llvm::StringMap<ModuleDependenciesVector> clangModuleDependencies;
709+
llvm::StringMap<ModuleDependencyInfo> clangModuleDependencies;
662710

663711
/// Retrieve the dependencies map that corresponds to the given dependency
664712
/// kind.
@@ -669,8 +717,8 @@ class ModuleDependenciesCache {
669717

670718
/// Local cache results lookup, only for modules which were discovered during
671719
/// the current scanner invocation.
672-
bool hasDependenciesLocalOnly(StringRef moduleName,
673-
Optional<ModuleDependencyKind> kind) const;
720+
bool hasDependencyLocalOnly(StringRef moduleName,
721+
Optional<ModuleDependencyKind> kind) const;
674722

675723
/// Local cache results lookup, only for modules which were discovered during
676724
/// the current scanner invocation.
@@ -687,8 +735,8 @@ class ModuleDependenciesCache {
687735

688736
public:
689737
/// Whether we have cached dependency information for the given module.
690-
bool hasDependencies(StringRef moduleName,
691-
Optional<ModuleDependencyKind> kind) const;
738+
bool hasDependency(StringRef moduleName,
739+
Optional<ModuleDependencyKind> kind) const;
692740

693741
/// Produce a reference to the Clang scanner tool associated with this cache
694742
clang::tooling::dependencies::DependencyScanningTool& getClangScannerTool() {
@@ -702,16 +750,21 @@ class ModuleDependenciesCache {
702750
///
703751
/// \returns the cached result, or \c None if there is no cached entry.
704752
Optional<ModuleDependencyInfo>
705-
findDependencies(StringRef moduleName,
706-
Optional<ModuleDependencyKind> kind) const;
753+
findDependency(StringRef moduleName,
754+
Optional<ModuleDependencyKind> kind) const;
707755

708756
/// Record dependencies for the given module.
709-
void recordDependencies(StringRef moduleName,
710-
ModuleDependencyInfo dependencies);
757+
void recordDependency(StringRef moduleName,
758+
ModuleDependencyInfo dependencies);
711759

712760
/// Update stored dependencies for the given module.
713-
void updateDependencies(ModuleDependencyID moduleID,
714-
ModuleDependencyInfo dependencies);
761+
void updateDependency(ModuleDependencyID moduleID,
762+
ModuleDependencyInfo dependencies);
763+
764+
/// Resolve a dependency module's set of imports
765+
/// to a kind-qualified set of module IDs.
766+
void resolveDependencyImports(ModuleDependencyID moduleID,
767+
ModuleDependencyIDSetVector dependencyIDs);
715768

716769
const std::vector<ModuleDependencyID> &getAllSourceModules() const {
717770
return globalScanningService.getAllSourceModules();

include/swift/DependencyScan/SerializedModuleDependencyCacheFormat.h

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ using llvm::BCVBR;
3636

3737
/// Every .moddepcache file begins with these 4 bytes, for easy identification.
3838
const unsigned char MODULE_DEPENDENCY_CACHE_FORMAT_SIGNATURE[] = {'I', 'M', 'D','C'};
39-
const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MAJOR = 3;
39+
const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MAJOR = 4;
4040
/// Increment this on every change.
4141
const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MINOR = 0;
4242

@@ -45,19 +45,21 @@ const unsigned MODULE_DEPENDENCY_CACHE_FORMAT_VERSION_MINOR = 0;
4545
using IdentifierIDField = BCVBR<13>;
4646
using FileIDField = IdentifierIDField;
4747
using ModuleIDField = IdentifierIDField;
48-
using ContextHashField = IdentifierIDField;
48+
using ContextHashIDField = IdentifierIDField;
4949

5050
/// A bit that indicates whether or not a module is a framework
5151
using IsFrameworkField = BCFixed<1>;
5252

5353
/// Arrays of various identifiers, distinguished for readability
5454
using IdentifierIDArryField = llvm::BCArray<IdentifierIDField>;
55+
using ModuleIDArryField = llvm::BCArray<IdentifierIDField>;
5556

5657
/// Identifiers used to refer to the above arrays
5758
using FileIDArrayIDField = IdentifierIDField;
5859
using ContextHashIDField = IdentifierIDField;
59-
using DependencyIDArrayIDField = IdentifierIDField;
60+
using ImportArrayIDField = IdentifierIDField;
6061
using FlagIDArrayIDField = IdentifierIDField;
62+
using DependencyIDArrayIDField = IdentifierIDField;
6163

6264
/// The ID of the top-level block containing the dependency graph
6365
const unsigned GRAPH_BLOCK_ID = llvm::bitc::FIRST_APPLICATION_BLOCKID;
@@ -115,34 +117,35 @@ using IdentifierArrayLayout =
115117
// - SwiftPlaceholderModuleDetails
116118
// - ClangModuleDetails
117119
using ModuleInfoLayout =
118-
BCRecordLayout<MODULE_NODE, // ID
119-
IdentifierIDField, // module name
120-
ContextHashIDField, //
121-
DependencyIDArrayIDField // directDependencies
120+
BCRecordLayout<MODULE_NODE, // ID
121+
IdentifierIDField, // moduleName
122+
ContextHashIDField, // contextHash
123+
ImportArrayIDField, // moduleImports
124+
DependencyIDArrayIDField // resolvedModuleDependencies
122125
>;
123126

124127
using SwiftInterfaceModuleDetailsLayout =
125128
BCRecordLayout<SWIFT_INTERFACE_MODULE_DETAILS_NODE, // ID
126-
FileIDField, // outputFilePath
127-
FileIDField, // swiftInterfaceFile
128-
FileIDArrayIDField, // compiledModuleCandidates
129-
FlagIDArrayIDField, // buildCommandLine
130-
FlagIDArrayIDField, // extraPCMArgs
131-
ContextHashField, // contextHash
132-
IsFrameworkField, // isFramework
133-
FileIDField, // bridgingHeaderFile
134-
FileIDArrayIDField, // sourceFiles
135-
FileIDArrayIDField, // bridgingSourceFiles
136-
FileIDArrayIDField // bridgingModuleDependencies
129+
FileIDField, // outputFilePath
130+
FileIDField, // swiftInterfaceFile
131+
FileIDArrayIDField, // compiledModuleCandidates
132+
FlagIDArrayIDField, // buildCommandLine
133+
FlagIDArrayIDField, // extraPCMArgs
134+
ContextHashIDField, // contextHash
135+
IsFrameworkField, // isFramework
136+
FileIDField, // bridgingHeaderFile
137+
FileIDArrayIDField, // sourceFiles
138+
FileIDArrayIDField, // bridgingSourceFiles
139+
FileIDArrayIDField // bridgingModuleDependencies
137140
>;
138141

139142
using SwiftSourceModuleDetailsLayout =
140143
BCRecordLayout<SWIFT_SOURCE_MODULE_DETAILS_NODE, // ID
141-
FlagIDArrayIDField, // extraPCMArgs
142-
FileIDField, // bridgingHeaderFile
143-
FileIDArrayIDField, // sourceFiles
144-
FileIDArrayIDField, // bridgingSourceFiles
145-
FileIDArrayIDField // bridgingModuleDependencies
144+
FlagIDArrayIDField, // extraPCMArgs
145+
FileIDField, // bridgingHeaderFile
146+
FileIDArrayIDField, // sourceFiles
147+
FileIDArrayIDField, // bridgingSourceFiles
148+
FileIDArrayIDField // bridgingModuleDependencies
146149
>;
147150

148151
using SwiftBinaryModuleDetailsLayout =
@@ -157,14 +160,14 @@ using SwiftPlaceholderModuleDetailsLayout =
157160
BCRecordLayout<SWIFT_PLACEHOLDER_MODULE_DETAILS_NODE, // ID
158161
FileIDField, // compiledModulePath
159162
FileIDField, // moduleDocPath
160-
FileIDField // moduleSourceInfoPath
163+
FileIDField // moduleSourceInfoPath
161164
>;
162165

163166
using ClangModuleDetailsLayout =
164167
BCRecordLayout<CLANG_MODULE_DETAILS_NODE, // ID
165168
FileIDField, // pcmOutputPath
166169
FileIDField, // moduleMapPath
167-
ContextHashField, // contextHash
170+
ContextHashIDField, // contextHash
168171
FlagIDArrayIDField, // commandLine
169172
FileIDArrayIDField, // fileDependencies
170173
FlagIDArrayIDField // capturedPCMArgs

0 commit comments

Comments
 (0)