Skip to content

Commit 4c61354

Browse files
author
David Ungar
committed
Added some comments around incremental compilation.
1 parent ff132a7 commit 4c61354

File tree

4 files changed

+36
-4
lines changed

4 files changed

+36
-4
lines changed

include/swift/Driver/DependencyGraph.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,16 @@ class DependencyGraphImpl {
121121
llvm::StringMap<std::pair<std::vector<DependencyEntryTy>, DependencyMaskTy>> Dependencies;
122122

123123
/// The set of marked nodes.
124+
/// "Marked" means that everything provided by this node (i.e. Job) is
125+
/// dirty. Thus any file using any of these provides must be recompiled.
126+
/// (Only non-private entities are output as provides.)
127+
/// In other words, this Job "cascades"; the need to recompile it causes other
128+
/// recompilations. It is possible that the current code marks things that do
129+
/// not need to be marked. Nothing would break if that were the case.
130+
///
131+
/// "Marked" does NOT mean that any successors in the graph have been
132+
/// processed. The traversal routines use "Visited" to avoid endless
133+
/// "recursion".
124134
llvm::SmallPtrSet<const void *, 16> Marked;
125135

126136
/// A list of all external dependencies that cannot be resolved from just this
@@ -153,6 +163,12 @@ class DependencyGraphImpl {
153163
(void)newlyInserted;
154164
}
155165

166+
/// Starting at \p node, visit the transitive closure of every dependent node.
167+
/// Assume that the starting node is "cascading"; that all dependencies must
168+
/// be dirty if the start is dirty. Therefore, mark the start. For each
169+
/// visited node, add it to visited, and mark it if it cascades. The start
170+
/// node is NOT added to visited.
171+
156172
void markTransitive(SmallVectorImpl<const void *> &visited,
157173
const void *node, MarkTracerImpl *tracer = nullptr);
158174
bool markIntransitive(const void *node) {

include/swift/Driver/Job.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,9 +230,17 @@ class CommandOutput {
230230
class Job {
231231
public:
232232
enum class Condition {
233+
// There was no input map, or the map makred this Job as dirty
234+
// but the input didn't change.
235+
// Be maximally conservative with dependencies.
233236
Always,
237+
// The input changed, or this job was private or [it was not dirty but
238+
// primary output was missing].
234239
RunWithoutCascading,
240+
// The best case: input didn't change, output exists.
241+
// Only run if it depends on some other thing that changed.
235242
CheckDependencies,
243+
// Pessimal: run no matter what.
236244
NewlyAdded
237245
};
238246

lib/Driver/Compilation.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,9 @@ namespace driver {
206206
/// Jobs that incremental-mode has decided it can skip.
207207
CommandSet DeferredCommands;
208208

209-
/// Jobs in the initial set with Condition::Always, or lacking existing
209+
/// Jobs in the initial set with Condition::Always, and having an existing
210210
/// .swiftdeps files.
211+
/// Set by scheduleInitialJobs and used only by scheduleAdditionalJobs.
211212
SmallVector<const Job *, 16> InitialOutOfDateCommands;
212213

213214
/// Dependency graph for deciding which jobs are dirty (need running)
@@ -382,7 +383,7 @@ namespace driver {
382383
DeferredCommands.clear();
383384
}
384385

385-
/// Helper that attmepts to reload a job's .swiftdeps file after the job
386+
/// Helper that attempts to reload a job's .swiftdeps file after the job
386387
/// exits, and re-run transitive marking to ensure everything is properly
387388
/// invalidated by any new dependency edges introduced by it. If reloading
388389
/// fails, this can cause deferred jobs to be immediately scheduled.
@@ -679,6 +680,11 @@ namespace driver {
679680
}
680681

681682
/// Schedule all jobs we can from the initial list provided by Compilation.
683+
///
684+
/// It would probably be safe and simpler to markTransitive on the start
685+
/// nodes in the "Always" condition from the start instead of using
686+
/// markIntransitive and having later functions call markTransitive. That
687+
/// way markInstransitive would be an implemenation detail.
682688
void scheduleInitialJobs() {
683689
for (const Job *Cmd : Comp.getJobs()) {
684690
if (!Comp.getIncrementalBuildEnabled()) {
@@ -715,7 +721,9 @@ namespace driver {
715721
switch (Condition) {
716722
case Job::Condition::Always:
717723
if (Comp.getIncrementalBuildEnabled() && !DependenciesFile.empty()) {
724+
// Ensure dependents will get recompiled.
718725
InitialOutOfDateCommands.push_back(Cmd);
726+
// Mark this job as cascading.
719727
DepGraph.markIntransitive(Cmd);
720728
}
721729
LLVM_FALLTHROUGH;

lib/Driver/CompilationRecord.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ namespace swift {
1919
namespace driver {
2020
namespace compilation_record {
2121

22-
/// Compilation record files (.swiftdeps files) are YAML files composed of these
23-
/// top-level keys.
22+
/// Compilation record files (-master.swiftdeps files) are YAML files composed
23+
/// of these top-level keys.
2424
enum class TopLevelKey {
2525
/// The key for the Swift compiler version used to produce the compilation
2626
/// record.

0 commit comments

Comments
 (0)