Merge pull request #84265 from hamishknight/tracksuit

[Frontend] Only enable request reference tracking when needed

Author: hamishknight

docs/RequestEvaluator.md

@@ -66,9 +66,8 @@ To define a request as a dependency source, it must implement an accessor for th
 
 ## Open Projects
 
-The request-evaluator is relatively new to the Swift compiler, having been introduced in mid-2018. There are a number of improvements that can be made to the evaluator itself and how it is used in the compiler:
+There are a number of improvements that can be made to the evaluator itself and how it is used in the compiler:
 
-* The evaluator uses a `DenseMap<AnyRequest, AnyValue>` as its cache: we can almost certainly do better with per-request-kind caches that don't depend on so much type erasure.
 * Explore how best to cache data structures in the evaluator. For example, caching `std::vector<T>` or `std::string` implies that we'll make copies of the underlying data structure each time we access the data. Could we automatically intern the data into an allocation arena owned by the evaluator, and vend `ArrayRef<T>` and `StringRef` to clients instead?
 * Cycle diagnostics are far too complicated and produce very poor results. Consider replacing the current `diagnoseCycle`/`noteCycleStep` scheme with a single method that produces summary information (e.g., a short summary string + source location information) and provides richer diagnostics from that string.
 * The `isCached()` check to determine whether a specific instance of a request is worth caching may be at the wrong level, because one generally has to duplicate effort (or worse, code!) to make the decision in `isCached()`. Consider whether the `evaluator()` function could return something special to say "produce this value without caching" vs. the normal "produce this value with caching".

include/swift/AST/Evaluator.h

@@ -22,6 +22,7 @@
 #include "swift/AST/EvaluatorDependencies.h"
 #include "swift/AST/RequestCache.h"
 #include "swift/Basic/AnyValue.h"
+#include "swift/Basic/Assertions.h"
 #include "swift/Basic/Debug.h"
 #include "swift/Basic/LangOptions.h"
 #include "swift/Basic/Statistic.h"
@@ -208,6 +209,7 @@ class Evaluator {
   void enumerateReferencesInFile(
       const SourceFile *SF,
       evaluator::DependencyRecorder::ReferenceEnumerator f) const {
+    ASSERT(recorder.isRecordingEnabled() && "Dep recording should be enabled");
     return recorder.enumerateReferencesInFile(SF, f);
   }
 
@@ -414,6 +416,8 @@ class Evaluator {
             typename std::enable_if<Request::isDependencySink>::type * = nullptr>
   void handleDependencySinkRequest(const Request &r,
                                    const typename Request::OutputType &o) {
+    if (!recorder.isRecordingEnabled())
+      return;
     evaluator::DependencyCollector collector(recorder);
     r.writeDependencySink(collector, o);
   }
@@ -425,6 +429,8 @@ class Evaluator {
   template <typename Request,
             typename std::enable_if<Request::isDependencySource>::type * = nullptr>
   void handleDependencySourceRequest(const Request &r) {
+    if (!recorder.isRecordingEnabled())
+      return;
     auto source = r.readDependencySource(recorder);
     if (!source.isNull() && source.get()->isPrimary()) {
       recorder.handleDependencySourceRequest(r, source.get());

include/swift/AST/EvaluatorDependencies.h

@@ -94,6 +94,9 @@ class DependencyRecorder {
 public:
   DependencyRecorder(bool shouldRecord) : shouldRecord(shouldRecord) {}
 
+  /// Whether dependency recording is enabled.
+  bool isRecordingEnabled() const { return shouldRecord; }
+
   /// Push a new empty set onto the activeRequestReferences stack.
   template<typename Request>
   void beginRequest();

lib/Frontend/Frontend.cpp

@@ -308,6 +308,16 @@ void CompilerInstance::recordPrimaryInputBuffer(unsigned BufID) {
   PrimaryBufferIDs.insert(BufID);
 }
 
+static bool shouldEnableRequestReferenceTracking(const CompilerInstance &CI) {
+  // Enable request reference dependency tracking when we're either writing
+  // dependencies for incremental mode, verifying dependencies, or collecting
+  // stats.
+  auto &opts = CI.getInvocation().getFrontendOptions();
+  return opts.InputsAndOutputs.hasReferenceDependenciesFilePath() ||
+         opts.EnableIncrementalDependencyVerifier ||
+         !opts.StatsOutputDir.empty();
+}
+
 bool CompilerInstance::setUpASTContextIfNeeded() {
   if (FrontendOptions::doesActionBuildModuleFromInterface(
           Invocation.getFrontendOptions().RequestedAction) &&
@@ -318,10 +328,8 @@ bool CompilerInstance::setUpASTContextIfNeeded() {
     return false;
   }
 
-  // For the time being, we only need to record dependencies in batch mode
-  // and single file builds.
-  Invocation.getLangOptions().RecordRequestReferences
-    = !isWholeModuleCompilation();
+  Invocation.getLangOptions().RecordRequestReferences =
+      shouldEnableRequestReferenceTracking(*this);
 
   Context.reset(ASTContext::get(
       Invocation.getLangOptions(), Invocation.getTypeCheckerOptions(),

test/Driver/createCompilerInvocation.swift

@@ -40,3 +40,10 @@
 // NOOUTPUT_ARGS-DAG: -typecheck
 // NOOUTPUT_ARGS-DAG: -module-name
 // NOOUTPUT_ARGS: Frontend Arguments END
+
+// Make sure that '-incremental' is ignored, we don't want SourceKit to run with
+// reference dependency tracking enabled. The legacy driver simply doesn't
+// implement incremental compilation, but make sure when we switch to the new
+// driver this doesn't start failing.
+// RUN: %swift-ide-test_plain -test-createCompilerInvocation -incremental %S/Input/main.swift | %FileCheck --check-prefix INCREMENTAL %s
+// INCREMENTAL-NOT: emit-reference-dependencies