Fast dependency scanning for Swift #28515

DougGregor · 2019-12-01T07:44:47Z

Implement a new "fast" dependency scanning option,
-scan-dependencies, in the Swift frontend that determines all
of the source file and module dependencies for a given set of
Swift sources. It covers all forms of dependencies:

Swift (serialized) module files, by reading the module header
Swift interface files, by parsing the source code to find imports
Swift source modules, by parsing the source code to find imports
Clang modules, using Clang's fast dependency scanning tool
Bridging headers, including the source files and modules they depend on

A single -scan-dependencies operation maps out the full
dependency graph for the given Swift source files, including all
of the Swift and Clang modules that may need to be built, such
that all of the work can be scheduled up front by the Swift
driver or any other build system that understands this
option. The dependency graph is emitted as JSON, which can be
consumed by these other tools. Swift code with Codable
data structures is used for validating this JSON, and can
(eventually) be picked up by other tools.

This operation also supports -emit-dependencies, the flag that
produces make-style dependencies to determine all of the input
files that can affect the compilation, making it usable as a fast
dependency scanner even for clients not interesting in prebuilding
modules.

In a source file that imports just Cocoa on macOS, fast
dependency scanning with this patch is ~11x faster than
the current dependency scanner (with -resolve-imports), taking
it from 5.7s (and writing 48MB of Clang modules) down to .49s.

DougGregor · 2019-12-01T07:48:35Z

@Bigcheese this totally doesn't work yet, but it's getting closer to mapping out the various Clang + Swift module builds that are involved in a Swift compile. You can see my super hacky interaction with Clang's fast dependency scanner in ClangModuleDependencyScanner.cpp.

DougGregor · 2019-12-02T00:58:23Z

Hey, it works a little bit now with a Clang change to handle @import directives in Clang's dependency scanner.

DougGregor · 2019-12-02T01:44:06Z

The necessary Clang changes are at swiftlang/llvm-project#392

slavapestov · 2019-12-04T21:55:16Z

Would this allow the frontend to get out of the business of building interface files altogether?

DougGregor · 2019-12-06T00:32:19Z

Would this allow the frontend to get out of the business of building interface files altogether?

Yes, that's the intent. The driver would coordinate the interface file builds so the frontend wouldn't have to do it.

DougGregor · 2020-01-05T06:53:22Z

I collapsed everything because it doesn't make sense for this to ride on top of -emit-imported-modules; rather, that flag should (separately) go away. Instead, implement a new flag (-scan-dependencies) whose primary output is the JSON dependency graph and which will eventually also emit normal makefile dependencies (.d file) as well, for clients that need it.

jerrymarino · 2020-01-11T19:00:44Z

So this should this allow us to directly run the frontend in external build systems directly? Seems like we can use the new .d files

DougGregor · 2020-01-12T05:19:45Z

So this should this allow us to directly run the frontend in external build systems directly? Seems like we can use the new .d files

We'd prefer that external build systems go through the driver in general, because the frontend interface is the supported way to call the compiler. However, we can expose this as a driver option once it's been tested a bit better. I would certainly appreciate if others would test it out and see what I've missed.

DougGregor · 2020-03-29T05:38:49Z

Brought it up to date with master. It no longer needs cherry-picks on top of Clang

DougGregor · 2020-03-29T05:39:06Z

@swift-ci please smoke test

DougGregor · 2020-03-29T06:47:01Z

@swift-ci please smoke test

DougGregor · 2020-03-29T06:47:15Z

@swift-ci please smoke test

davidungar · 2020-04-16T00:49:26Z

Lovely, readable code. But, why were the storage classes factored out, instead of just having a base ModuleDependencies class and two subclasses? Is there not always a one-to-one relationship to the storage? I'm missing something here...

davidungar

I can't for correctness, but overall it looks pretty good. I threw in a few suggestions that could incrementally improve the clarity, IMO. The most important of those would be the places where a "why" comment could help.

include/swift/AST/ModuleDependencies.h

davidungar · 2020-04-16T00:39:57Z

include/swift/AST/ModuleDependencies.h

+  const std::string compiledModulePath;
+
+  /// The set of modules on which this module depends.
+  std::vector<std::string> moduleDependencies;


OK, now I see that a ModuleDependencies of some sort links a module that uses things to modules that define things. Hmmm.... it's the reverse of what I think I'd need to do dependency propagation, which goes from def to use. This class goes from use to def. A "why" comment explaining why this choice of direction might be nice.

It describes "what has to be built before me". I'll clarify this in the overall file/class comment.

davidungar · 2020-04-16T00:51:47Z

lib/AST/ASTContext.cpp

+    StringRef moduleName, bool isUnderlyingClangModule,
+    ModuleDependenciesCache &cache) {
+  for (auto &loader : getImpl().ModuleLoaders) {
+    if (isUnderlyingClangModule &&


A "why" comment could be helpful here.

This code stinks; it'll be removed when we refactor more.

davidungar · 2020-04-16T00:53:28Z

lib/AST/ModuleDependencies.cpp

+  }
+}
+
+bool ModuleDependenciesCache::hasDependencies(


Would hasDependenciesOnOtherModules be clearer or just redundant? I'm not sure...

Redundant, IMO.

davidungar · 2020-04-16T00:55:49Z

lib/ClangImporter/ClangModuleDependencyScanner.cpp

+  }
+}
+
+llvm::ErrorOr<StringRef> ClangModuleDependenciesCacheImpl::getImportHackFile() {


ImportHackFile?! I hadn't seen that term before. Is it one of our standard terms? Where one go to find it? Maybe it's in a comment on the function declaration & I missed it?

Clang will provide us with an API that makes this unnecessary. It's a HACK.

davidungar · 2020-04-16T05:27:29Z

lib/ClangImporter/ClangModuleDependencyScanner.cpp

+  return cache.findDependencies(moduleName, ModuleDependenciesKind::Clang);
+}
+
+bool ClangImporter::addBridgingHeaderDependencies(


You know me; I would prefer to break up this function into more understandable (for me) pieces.

davidungar · 2020-04-16T05:28:47Z

lib/ClangImporter/ClangModuleDependencyScanner.cpp

+  ClangImporterOptions importerOpts;
+
+  // Retrieve the bridging header.
+  std::string bridgingHeader = *targetModule.getBridgingHeader();


davidungar · 2020-04-16T05:30:46Z

lib/ClangImporter/ClangModuleDependencyScanner.cpp

+
+  // Determine the command-line arguments for dependency scanning.
+  auto &ctx = Impl.SwiftContext;
+  std::vector<std::string> commandLineArgs =


Sorry if all these consts are getting tedious. As you know, the idea is that if there are more than a few lines after the assignment, if something is const the first assignment is enough to know where it's coming from without having to inspect all uses. (Esp. in C++ where one can pass a reference, sigh.) Forgive me for stating the obvious.

davidungar · 2020-04-16T05:32:59Z

lib/Frontend/Frontend.cpp

-shouldImplicityImportSwiftOnoneSupportModule(CompilerInvocation &Invocation) {
-  if (Invocation.getImplicitModuleImportKind() !=
-      SourceFile::ImplicitModuleImportKind::Stdlib)
+bool CompilerInvocation::shouldImportSwiftONoneSupport() const {


davidungar · 2020-04-16T05:43:04Z

lib/FrontendTool/FrontendTool.cpp

+    bool EvaluateConditionals =
+        Action == FrontendOptions::ActionType::EmitImportedModules
+        || Action == FrontendOptions::ActionType::ScanDependencies;
+    Instance.performParseOnly(EvaluateConditionals,


The flow in here and through 1271 is confusing. The predicate here is FrontendOptions::shouldActionOnlyParse(Action) and I would expect a return right after line 1262.
But instead, it falls through to 1271: if (Action == FrontendOptions::ActionType::Parse) and it that is true it returns. Which leaves me pondering: what's the difference between the two predicates? Is one a subset of the other? There must be a clearer way to write this. Maybe also do a return right here?
OK, I see it, now. Will leave the above confused ramblings in FWIW.

The whole business starting at 1255 could use a comment to the effect of do the the right about of parsing, etc. for the mode, or some such. (I would factor it out into a function with that name.)

Then, a blank line after 1258 would make it clear that the comment above it only applies to that one line.
(Not your fault!)

Then, a "why" comment might be nice about why conditionals are only being evaluated in these two cases.

I guess what's going on is that this function could be split in a "parse" phase, through 1272 and an after-parse phase, and that demarcation would clarify what's going on. (Not your fault, but this change just makes it incrementally worse, like an amoeba that's overdue for fission.)

Implement a new "fast" dependency scanning option, `-scan-dependencies`, in the Swift frontend that determines all of the source file and module dependencies for a given set of Swift sources. It covers four forms of modules: 1) Swift (serialized) module files, by reading the module header 2) Swift interface files, by parsing the source code to find imports 3) Swift source modules, by parsing the source code to find imports 4) Clang modules, using Clang's fast dependency scanning tool A single `-scan-dependencies` operation maps out the full dependency graph for the given Swift source files, including all of the Swift and Clang modules that may need to be built, such that all of the work can be scheduled up front by the Swift driver or any other build system that understands this option. The dependency graph is emitted as JSON, which can be consumed by these other tools.

…les.

If requested, produce the normal make-style dependency file from the fast dependency scanner, using the same dependency-tracking infrastructure.

When there is a bridging header associated with the module, scan and record its dependencies. Note them in a separate structure to capture the specific dependencies of the bridging header.

…encies

…ments.

DougGregor · 2020-04-24T19:59:21Z

Rebasing to fix merge conflicts, adding some comments, tweaking tests... and let's get this landed.

DougGregor · 2020-04-24T19:59:30Z

@swift-ci please test

DougGregor · 2020-04-24T20:00:03Z

@swift-ci please test

DougGregor · 2020-04-24T20:00:15Z

@swift-ci please test platform Windows

DougGregor · 2020-04-24T20:02:13Z

@swift-ci please test platform Windows

swift-ci · 2020-04-24T21:13:14Z

Build failed
Swift Test Linux Platform
Git Sha - 5351d54

swift-ci · 2020-04-24T22:02:27Z

Build failed
Swift Test OS X Platform
Git Sha - 5351d54

DougGregor · 2020-04-25T02:44:03Z

@swift-ci please test

DougGregor force-pushed the fast-dependency-scanning branch from 86e6426 to 52869a3 Compare December 4, 2019 01:06

DougGregor force-pushed the fast-dependency-scanning branch from 2aa3524 to 959585d Compare December 28, 2019 06:54

DougGregor mentioned this pull request Dec 30, 2019

[WIP] Invoke the Swift frontend to prescan for dependencies. swiftlang/swift-driver#47

Closed

DougGregor force-pushed the fast-dependency-scanning branch from 959585d to a23be61 Compare January 5, 2020 06:50

DougGregor force-pushed the fast-dependency-scanning branch from a23be61 to 9fab4f2 Compare January 6, 2020 05:29

BasThomas mentioned this pull request Jan 11, 2020

[151] Issue #151 - January 16, 2020 SwiftWeekly/swiftweekly.github.io#490

Closed

DougGregor force-pushed the fast-dependency-scanning branch from 93aef39 to 64aad80 Compare March 29, 2020 05:38

davidungar approved these changes Apr 16, 2020

View reviewed changes

DougGregor added 9 commits April 24, 2020 12:58

[Scan dependencies] Find overlays associated with imported Clang modu…

b19606c

…les.

[Dependency scanning] Emit make-style dependency files.

08ec878

If requested, produce the normal make-style dependency file from the fast dependency scanner, using the same dependency-tracking infrastructure.

[Dependency scanning] Scan and record bridging header dependencies.

d6484c8

When there is a bridging header associated with the module, scan and record its dependencies. Note them in a separate structure to capture the specific dependencies of the bridging header.

[Dependency scanning] Parse JSON output into Swift data structures.

5f953da

[Dependency scanning] Add implicit Swift and SwiftOnoneSupport depend…

71e7d6a

…encies

[Dependency scanning] Cope with inferred module maps

a4b0e82

Sink the module dependency scanner down into the Serialization library

702c38e

[Dependency scanner] Get the Clang module file name from Clang.

fdfadf8

DougGregor added 3 commits April 24, 2020 12:58

[Dependency scanner] Capture Clang context hash and command-line argu…

6b5e7ef

…ments.

[Dependency scanning] Improve overview documentation somewhat.

0128786

Update test to reflect unimportant change in output results

5351d54

DougGregor force-pushed the fast-dependency-scanning branch from 9febd18 to 5351d54 Compare April 24, 2020 19:59

DougGregor changed the title ~~[WIP] Dependency scanning for Swift~~ Dependency scanning for Swift Apr 24, 2020

DougGregor changed the title ~~Dependency scanning for Swift~~ Fast dependency scanning for Swift Apr 24, 2020

DougGregor merged commit 172c047 into swiftlang:master Apr 25, 2020

DougGregor deleted the fast-dependency-scanning branch April 25, 2020 06:54

Fast dependency scanning for Swift #28515

Fast dependency scanning for Swift #28515

Uh oh!

Conversation

DougGregor commented Dec 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DougGregor commented Dec 1, 2019

Uh oh!

DougGregor commented Dec 2, 2019

Uh oh!

DougGregor commented Dec 2, 2019

Uh oh!

slavapestov commented Dec 4, 2019

Uh oh!

DougGregor commented Dec 6, 2019

Uh oh!

DougGregor commented Jan 5, 2020

Uh oh!

jerrymarino commented Jan 11, 2020

Uh oh!

DougGregor commented Jan 12, 2020

Uh oh!

DougGregor commented Mar 29, 2020

Uh oh!

DougGregor commented Mar 29, 2020

Uh oh!

DougGregor commented Mar 29, 2020

Uh oh!

DougGregor commented Mar 29, 2020

Uh oh!

davidungar commented Apr 16, 2020

Uh oh!

davidungar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DougGregor commented Apr 24, 2020

Uh oh!

DougGregor commented Apr 24, 2020

Uh oh!

DougGregor commented Apr 24, 2020

Uh oh!

DougGregor commented Apr 24, 2020

Uh oh!

DougGregor commented Apr 24, 2020

Uh oh!

swift-ci commented Apr 24, 2020

Uh oh!

swift-ci commented Apr 24, 2020

Uh oh!

DougGregor commented Apr 25, 2020

Uh oh!

Uh oh!

DougGregor commented Dec 1, 2019 •

edited

Loading