Merge pull request #29539 from davidungar/type-fingerprints-off-by-default-3

[Incremental: Add a unit test for individual node dependency propagation]

Author: David Ungar

include/swift/AST/FineGrainedDependencies.h

@@ -799,6 +799,7 @@ class SourceFileDepGraph {
                    compoundNamesByRDK);
 
   static constexpr char noncascadingOrPrivatePrefix = '#';
+  static constexpr char nameFingerprintSeparator = ',';
 
   static std::string noncascading(std::string name);
 

include/swift/Driver/FineGrainedDependencyDriverGraph.h

@@ -437,7 +437,7 @@ class ModuleDepGraph {
   /// Call \p fn for each node whose key matches \p key.
   void
   forEachMatchingNode(const DependencyKey &key,
-                      function_ref<void(const ModuleDepGraphNode *)>) const;
+                      function_ref<void(ModuleDepGraphNode *)>) const;
 
   void forEachNodeInJob(StringRef swiftDeps,
                         function_ref<void(ModuleDepGraphNode *)>) const;

lib/AST/FineGrainedDependenciesSourceFileDepGraphConstructor.cpp

@@ -848,33 +848,54 @@ bool swift::fine_grained_dependencies::emitReferenceDependencies(
 static StringRef stripPrefix(const StringRef name) {
   return name.ltrim(SourceFileDepGraph::noncascadingOrPrivatePrefix);
 }
+static StringRef stripFingerprint(const StringRef nameAndFingerprint) {
+  return nameAndFingerprint.split(SourceFileDepGraph::nameFingerprintSeparator)
+           .first;
+}
+static StringRef stripName(const StringRef nameAndFingerprint) {
+  return nameAndFingerprint.split(SourceFileDepGraph::nameFingerprintSeparator)
+           .second;
+}
+static std::string extractName(const StringRef prefixNameFingerprint) {
+  return stripFingerprint(stripPrefix(prefixNameFingerprint)).str();
+}
+static Optional<std::string> extractFingerprint(
+         const StringRef prefixNameFingerprint) {
+  const auto fp = stripName(stripPrefix(prefixNameFingerprint));
+  return fp.empty() ? None : Optional<std::string>(fp.str());
+}
 
 static std::vector<ContextNameFingerprint>
 getBaseNameProvides(ArrayRef<std::string> simpleNames) {
   std::vector<ContextNameFingerprint> result;
   for (StringRef n : simpleNames)
-    result.push_back(ContextNameFingerprint("", stripPrefix(n).str(), None));
+    result.push_back(ContextNameFingerprint("", extractName(n),
+                     extractFingerprint(n)));
   return result;
 }
 
 static std::vector<ContextNameFingerprint>
 getMangledHolderProvides(ArrayRef<std::string> simpleNames) {
   std::vector<ContextNameFingerprint> result;
   for (StringRef n : simpleNames)
-    result.push_back(ContextNameFingerprint(stripPrefix(n).str(), "", None));
+    result.push_back(ContextNameFingerprint(extractName(n), "",
+                     extractFingerprint(n)));
   return result;
 }
 
 static std::vector<ContextNameFingerprint> getCompoundProvides(
     ArrayRef<std::pair<std::string, std::string>> compoundNames) {
   std::vector<ContextNameFingerprint> result;
   for (const auto &p : compoundNames)
-    result.push_back(ContextNameFingerprint(stripPrefix(p.first),
-                                            stripPrefix(p.second), None));
+    result.push_back(ContextNameFingerprint(extractName(p.first),
+                                            extractName(p.second),
+                                            extractFingerprint(p.second)));
   return result;
 }
 
-static bool cascades(const std::string &s) { return s.empty() || s[0] != SourceFileDepGraph::noncascadingOrPrivatePrefix; }
+static bool cascades(const std::string &s) {
+  return s.empty() || s[0] != SourceFileDepGraph::noncascadingOrPrivatePrefix;
+}
 
 // Use '_' as a prefix for a file-private member
 static bool isPrivate(const std::string &s) {
@@ -904,7 +925,8 @@ getCompoundDepends(
     // (On Linux, the compiler needs more verbosity than:
     //  result.push_back({{n, "", false}, cascades(n)});
     result.push_back(
-        std::make_pair(std::make_tuple(stripPrefix(n), std::string(), false), cascades(n)));
+        std::make_pair(std::make_tuple(stripPrefix(n), std::string(), false),
+                       cascades(n)));
   }
   for (auto &p : compoundNames) {
     // Likewise, for Linux expand the following out:
@@ -932,7 +954,8 @@ SourceFileDepGraph SourceFileDepGraph::simulateLoad(
   SourceFileDepGraphConstructor c(
     swiftDepsFilename, includePrivateDeps, hadCompilationError, interfaceHash,
     getSimpleDepends(simpleNamesByRDK[dependsTopLevel]),
-    getCompoundDepends(simpleNamesByRDK[dependsNominal], compoundNamesByRDK[dependsMember]),
+    getCompoundDepends(simpleNamesByRDK[dependsNominal],
+                       compoundNamesByRDK[dependsMember]),
     getSimpleDepends(simpleNamesByRDK[dependsDynamicLookup]),
     getExternalDepends(simpleNamesByRDK[dependsExternal]),
     {}, // precedence groups

lib/Driver/FineGrainedDependencyDriverGraph.cpp

@@ -171,6 +171,11 @@ ModuleDepGraph::findJobsToRecompileWhenNodesChange<
     std::unordered_set<ModuleDepGraphNode *>>(
     const std::unordered_set<ModuleDepGraphNode *> &);
 
+template std::vector<const Job *>
+ModuleDepGraph::findJobsToRecompileWhenNodesChange<
+    std::vector<ModuleDepGraphNode *>>(
+    const std::vector<ModuleDepGraphNode *> &);
+
 std::vector<std::string> ModuleDepGraph::computeSwiftDepsFromNodes(
     ArrayRef<const ModuleDepGraphNode *> nodes) const {
   llvm::StringSet<> swiftDepsOfNodes;
@@ -441,7 +446,7 @@ void ModuleDepGraph::forEachNode(
 
 void ModuleDepGraph::forEachMatchingNode(
     const DependencyKey &key,
-    function_ref<void(const ModuleDepGraphNode *)> fn) const {
+    function_ref<void(ModuleDepGraphNode *)> fn) const {
   nodeMap.forEachValueMatching(
       key, [&](const std::string &, ModuleDepGraphNode *n) { fn(n); });
 }

unittests/Driver/FineGrainedDependencyGraphTests.cpp

@@ -808,3 +808,19 @@ TEST(ModuleDepGraph, MutualInterfaceHash) {
   const auto jobs = graph.findJobsToRecompileWhenWholeJobChanges(&job0);
   EXPECT_TRUE(contains(jobs, &job1));
 }
+
+TEST(ModuleDepGraph, DisabledTypeBodyFingerprints) {
+  ModuleDepGraph graph(/*EnableTypeFingerprints=*/ false);
+
+  graph.simulateLoad(&job0, {{dependsNominal, {"B2"}}});
+  graph.simulateLoad(&job1, {{providesNominal, {"B1", "B2"}}});
+  graph.simulateLoad(&job2, {{dependsNominal, {"B1"}}});
+
+  {
+  const auto jobs = graph.findJobsToRecompileWhenWholeJobChanges(&job1);
+  EXPECT_EQ(3u, jobs.size());
+  EXPECT_TRUE(contains(jobs, &job0));
+  EXPECT_TRUE(contains(jobs, &job1));
+  EXPECT_TRUE(contains(jobs, &job2));
+  }
+}

unittests/Driver/TypeBodyFingerprintsDependencyGraphTests.cpp

@@ -808,3 +808,24 @@ TEST(ModuleDepGraphWithTypeBodyFingerprints, MutualInterfaceHash) {
   const auto jobs = graph.findJobsToRecompileWhenWholeJobChanges(&job0);
   EXPECT_TRUE(contains(jobs, &job1));
 }
+
+TEST(ModuleDepGraph, EnabledTypeBodyFingerprints) {
+  ModuleDepGraph graph(/*EnableTypeFingerprints=*/ true);
+
+  graph.simulateLoad(&job0, {{dependsNominal, {"B2"}}});
+  graph.simulateLoad(&job1, {{providesNominal, {"B1", "B2"}}});
+  graph.simulateLoad(&job2, {{dependsNominal, {"B1"}}});
+
+
+  const DependencyKey k = DependencyKey(NodeKind::nominal,
+                                        DeclAspect::interface, "B1", "");
+  std::vector<ModuleDepGraphNode *> changedNodes;
+  graph.forEachMatchingNode(
+          k,
+          [&](ModuleDepGraphNode* n) {changedNodes.push_back(n);});
+  {
+    const auto jobs = graph.findJobsToRecompileWhenNodesChange(changedNodes);
+    EXPECT_TRUE(contains(jobs, &job2));
+    EXPECT_FALSE(contains(jobs, &job0));
+    }
+}