Skip to content

Commit 764df6d

Browse files
committed
Fold Incremental External Dependency Nodes Into External Dependency Nodes
Remove this distinction without a difference. Originally, the thought was to 1) Isolate the cross-module build infrastructure 2) Provide a signal to the driver that a dependency had swiftdeps info in it But the driver need only notice swiftmodule files as external dependencies and try to extract that information if it can to divine the signal it needs. Additionally, we can give it fingerprints as priors to let it know there might be incremental info to be had.
1 parent c269d00 commit 764df6d

File tree

8 files changed

+7
-286
lines changed

8 files changed

+7
-286
lines changed

include/swift/Basic/ReferenceDependencyKeys.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ enum class NodeKind {
5353
dynamicLookup,
5454
externalDepend,
5555
sourceFileProvide,
56-
incrementalExternalDepend,
5756
/// For iterating through the NodeKinds.
5857
kindCount
5958
};
@@ -64,7 +63,7 @@ const std::string NodeKindNames[]{
6463
"topLevel", "nominal",
6564
"potentialMember", "member",
6665
"dynamicLookup", "externalDepend",
67-
"sourceFileProvide", "incrementalExternalDepend"};
66+
"sourceFileProvide"};
6867
} // end namespace fine_grained_dependencies
6968
} // end namespace swift
7069

include/swift/Driver/FineGrainedDependencyDriverGraph.h

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ class ModuleDepGraph {
170170

171171
// Supports requests from the driver to getExternalDependencies.
172172
std::unordered_set<std::string> externalDependencies;
173-
std::unordered_set<std::string> incrementalExternalDependencies;
174173

175174
/// Keyed by swiftdeps filename, so we can get back to Jobs.
176175
std::unordered_map<std::string, const driver::Job *> jobsBySwiftDeps;
@@ -515,22 +514,12 @@ class ModuleDepGraph {
515514
std::vector<const driver::Job *>
516515
findExternallyDependentUntracedJobs(StringRef externalDependency);
517516

518-
/// Find jobs that were previously not known to need compilation but that
519-
/// depend on \c incrementalExternalDependency.
520-
///
521-
/// This code path should only act as a fallback to the status-quo behavior.
522-
/// Otherwise it acts to pessimize the behavior of cross-module incremental
523-
/// builds.
524-
std::vector<const driver::Job *>
525-
findIncrementalExternallyDependentUntracedJobs(StringRef externalDependency);
526-
527517
//============================================================================
528518
// MARK: ModuleDepGraph - External dependencies
529519
//============================================================================
530520

531521
public:
532522
std::vector<StringRef> getExternalDependencies() const;
533-
std::vector<StringRef> getIncrementalExternalDependencies() const;
534523

535524
void forEachUntracedJobDirectlyDependentOnExternalSwiftDeps(
536525
StringRef externalDependency, function_ref<void(const driver::Job *)> fn);

lib/AST/FineGrainedDependencies.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,6 @@ std::string DependencyKey::humanReadableName() const {
240240
switch (kind) {
241241
case NodeKind::member:
242242
return demangleTypeAsContext(context) + "." + name;
243-
case NodeKind::incrementalExternalDepend:
244243
case NodeKind::externalDepend:
245244
case NodeKind::sourceFileProvide:
246245
return llvm::sys::path::filename(name).str();
@@ -271,12 +270,9 @@ raw_ostream &fine_grained_dependencies::operator<<(raw_ostream &out,
271270
bool DependencyKey::verify() const {
272271
assert((getKind() != NodeKind::externalDepend || isInterface()) &&
273272
"All external dependencies must be interfaces.");
274-
assert((getKind() != NodeKind::incrementalExternalDepend || isInterface()) &&
275-
"All incremental external dependencies must be interfaces.");
276273
switch (getKind()) {
277274
case NodeKind::topLevel:
278275
case NodeKind::dynamicLookup:
279-
case NodeKind::incrementalExternalDepend:
280276
case NodeKind::externalDepend:
281277
case NodeKind::sourceFileProvide:
282278
assert(context.empty() && !name.empty() && "Must only have a name");
@@ -307,7 +303,6 @@ void DependencyKey::verifyNodeKindNames() {
307303
CHECK_NAME(potentialMember)
308304
CHECK_NAME(member)
309305
CHECK_NAME(dynamicLookup)
310-
CHECK_NAME(incrementalExternalDepend)
311306
CHECK_NAME(externalDepend)
312307
CHECK_NAME(sourceFileProvide)
313308
case NodeKind::kindCount:

lib/AST/FrontendSourceFileDepGraphFactory.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -491,8 +491,8 @@ class ExternalDependencyEnumerator {
491491

492492
void enumerateExternalUses(UseEnumerator enumerator) {
493493
for (const auto &id : depTracker.getIncrementalDependencies()) {
494-
enumerateUse<NodeKind::incrementalExternalDepend>(enumerator, id.path,
495-
id.fingerprint);
494+
enumerateUse<NodeKind::externalDepend>(enumerator, id.path,
495+
id.fingerprint);
496496
}
497497
for (StringRef s : depTracker.getDependencies()) {
498498
enumerateUse<NodeKind::externalDepend>(enumerator, s, None);
@@ -503,8 +503,7 @@ class ExternalDependencyEnumerator {
503503
template <NodeKind kind>
504504
void enumerateUse(UseEnumerator createDefUse, StringRef name,
505505
Optional<Fingerprint> maybeFP) {
506-
static_assert(kind == NodeKind::incrementalExternalDepend ||
507-
kind == NodeKind::externalDepend,
506+
static_assert(kind == NodeKind::externalDepend,
508507
"Not a kind of external dependency!");
509508
createDefUse(DependencyKey(kind, DeclAspect::interface, "", name.str()),
510509
sourceFileImplementation, maybeFP);

lib/Driver/Compilation.cpp

Lines changed: 1 addition & 195 deletions
Original file line numberDiff line numberDiff line change
@@ -918,9 +918,6 @@ namespace driver {
918918
for (const auto cmd :
919919
collectExternallyDependentJobsFromDependencyGraph())
920920
jobsToSchedule.insert(cmd);
921-
for (const auto cmd :
922-
collectIncrementalExternallyDependentJobsFromDependencyGraph())
923-
jobsToSchedule.insert(cmd);
924921

925922
// The merge-modules job is special: it *must* be scheduled if any other
926923
// job has been scheduled because any other job can influence the
@@ -1073,103 +1070,6 @@ namespace driver {
10731070
return ExternallyDependentJobs;
10741071
}
10751072

1076-
using ChangeSet = fine_grained_dependencies::ModuleDepGraph::Changes::value_type;
1077-
static void
1078-
pruneChangeSetFromExternalDependency(ChangeSet &changes) {
1079-
// The changeset includes detritus from the graph that gets consed up
1080-
// in \c writePriorDependencyGraph. We need to ignore the fake
1081-
// source file provides nodes and the fake incremental external
1082-
// dependencies linked to them.
1083-
swift::erase_if(
1084-
changes, [&](fine_grained_dependencies::ModuleDepGraphNode *node) {
1085-
if (node->getKey().getKind() ==
1086-
fine_grained_dependencies::NodeKind::sourceFileProvide ||
1087-
node->getKey().getKind() ==
1088-
fine_grained_dependencies::NodeKind::
1089-
incrementalExternalDepend) {
1090-
return true;
1091-
}
1092-
if (node->getKey().getAspect() ==
1093-
fine_grained_dependencies::DeclAspect::implementation) {
1094-
return true;
1095-
}
1096-
return !node->getIsProvides();
1097-
});
1098-
}
1099-
1100-
SmallVector<const Job *, 16>
1101-
collectIncrementalExternallyDependentJobsFromDependencyGraph() {
1102-
SmallVector<const Job *, 16> ExternallyDependentJobs;
1103-
auto fallbackToExternalBehavior = [&](StringRef external) {
1104-
for (const auto cmd :
1105-
markIncrementalExternalInDepGraph(external)) {
1106-
ExternallyDependentJobs.push_back(cmd);
1107-
}
1108-
};
1109-
1110-
for (auto external : getFineGrainedDepGraph()
1111-
.getIncrementalExternalDependencies()) {
1112-
llvm::sys::fs::file_status depStatus;
1113-
// Can't `stat` this dependency? Treat it as a plain external
1114-
// dependency and drop schedule all of its consuming jobs to run.
1115-
if (llvm::sys::fs::status(external, depStatus)) {
1116-
fallbackToExternalBehavior(external);
1117-
continue;
1118-
}
1119-
1120-
// Is this module out of date? If not, just keep searching.
1121-
if (Comp.getLastBuildTime() >= depStatus.getLastModificationTime())
1122-
continue;
1123-
1124-
// Can we run a cross-module incremental build at all?
1125-
// If not, fall back.
1126-
if (!Comp.getEnableCrossModuleIncrementalBuild()) {
1127-
fallbackToExternalBehavior(external);
1128-
continue;
1129-
}
1130-
1131-
// If loading the buffer fails, the user may have deleted this external
1132-
// dependency or it could have become corrupted. We need to
1133-
// pessimistically schedule a rebuild to get dependent jobs to drop
1134-
// this dependency from their swiftdeps files if possible.
1135-
auto buffer = llvm::MemoryBuffer::getFile(external);
1136-
if (!buffer) {
1137-
fallbackToExternalBehavior(external);
1138-
continue;
1139-
}
1140-
1141-
// Cons up a fake `Job` to satisfy the incremental job tracing
1142-
// code's internal invariants.
1143-
const auto *externalJob = Comp.addExternalJob(
1144-
std::make_unique<Job>(Comp.getDerivedOutputFileMap(), external));
1145-
auto maybeChanges =
1146-
getFineGrainedDepGraph().loadFromSwiftModuleBuffer(
1147-
externalJob, *buffer.get(), Comp.getDiags());
1148-
1149-
// If the incremental dependency graph failed to load, fall back to
1150-
// treating this as plain external job.
1151-
if (!maybeChanges.hasValue()) {
1152-
fallbackToExternalBehavior(external);
1153-
continue;
1154-
}
1155-
1156-
// Prune away the detritus from the build record.
1157-
auto &changes = maybeChanges.getValue();
1158-
pruneChangeSetFromExternalDependency(changes);
1159-
1160-
for (auto *CMD : getFineGrainedDepGraph()
1161-
.findJobsToRecompileWhenNodesChange(changes)) {
1162-
if (CMD == externalJob) {
1163-
continue;
1164-
}
1165-
ExternallyDependentJobs.push_back(CMD);
1166-
}
1167-
}
1168-
noteBuildingJobs(ExternallyDependentJobs,
1169-
"because of incremental external dependencies");
1170-
return ExternallyDependentJobs;
1171-
}
1172-
11731073
/// Insert all jobs in \p Cmds (of descriptive name \p Kind) to the \c
11741074
/// TaskQueue, and clear \p Cmds.
11751075
template <typename Container>
@@ -1628,24 +1528,12 @@ namespace driver {
16281528
return getFineGrainedDepGraph().getExternalDependencies();
16291529
}
16301530

1631-
std::vector<StringRef>
1632-
getIncrementalExternalDependencies() const {
1633-
return getFineGrainedDepGraph()
1634-
.getIncrementalExternalDependencies();
1635-
}
1636-
16371531
std::vector<const Job*>
16381532
markExternalInDepGraph(StringRef externalDependency) {
16391533
return getFineGrainedDepGraph()
16401534
.findExternallyDependentUntracedJobs(externalDependency);
16411535
}
16421536

1643-
std::vector<const Job *>
1644-
markIncrementalExternalInDepGraph(StringRef externalDependency) {
1645-
return getFineGrainedDepGraph()
1646-
.findIncrementalExternallyDependentUntracedJobs(externalDependency);
1647-
}
1648-
16491537
std::vector<const Job *> findJobsToRecompileWhenWholeJobChanges(const Job *Cmd) {
16501538
return getFineGrainedDepGraph()
16511539
.findJobsToRecompileWhenWholeJobChanges(Cmd);
@@ -1771,76 +1659,6 @@ static void writeCompilationRecord(StringRef path, StringRef argsHash,
17711659
}
17721660
}
17731661

1774-
using SourceFileDepGraph = swift::fine_grained_dependencies::SourceFileDepGraph;
1775-
1776-
/// Render out the unified module dependency graph to the given \p path, which
1777-
/// is expected to be a path relative to the build record.
1778-
static void withPriorDependencyGraph(StringRef path,
1779-
const Compilation::Result &result,
1780-
llvm::function_ref<void(SourceFileDepGraph &&)> cont) {
1781-
// Building a source file dependency graph from the module dependency graph
1782-
// is a strange task on its face because a source file dependency graph is
1783-
// usually built for exactly one file. However, the driver is going to use
1784-
// some encoding tricks to get the dependencies for each incremental external
1785-
// dependency into one big file. Note that these tricks
1786-
// are undone in \c pruneChangeSetFromExternalDependency, so if you modify
1787-
// this you need to go fix that algorithm up as well. This is a diagrammatic
1788-
// view of the structure of the dependencies this function builds:
1789-
//
1790-
// SourceFile => interface <BUILD_RECORD>.external
1791-
// | - Incremetal External Dependency => interface <MODULE_1>.swiftmodule
1792-
// | | - <dependency> ...
1793-
// | | - <dependency> ...
1794-
// | | - <dependency> ...
1795-
// | - Incremetal External Dependency => interface <MODULE_2>.swiftmodule
1796-
// | | - <dependency> ...
1797-
// | | - <dependency> ...
1798-
// | - Incremetal External Dependency => interface <MODULE_3>.swiftmodule
1799-
// | - ...
1800-
//
1801-
// Where each <dependency> node has an arc back to its parent swiftmodule.
1802-
// That swiftmodule, in turn, takes the form of as an incremental external
1803-
// dependency. This formulation allows us to easily discern the original
1804-
// swiftmodule that a <dependency> came from just by examining that arc. This
1805-
// is used in integrate to "move" the <dependency> from the build record to
1806-
// the swiftmodule by swapping the key it uses.
1807-
using namespace swift::fine_grained_dependencies;
1808-
SourceFileDepGraph g;
1809-
const auto &resultModuleGraph = result.depGraph;
1810-
// Create the key for the entire external build record.
1811-
auto fileKey =
1812-
DependencyKey::createKeyForWholeSourceFile(DeclAspect::interface, path);
1813-
auto fileNodePair = g.findExistingNodePairOrCreateAndAddIfNew(fileKey, None);
1814-
for (StringRef incrExternalDep :
1815-
resultModuleGraph.getIncrementalExternalDependencies()) {
1816-
// Now make a node for each incremental external dependency.
1817-
auto interfaceKey =
1818-
DependencyKey(NodeKind::incrementalExternalDepend,
1819-
DeclAspect::interface, "", incrExternalDep.str());
1820-
auto ifaceNode = g.findExistingNodeOrCreateIfNew(interfaceKey, None,
1821-
false /* = !isProvides */);
1822-
resultModuleGraph.forEachNodeInJob(incrExternalDep, [&](const auto *node) {
1823-
// Reject
1824-
// 1) Implementation nodes: We don't care about the interface nodes
1825-
// for cross-module dependencies because the client cannot observe it
1826-
// by definition.
1827-
// 2) Source file nodes: we're about to define our own.
1828-
if (!node->getKey().isInterface() ||
1829-
node->getKey().getKind() == NodeKind::sourceFileProvide) {
1830-
return;
1831-
}
1832-
assert(node->getIsProvides() &&
1833-
"Found a node in module depdendencies that is not a provides!");
1834-
auto *newNode = new SourceFileDepGraphNode(
1835-
node->getKey(), node->getFingerprint(), /*isProvides*/ true);
1836-
g.addNode(newNode);
1837-
g.addArc(ifaceNode, newNode);
1838-
});
1839-
g.addArc(fileNodePair.getInterface(), ifaceNode);
1840-
}
1841-
return cont(std::move(g));
1842-
}
1843-
18441662
static void writeInputJobsToFilelist(llvm::raw_fd_ostream &out, const Job *job,
18451663
const file_types::ID infoType) {
18461664
// FIXME: Duplicated from ToolChains.cpp.
@@ -1937,22 +1755,10 @@ Compilation::performJobsImpl(std::unique_ptr<TaskQueue> &&TQ) {
19371755
State.populateInputInfoMap(InputInfo);
19381756
checkForOutOfDateInputs(Diags, InputInfo);
19391757

1940-
auto result = std::move(State).takeResult();
19411758
writeCompilationRecord(CompilationRecordPath, ArgsHash, BuildStartTime,
19421759
InputInfo);
1943-
if (EnableCrossModuleIncrementalBuild) {
1944-
// Write out our priors adjacent to the build record so we can pick
1945-
// the up in a subsequent build.
1946-
withPriorDependencyGraph(getExternalSwiftDepsFilePath(), result,
1947-
[&](SourceFileDepGraph &&g) {
1948-
writeFineGrainedDependencyGraphToPath(
1949-
Diags, getExternalSwiftDepsFilePath(), g);
1950-
});
1951-
}
1952-
return result;
1953-
} else {
1954-
return std::move(State).takeResult();
19551760
}
1761+
return std::move(State).takeResult();
19561762
}
19571763

19581764
Compilation::Result Compilation::performSingleCommand(const Job *Cmd) {

0 commit comments

Comments
 (0)