Skip to content

Commit 8335a03

Browse files
artemcmartemcm
committed
[Dependency Scanning] Refactor batch clang dependency query to preserve only partial results
Previously, with the change to bridge Clang dependency scanning results on-demand, the scanner would execute Clang dependency scanning queries for each unresolved import, in parallel, and aggregate all of the results to post-process (including on-demand bridging) later. As a consequence of that change, all of the Clang scanner queries' results ('TranslationUnitDeps') got aggregated during a scan and had their lifetimes extended until a later point when they got processed and added to the scanner's cache. This change refactors the Clang dependency scanner invocation to, upon query completion, accumulate only the 'ModuleDeps' nodes which have not been registered by a prior scan, discarding the rest of the 'TranslationUnitDeps' graph. The arrgegated 'ModuleDeps' objects are still bridged on-demand downstream. This change further splits up the 'resolveAllClangModuleDependencies' method's functionality to improve readability and maintainability, into: - 'gatherUnresolvedImports' method which collects all of collected Swift dependents' imports which did not get resolved to Swift dependencies - 'performParallelClangModuleLookup' which actually executes the parallel queries and includes the new logic described above - 'cacheComputedClangModuleLookupResults' method which takes the result of the parallel Clang scanner query and records in in the Swift scanner cache - 'reQueryMissedModulesFromCache' method which covers the scenario where Clang scanner query returned no result because either the dependency can only be found transitively, or the query is for a dependency previously-queried.
1 parent 2af325f commit 8335a03

File tree

4 files changed

+306
-169
lines changed

4 files changed

+306
-169
lines changed

‎include/swift/AST/ModuleDependencies.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1187,6 +1187,12 @@ class ModuleDependenciesCache {
11871187
DiagnosticEngine &diags,
11881188
BridgeClangDependencyCallback bridgeClangModule);
11891189

1190+
/// Record Clang module dependency.
1191+
void recordClangDependency(
1192+
const clang::tooling::dependencies::ModuleDeps &dependency,
1193+
DiagnosticEngine &diags,
1194+
BridgeClangDependencyCallback bridgeClangModule);
1195+
11901196
/// Update stored dependencies for the given module.
11911197
void updateDependency(ModuleDependencyID moduleID,
11921198
ModuleDependencyInfo dependencyInfo);

‎include/swift/DependencyScan/ModuleDependencyScanner.h‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ using LookupModuleOutputCallback = llvm::function_ref<std::string(
3232
clang::tooling::dependencies::ModuleOutputKind)>;
3333
using RemapPathCallback = llvm::function_ref<std::string(StringRef)>;
3434

35+
/// A map from a module id to a collection of import statement infos.
36+
using ImportStatementInfoMap =
37+
std::unordered_map<ModuleDependencyID,
38+
std::vector<ScannerImportStatementInfo>>;
39+
3540
/// A dependency scanning worker which performs filesystem lookup
3641
/// of a named module dependency.
3742
class ModuleDependencyScanningWorker {
@@ -329,6 +334,56 @@ class ModuleDependencyScanner {
329334
/// for a given module name.
330335
Identifier getModuleImportIdentifier(StringRef moduleName);
331336

337+
private:
338+
struct BatchClangModuleLookupResult {
339+
llvm::StringMap<clang::tooling::dependencies::ModuleDeps>
340+
discoveredDependencyInfos;
341+
llvm::StringMap<std::vector<std::string>> visibleModules;
342+
};
343+
344+
/// For the provided collection of unresolved imports
345+
/// belonging to identified Swift dependnecies, execute a parallel
346+
/// query to the Clang dependency scanner for each import's module identifier.
347+
void performParallelClangModuleLookup(
348+
const ImportStatementInfoMap &unresolvedImportsMap,
349+
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
350+
ModuleDependenciesCache &cache,
351+
BatchClangModuleLookupResult &result);
352+
353+
/// Given a result of a batch Clang module dependency lookup,
354+
/// record its results in the cache:
355+
/// 1. Record all discovered Clang module dependency infos
356+
/// in the \c cache.
357+
/// 1. Update the set of visible Clang modules from each Swift module
358+
/// in the \c cache.
359+
/// 2. Update the total collection of all disovered clang modules
360+
/// in \c allDiscoveredClangModules.
361+
/// 3. Record all import identifiers which the scan failed to resolve
362+
/// in \c failedToResolveImports.
363+
/// 4. Update the set of resolved Clang dependencies for each Swift
364+
/// module dependency in \c resolvedClangDependenciesMap.
365+
void cacheComputedClangModuleLookupResults(
366+
const BatchClangModuleLookupResult &lookupResult,
367+
const ImportStatementInfoMap &unresolvedImportsMap,
368+
const ImportStatementInfoMap &unresolvedOptionalImportsMap,
369+
ArrayRef<ModuleDependencyID> swiftModuleDependents,
370+
ModuleDependenciesCache &cache,
371+
ModuleDependencyIDSetVector &allDiscoveredClangModules,
372+
std::vector<std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
373+
&failedToResolveImports,
374+
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
375+
&resolvedClangDependenciesMap);
376+
377+
/// Re-query some failed-to-resolve Clang imports from cache
378+
/// in chance they were brought in as transitive dependencies.
379+
void reQueryMissedModulesFromCache(
380+
ModuleDependenciesCache &cache,
381+
const std::vector<
382+
std::pair<ModuleDependencyID, ScannerImportStatementInfo>>
383+
&failedToResolveImports,
384+
std::unordered_map<ModuleDependencyID, ModuleDependencyIDSetVector>
385+
&resolvedClangDependenciesMap);
386+
332387
/// Assuming the \c `moduleImport` failed to resolve,
333388
/// iterate over all binary Swift module dependencies with serialized
334389
/// search paths and attempt to diagnose if the failed-to-resolve module

‎lib/AST/ModuleDependencies.cpp‎

Lines changed: 62 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -788,61 +788,68 @@ void ModuleDependenciesCache::recordClangDependencies(
788788
const clang::tooling::dependencies::ModuleDepsGraph &dependencies,
789789
DiagnosticEngine &diags,
790790
BridgeClangDependencyCallback bridgeClangModule) {
791-
for (const auto &dep : dependencies) {
792-
auto depID =
793-
ModuleDependencyID{dep.ID.ModuleName, ModuleDependencyKind::Clang};
794-
if (hasDependency(depID)) {
795-
auto priorClangModuleDetails =
796-
findKnownDependency(depID).getAsClangModule();
797-
DEBUG_ASSERT(priorClangModuleDetails);
798-
auto priorContextHash = priorClangModuleDetails->contextHash;
799-
auto newContextHash = dep.ID.ContextHash;
800-
if (priorContextHash != newContextHash) {
801-
// This situation means that within the same scanning action, Clang
802-
// Dependency Scanner has produced two different variants of the same
803-
// module. This is not supposed to happen, but we are currently
804-
// hunting down the rare cases where it does, seemingly due to
805-
// differences in Clang Scanner direct by-name queries and transitive
806-
// header lookup queries.
807-
//
808-
// Emit a failure diagnostic here that is hopefully more actionable
809-
// for the time being.
810-
diags.diagnose(SourceLoc(),
811-
diag::dependency_scan_unexpected_variant,
812-
dep.ID.ModuleName);
813-
diags.diagnose(
814-
SourceLoc(),
815-
diag::dependency_scan_unexpected_variant_context_hash_note,
816-
priorContextHash, newContextHash);
817-
diags.diagnose(
818-
SourceLoc(),
819-
diag::dependency_scan_unexpected_variant_module_map_note,
820-
priorClangModuleDetails->moduleMapFile, dep.ClangModuleMapFile);
821-
822-
auto newClangModuleDetails = bridgeClangModule(dep).getAsClangModule();
823-
auto diagnoseExtraCommandLineFlags =
824-
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
825-
const ClangModuleDependencyStorage *baseModuleDetails,
826-
bool isNewlyDiscovered) -> void {
827-
std::unordered_set<std::string> baseCommandLineSet(
828-
baseModuleDetails->buildCommandLine.begin(),
829-
baseModuleDetails->buildCommandLine.end());
830-
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
831-
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
832-
diags.diagnose(
833-
SourceLoc(),
834-
diag::dependency_scan_unexpected_variant_extra_arg_note,
835-
isNewlyDiscovered, checkArg);
836-
};
837-
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
838-
newClangModuleDetails, true);
839-
diagnoseExtraCommandLineFlags(newClangModuleDetails,
840-
priorClangModuleDetails, false);
841-
}
842-
} else {
843-
recordDependency(dep.ID.ModuleName, bridgeClangModule(dep));
844-
addSeenClangModule(dep.ID);
845-
}
791+
for (const auto &dep : dependencies)
792+
recordClangDependency(dep, diags, bridgeClangModule);
793+
}
794+
795+
void ModuleDependenciesCache::recordClangDependency(
796+
const clang::tooling::dependencies::ModuleDeps &dependency,
797+
DiagnosticEngine &diags,
798+
BridgeClangDependencyCallback bridgeClangModule) {
799+
auto depID =
800+
ModuleDependencyID{dependency.ID.ModuleName, ModuleDependencyKind::Clang};
801+
if (!hasDependency(depID)) {
802+
recordDependency(dependency.ID.ModuleName, bridgeClangModule(dependency));
803+
addSeenClangModule(dependency.ID);
804+
return;
805+
}
806+
807+
auto priorClangModuleDetails =
808+
findKnownDependency(depID).getAsClangModule();
809+
DEBUG_ASSERT(priorClangModuleDetails);
810+
auto priorContextHash = priorClangModuleDetails->contextHash;
811+
auto newContextHash = dependency.ID.ContextHash;
812+
if (priorContextHash != newContextHash) {
813+
// This situation means that within the same scanning action, Clang
814+
// Dependency Scanner has produced two different variants of the same
815+
// module. This is not supposed to happen, but we are currently
816+
// hunting down the rare cases where it does, seemingly due to
817+
// differences in Clang Scanner direct by-name queries and transitive
818+
// header lookup queries.
819+
//
820+
// Emit a failure diagnostic here that is hopefully more actionable
821+
// for the time being.
822+
diags.diagnose(SourceLoc(),
823+
diag::dependency_scan_unexpected_variant,
824+
dependency.ID.ModuleName);
825+
diags.diagnose(
826+
SourceLoc(),
827+
diag::dependency_scan_unexpected_variant_context_hash_note,
828+
priorContextHash, newContextHash);
829+
diags.diagnose(
830+
SourceLoc(),
831+
diag::dependency_scan_unexpected_variant_module_map_note,
832+
priorClangModuleDetails->moduleMapFile, dependency.ClangModuleMapFile);
833+
834+
auto newClangModuleDetails = bridgeClangModule(dependency).getAsClangModule();
835+
auto diagnoseExtraCommandLineFlags =
836+
[&diags](const ClangModuleDependencyStorage *checkModuleDetails,
837+
const ClangModuleDependencyStorage *baseModuleDetails,
838+
bool isNewlyDiscovered) -> void {
839+
std::unordered_set<std::string> baseCommandLineSet(
840+
baseModuleDetails->buildCommandLine.begin(),
841+
baseModuleDetails->buildCommandLine.end());
842+
for (const auto &checkArg : checkModuleDetails->buildCommandLine)
843+
if (baseCommandLineSet.find(checkArg) == baseCommandLineSet.end())
844+
diags.diagnose(
845+
SourceLoc(),
846+
diag::dependency_scan_unexpected_variant_extra_arg_note,
847+
isNewlyDiscovered, checkArg);
848+
};
849+
diagnoseExtraCommandLineFlags(priorClangModuleDetails,
850+
newClangModuleDetails, true);
851+
diagnoseExtraCommandLineFlags(newClangModuleDetails,
852+
priorClangModuleDetails, false);
846853
}
847854
}
848855

0 commit comments

Comments
 (0)