Merge pull request swiftlang#28867 from davidungar/enable-disable-off-by-default

[Driver, Incremental] Add enable- and disable- only-one-dependency-file flags off-by-default

Author: David Ungar

include/swift/Driver/Compilation.h

@@ -257,6 +257,23 @@ class Compilation {
   /// limit filelists will be used.
   size_t FilelistThreshold;
 
+  /// Because each frontend job outputs the same info in its .d file, only do it
+  /// on the first job that actually runs. Write out dummies for the rest of the
+  /// jobs. This hack saves a lot of time in the build system when incrementally
+  /// building a project with many files. Record if a scheduled job has already
+  /// added -emit-dependency-path.
+  bool HaveAlreadyAddedDependencyPath = false;
+
+public:
+  /// When set, only the first scheduled frontend job gets the argument needed
+  /// to produce a make-style dependency file. The other jobs create dummy files
+  /// in the driver. This hack speeds up incremental compilation by reducing the
+  /// time for the build system to read each dependency file, which are all
+  /// identical. This optimization can be disabled by passing
+  /// -disable-only-one-dependency-file on the command line.
+  const bool OnlyOneDependencyFile;
+
+private:
   /// Scaffolding to permit experimentation with finer-grained dependencies and
   /// faster rebuilds.
   const bool EnableFineGrainedDependencies;
@@ -309,6 +326,7 @@ class Compilation {
               bool SaveTemps = false,
               bool ShowDriverTimeCompilation = false,
               std::unique_ptr<UnifiedStatsReporter> Stats = nullptr,
+              bool OnlyOneDependencyFile = false,
               bool EnableFineGrainedDependencies = false,
               bool VerifyFineGrainedDependencyGraphAfterEveryImport = false,
               bool EmitFineGrainedDependencyDotFileAfterEveryImport = false,
@@ -427,6 +445,16 @@ class Compilation {
     return FilelistThreshold;
   }
 
+  /// Since every make-style dependency file contains
+  /// the same information, incremental builds are sped up by only emitting one
+  /// of those files. Since the build system expects to see the files existing,
+  /// create dummy files for those jobs that don't emit real dependencies.
+  /// \param path The dependency file path
+  /// \param addDependencyPath A function to add an -emit-dependency-path
+  /// argument
+  void addDependencyPathOrCreateDummy(StringRef path,
+                                      function_ref<void()> addDependencyPath);
+
   UnifiedStatsReporter *getStatsReporter() const {
     return Stats.get();
   }

include/swift/Driver/Job.h

@@ -194,6 +194,11 @@ class CommandOutput {
   /// first primary input.
   StringRef getAdditionalOutputForType(file_types::ID type) const;
 
+  /// Assuming (and asserting) that there are one or more input pairs, return true if there exists
+  /// an _additional_ (not primary) output of type \p type associated with the
+  /// first primary input.
+  bool hasAdditionalOutputForType(file_types::ID type) const;
+
   /// Return a vector of additional (not primary) outputs of type \p type
   /// associated with the primary inputs.
   ///

include/swift/Option/Options.td

@@ -139,6 +139,16 @@ def enable_fine_grained_dependencies :
 Flag<["-"], "enable-fine-grained-dependencies">, Flags<[FrontendOption, HelpHidden]>,
 HelpText<"Experimental work-in-progress to be more selective about incremental recompilation">;
 
+
+def enable_only_one_dependency_file :
+Flag<["-"], "enable-only-one-dependency-file">, Flags<[DoesNotAffectIncrementalBuild]>,
+ HelpText<"Enables incremental build optimization that only produces one dependencies file">;
+
+def disable_only_one_dependency_file :
+Flag<["-"], "disable-only-one-dependency-file">, Flags<[DoesNotAffectIncrementalBuild]>,
+ HelpText<"Disables incremental build optimization that only produces one dependencies file">;
+ 
+
 def enable_source_range_dependencies :
 Flag<["-"], "enable-source-range-dependencies">, Flags<[]>,
 HelpText<"Try using source range information">;

lib/Driver/Compilation.cpp

@@ -48,6 +48,7 @@
 
 #include "CompilationRecord.h"
 
+#include <fstream>
 #include <signal.h>
 
 #define DEBUG_TYPE "batch-mode"
@@ -121,6 +122,7 @@ Compilation::Compilation(DiagnosticEngine &Diags,
                          bool SaveTemps,
                          bool ShowDriverTimeCompilation,
                          std::unique_ptr<UnifiedStatsReporter> StatsReporter,
+                         bool OnlyOneDependencyFile,
                          bool EnableFineGrainedDependencies,
                          bool VerifyFineGrainedDependencyGraphAfterEveryImport,
                          bool EmitFineGrainedDependencyDotFileAfterEveryImport,
@@ -149,14 +151,16 @@ Compilation::Compilation(DiagnosticEngine &Diags,
     ShowDriverTimeCompilation(ShowDriverTimeCompilation),
     Stats(std::move(StatsReporter)),
     FilelistThreshold(FilelistThreshold),
+    OnlyOneDependencyFile(OnlyOneDependencyFile),
     EnableFineGrainedDependencies(EnableFineGrainedDependencies),
     VerifyFineGrainedDependencyGraphAfterEveryImport(
       VerifyFineGrainedDependencyGraphAfterEveryImport),
     EmitFineGrainedDependencyDotFileAfterEveryImport(
       EmitFineGrainedDependencyDotFileAfterEveryImport),
     FineGrainedDependenciesIncludeIntrafileOnes(
       FineGrainedDependenciesIncludeIntrafileOnes),
-    EnableSourceRangeDependencies(EnableSourceRangeDependencies) {
+    EnableSourceRangeDependencies(EnableSourceRangeDependencies)
+    {
     if (CompareIncrementalSchemes)
       IncrementalComparator.emplace(
       // Ensure the references are to inst vars, NOT arguments
@@ -2032,3 +2036,20 @@ unsigned Compilation::countSwiftInputs() const {
       ++inputCount;
   return inputCount;
 }
+
+void Compilation::addDependencyPathOrCreateDummy(
+    StringRef depPath, function_ref<void()> addDependencyPath) {
+
+  if (!OnlyOneDependencyFile) {
+    addDependencyPath();
+    return;
+  }
+  if (!HaveAlreadyAddedDependencyPath) {
+    addDependencyPath();
+    HaveAlreadyAddedDependencyPath = true;
+  } else if (!depPath.empty()) {
+    // Create dummy empty file
+    std::error_code EC;
+    llvm::raw_fd_ostream(depPath, EC, llvm::sys::fs::F_None);
+  }
+}

lib/Driver/Driver.cpp

@@ -951,6 +951,11 @@ Driver::buildCompilation(const ToolChain &TC,
         ArgList->hasArg(options::OPT_driver_time_compilation);
     std::unique_ptr<UnifiedStatsReporter> StatsReporter =
         createStatsReporter(ArgList.get(), Inputs, OI, DefaultTargetTriple);
+
+    const bool OnlyOneDependencyFile =
+        ArgList->hasFlag(options::OPT_enable_only_one_dependency_file,
+                         options::OPT_disable_only_one_dependency_file, false);
+
     // relies on the new dependency graph
     const bool EnableFineGrainedDependencies =
         ArgList->hasArg(options::OPT_enable_fine_grained_dependencies);
@@ -984,6 +989,7 @@ Driver::buildCompilation(const ToolChain &TC,
         SaveTemps,
         ShowDriverTimeCompilation,
         std::move(StatsReporter),
+        OnlyOneDependencyFile,
         EnableFineGrainedDependencies,
         VerifyFineGrainedDependencyGraphAfterEveryImport,
         EmitFineGrainedDependencyDotFileAfterEveryImport,
@@ -2421,60 +2427,74 @@ static bool hasExistingAdditionalOutput(CommandOutput &output,
   return false;
 }
 
-static void addAuxiliaryOutput(
+static llvm::SmallString<128> computeAuxiliaryOutputPath(
     Compilation &C, CommandOutput &output, file_types::ID outputType,
     const TypeToPathMap *outputMap, StringRef workingDirectory,
     StringRef outputPath = StringRef(),
     llvm::opt::OptSpecifier requireArg = llvm::opt::OptSpecifier()) {
 
   if (hasExistingAdditionalOutput(output, outputType, outputPath))
-    return;
+    return {};
 
-  StringRef outputMapPath;
   if (outputMap) {
     auto iter = outputMap->find(outputType);
-    if (iter != outputMap->end())
-      outputMapPath = iter->second;
+    if (iter != outputMap->end()) {
+      StringRef outputMapPath = iter->second;
+      // Prefer a path from the OutputMap.
+      if (!outputMapPath.empty())
+        return outputMapPath;
+    }
   }
+  if (!outputPath.empty())
+    return outputPath;
 
-  if (!outputMapPath.empty()) {
-    // Prefer a path from the OutputMap.
-    output.setAdditionalOutputForType(outputType, outputMapPath);
-  } else if (!outputPath.empty()) {
-    output.setAdditionalOutputForType(outputType, outputPath);
-  } else if (requireArg.isValid() && !C.getArgs().getLastArg(requireArg)) {
+  if (requireArg.isValid() && !C.getArgs().getLastArg(requireArg)) {
     // This auxiliary output only exists if requireArg is passed, but it
     // wasn't this time.
-    return;
-  } else {
-    // Put the auxiliary output file next to "the" primary output file.
-    //
-    // FIXME: when we're in WMO and have multiple primary outputs, we derive the
-    // additional filename here from the _first_ primary output name, which
-    // means that in the derived OFM (in Job.cpp) the additional output will
-    // have a possibly-surprising name. But that's only half the problem: it
-    // also get associated with the first primary _input_, even when there are
-    // multiple primary inputs; really it should be associated with the build as
-    // a whole -- derived OFM input "" -- but that's a more general thing to
-    // fix.
-    llvm::SmallString<128> path;
-    if (output.getPrimaryOutputType() != file_types::TY_Nothing)
-      path = output.getPrimaryOutputFilenames()[0];
-    else if (!output.getBaseInput(0).empty())
-      path = llvm::sys::path::filename(output.getBaseInput(0));
-    else {
-      formFilenameFromBaseAndExt(C.getOutputInfo().ModuleName, /*newExt=*/"",
-                                 workingDirectory, path);
-    }
-    assert(!path.empty());
-
-    bool isTempFile = C.isTemporaryFile(path);
-    llvm::sys::path::replace_extension(
-        path, file_types::getExtension(outputType));
-    output.setAdditionalOutputForType(outputType, path);
-    if (isTempFile)
-      C.addTemporaryFile(path);
+    return {};
   }
+
+  // Put the auxiliary output file next to "the" primary output file.
+  //
+  // FIXME: when we're in WMO and have multiple primary outputs, we derive the
+  // additional filename here from the _first_ primary output name, which
+  // means that in the derived OFM (in Job.cpp) the additional output will
+  // have a possibly-surprising name. But that's only half the problem: it
+  // also get associated with the first primary _input_, even when there are
+  // multiple primary inputs; really it should be associated with the build as
+  // a whole -- derived OFM input "" -- but that's a more general thing to
+  // fix.
+  llvm::SmallString<128> path;
+  if (output.getPrimaryOutputType() != file_types::TY_Nothing)
+    path = output.getPrimaryOutputFilenames()[0];
+  else if (!output.getBaseInput(0).empty())
+    path = llvm::sys::path::filename(output.getBaseInput(0));
+  else {
+    formFilenameFromBaseAndExt(C.getOutputInfo().ModuleName, /*newExt=*/"",
+                               workingDirectory, path);
+  }
+  assert(!path.empty());
+
+  const bool isTempFile = C.isTemporaryFile(path);
+  llvm::sys::path::replace_extension(path,
+                                     file_types::getExtension(outputType));
+  if (isTempFile)
+    C.addTemporaryFile(path);
+  return path;
+}
+
+static void addAuxiliaryOutput(
+    Compilation &C, CommandOutput &output, file_types::ID outputType,
+    const TypeToPathMap *outputMap, StringRef workingDirectory,
+    StringRef outputPath = StringRef(),
+    llvm::opt::OptSpecifier requireArg = llvm::opt::OptSpecifier()) {
+
+  const auto path =
+      computeAuxiliaryOutputPath(C, output, outputType, outputMap,
+                                 workingDirectory, outputPath, requireArg);
+  if (path.empty())
+    return;
+  output.setAdditionalOutputForType(outputType, path);
 }
 
 static void addDiagFileOutputForPersistentPCHAction(
@@ -3053,8 +3073,12 @@ void Driver::chooseDependenciesOutputPaths(Compilation &C,
                                            llvm::SmallString<128> &Buf,
                                            CommandOutput *Output) const {
   if (C.getArgs().hasArg(options::OPT_emit_dependencies)) {
-    addAuxiliaryOutput(C, *Output, file_types::TY_Dependencies, OutputMap,
-                       workingDirectory);
+    auto depPath = computeAuxiliaryOutputPath(
+        C, *Output, file_types::TY_Dependencies, OutputMap, workingDirectory);
+    C.addDependencyPathOrCreateDummy(depPath, [&] {
+      addAuxiliaryOutput(C, *Output, file_types::TY_Dependencies, OutputMap,
+                         workingDirectory);
+    });
   }
   if (C.getIncrementalBuildEnabled()) {
     file_types::forEachIncrementalOutputType([&](file_types::ID type) {

lib/Driver/Job.cpp

@@ -236,6 +236,10 @@ CommandOutput::getAdditionalOutputsForType(file_types::ID Type) const {
   return V;
 }
 
+bool CommandOutput::hasAdditionalOutputForType(file_types::ID type) const {
+  return AdditionalOutputTypes.count(type);
+}
+
 StringRef CommandOutput::getAnyOutputForType(file_types::ID Type) const {
   if (PrimaryOutputType == Type)
     return getPrimaryOutputFilename();

lib/Driver/ToolChain.cpp

@@ -238,6 +238,11 @@ bool ToolChain::jobIsBatchable(const Compilation &C, const Job *A) const {
   auto const *CJActA = dyn_cast<const CompileJobAction>(&A->getSource());
   if (!CJActA)
     return false;
+  // When having only one job output a dependency file, that job is not
+  // batchable since it has an oddball set of additional output types.
+  if (C.OnlyOneDependencyFile &&
+      A->getOutput().hasAdditionalOutputForType(file_types::TY_Dependencies))
+    return false;
   return findSingleSwiftInput(CJActA) != nullptr;
 }