Merge pull request #15665 from graydon/everybody-gets-a-tracer

Author: swift-ci

docs/RequestEvaluator.md

@@ -61,7 +61,6 @@ The request-evaluator is relatively new to the Swift compiler, having been intro
 * Add API to the evaluator to add a specific (request, result) pair to the cache, and make it "stick" even if the cache is cleared. This API should be usable from places in the compiler that inject specific known state, such as when the Clang importer synthesizes ASTs.
 * 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?
 * Sometimes the compiler crashes or otherwise fails due to a cycle detected by the evaluator but (currently) not diagnosed. Add some logic to record cycles encountered during compilation, and if the compiler crashes (or otherwise fails), dump the cycles at the end of execution to help with debugging.
-* Automatically create appropriate [`FrontendStatsTracer`](https://github.com/apple/swift/blob/master/include/swift/Basic/Statistic.h) instances when evaluating requests that embed AST references to anything the stats-tracing logic can handle (`Decl*`, `ProtocolConformance*`, etc.). Try to do this automatically for `SimpleRequest` so specific requests don't need to opt in for it to work. 
 * 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".
 * Try to eliminate more boilerplate from subclasses of [`SimpleRequest`](https://github.com/apple/swift/blob/master/include/swift/AST/SimpleRequest.h). We are going to have a *lot* of requests.

include/swift/AST/Evaluator.h

@@ -22,6 +22,7 @@
 #include "swift/Basic/AnyValue.h"
 #include "swift/Basic/CycleDiagnosticKind.h"
 #include "swift/Basic/Defer.h"
+#include "swift/Basic/Statistic.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SetVector.h"
@@ -338,7 +339,8 @@ class Evaluator {
 
     PrettyStackTraceRequest<Request> prettyStackTrace(request);
 
-    /// Update statistics.
+    // Trace and/or count statistics.
+    FrontendStatsTracer statsTracer = make_tracer(stats, request);
     if (stats) reportEvaluatedRequest(*stats, request);
 
     return getRequestFunction<Request>()(request, *this);

include/swift/AST/SimpleRequest.h

@@ -20,6 +20,7 @@
 #include "swift/AST/DiagnosticEngine.h"
 #include "swift/AST/DiagnosticsCommon.h"
 #include "swift/Basic/SimpleDisplay.h"
+#include "swift/Basic/Statistic.h"
 #include "swift/Basic/TypeID.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/STLExtras.h"
@@ -160,8 +161,12 @@ class SimpleRequest {
     out << TypeID<Derived>::getName();
     simple_display(out, request.storage);
   }
-};
 
+  friend FrontendStatsTracer
+  make_tracer(UnifiedStatsReporter *Reporter, const Derived &request) {
+    return make_tracer(Reporter, TypeID<Derived>::getName(), request.storage);
+  }
+};
 }
 
 #endif // SWIFT_BASIC_SIMPLEREQUEST_H

include/swift/Basic/Statistic.h

@@ -18,6 +18,9 @@
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/Timer.h"
 
+#include <thread>
+#include <tuple>
+
 #define SWIFT_FUNC_STAT                                                 \
   do {                                                                  \
     static llvm::Statistic FStat =                                      \
@@ -59,7 +62,11 @@ class ProtocolConformance;
 class Expr;
 class SILFunction;
 class FrontendStatsTracer;
+class Pattern;
+class SourceFile;
 class SourceManager;
+class Stmt;
+class TypeRepr;
 
 class UnifiedStatsReporter {
 
@@ -125,6 +132,7 @@ class UnifiedStatsReporter {
   SmallString<128> TraceFilename;
   SmallString<128> ProfileDirname;
   llvm::TimeRecord StartedTime;
+  std::thread::id MainThreadID;
 
   // This is unique_ptr because NamedRegionTimer is non-copy-constructable.
   std::unique_ptr<llvm::NamedRegionTimer> Timer;
@@ -169,6 +177,7 @@ class UnifiedStatsReporter {
 
   AlwaysOnDriverCounters &getDriverCounters();
   AlwaysOnFrontendCounters &getFrontendCounters();
+  void flushTracesAndProfiles();
   void noteCurrentProcessExitStatus(int);
   void saveAnyFrontendStatsEvents(FrontendStatsTracer const &T, bool IsEntry);
 };
@@ -216,6 +225,14 @@ class FrontendStatsTracer
                       const Expr *E);
   FrontendStatsTracer(UnifiedStatsReporter *Reporter, StringRef EventName,
                       const SILFunction *F);
+  FrontendStatsTracer(UnifiedStatsReporter *Reporter, StringRef EventName,
+                      const SourceFile *F);
+  FrontendStatsTracer(UnifiedStatsReporter *Reporter, StringRef EventName,
+                      const Stmt *S);
+  FrontendStatsTracer(UnifiedStatsReporter *Reporter, StringRef EventName,
+                      const Pattern *P);
+  FrontendStatsTracer(UnifiedStatsReporter *Reporter, StringRef EventName,
+                      const TypeRepr *TR);
 };
 
 // In particular cases, we do know how to format traced entities: we declare
@@ -245,6 +262,18 @@ template <>
 const UnifiedStatsReporter::TraceFormatter *
 FrontendStatsTracer::getTraceFormatter<const SILFunction *>();
 
+template<> const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const SourceFile *>();
+
+template<> const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const Stmt *>();
+
+template<> const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const Pattern *>();
+
+template<> const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const TypeRepr *>();
+
 // Provide inline definitions for the delegating constructors.  These avoid
 // introducing a circular dependency between libParse and libSIL.  They are
 // marked as `inline` explicitly to prevent ODR violations due to multiple
@@ -281,5 +310,88 @@ inline FrontendStatsTracer::FrontendStatsTracer(UnifiedStatsReporter *R,
                                                 const SILFunction *F)
     : FrontendStatsTracer(R, S, F, getTraceFormatter<const SILFunction *>()) {}
 
+inline FrontendStatsTracer::FrontendStatsTracer(UnifiedStatsReporter *R,
+                                                StringRef S,
+                                                const SourceFile *SF)
+    : FrontendStatsTracer(R, S, SF, getTraceFormatter<const SourceFile *>()) {}
+
+inline FrontendStatsTracer::FrontendStatsTracer(UnifiedStatsReporter *R,
+                                                StringRef S,
+                                                const Stmt *ST)
+    : FrontendStatsTracer(R, S, ST, getTraceFormatter<const Stmt *>()) {}
+
+inline FrontendStatsTracer::FrontendStatsTracer(UnifiedStatsReporter *R,
+                                                StringRef S,
+                                                const Pattern *P)
+    : FrontendStatsTracer(R, S, P, getTraceFormatter<const Pattern *>()) {}
+
+inline FrontendStatsTracer::FrontendStatsTracer(UnifiedStatsReporter *R,
+                                                StringRef S,
+                                                const TypeRepr *TR)
+    : FrontendStatsTracer(R, S, TR, getTraceFormatter<const TypeRepr *>()) {}
+
+/// Utilities for constructing TraceFormatters from entities in the request-evaluator:
+
+template <typename T>
+typename std::enable_if<
+    std::is_constructible<FrontendStatsTracer, UnifiedStatsReporter *,
+                          StringRef, const T *>::value,
+    FrontendStatsTracer>::type
+make_tracer_direct(UnifiedStatsReporter *Reporter, StringRef Name, T *Value) {
+  return FrontendStatsTracer(Reporter, Name, static_cast<const T *>(Value));
+}
+
+template <typename T>
+typename std::enable_if<
+    std::is_constructible<FrontendStatsTracer, UnifiedStatsReporter *,
+                          StringRef, const T *>::value,
+    FrontendStatsTracer>::type
+make_tracer_direct(UnifiedStatsReporter *Reporter, StringRef Name,
+                   const T *Value) {
+  return FrontendStatsTracer(Reporter, Name, Value);
+}
+
+template <typename T>
+typename std::enable_if<
+    !std::is_constructible<FrontendStatsTracer, UnifiedStatsReporter *,
+                           StringRef, const T *>::value,
+    FrontendStatsTracer>::type
+make_tracer_direct(UnifiedStatsReporter *Reporter, StringRef Name, T *Value) {
+  return FrontendStatsTracer(Reporter, Name);
+}
+
+template <typename T>
+typename std::enable_if<!std::is_pointer<T>::value, FrontendStatsTracer>::type
+make_tracer_direct(UnifiedStatsReporter *Reporter, StringRef Name, T Value) {
+  return FrontendStatsTracer(Reporter, Name);
+}
+
+template <typename T> struct is_pointerunion : std::false_type {};
+template <typename T, typename U>
+struct is_pointerunion<llvm::PointerUnion<T, U>> : std::true_type {};
+
+template <typename T, typename U>
+FrontendStatsTracer make_tracer_pointerunion(UnifiedStatsReporter *Reporter,
+                                             StringRef Name,
+                                             llvm::PointerUnion<T, U> Value) {
+  if (Value.template is<T>())
+    return make_tracer_direct(Reporter, Name, Value.template get<T>());
+  else
+    return make_tracer_direct(Reporter, Name, Value.template get<U>());
+}
+
+template <typename T>
+typename std::enable_if<!is_pointerunion<T>::value, FrontendStatsTracer>::type
+make_tracer_pointerunion(UnifiedStatsReporter *Reporter, StringRef Name,
+                         T Value) {
+  return make_tracer_direct(Reporter, Name, Value);
+}
+
+template <typename First, typename... Rest>
+FrontendStatsTracer make_tracer(UnifiedStatsReporter *Reporter, StringRef Name,
+                                std::tuple<First, Rest...> Value) {
+  return make_tracer_pointerunion(Reporter, Name, std::get<0>(Value));
+}
+
 } // namespace swift
 #endif // SWIFT_BASIC_STATISTIC_H

include/swift/Basic/Timer.h

@@ -20,6 +20,22 @@
 namespace swift {
   /// A convenience class for declaring a timer that's part of the Swift
   /// compilation timers group.
+  ///
+  /// Please don't use this class directly for anything other than the flat,
+  /// top-level compilation-phase timing numbers; unadorned SharedTimers are
+  /// enabled, summed and reported via -debug-time-compilation, using LLVM's
+  /// built-in logic for timer groups, and that logic doesn't work right if
+  /// there's any nesting or reentry in timers at all (crashes on reentry,
+  /// simply mis-reports nesting). Additional SharedTimers also confuse users
+  /// who are expecting to see only top-level phase timings when they pass
+  /// -debug-time-compilation.
+  ///
+  /// Instead, please use FrontendStatsTracer objects and the -stats-output-dir
+  /// subsystem in include/swift/Basic/Statistic.h. In addition to not
+  /// interfering with users passing -debug-time-compilation, the
+  /// FrontendStatsTracer objects automatically instantiate nesting-safe and
+  /// reentry-safe SharedTimers themselves, as well as supporting event and
+  /// source-entity tracing and profiling.
   class SharedTimer {
     enum class State {
       Initial,

lib/AST/Expr.cpp

@@ -2168,20 +2168,23 @@ void KeyPathExpr::Component::setSubscriptIndexHashableConformances(
   }
 }
 
-// See swift/Basic/Statistic.h for declaration: this enables tracing Decls, is
+// See swift/Basic/Statistic.h for declaration: this enables tracing Exprs, is
 // defined here to avoid too much layering violation / circular linkage
 // dependency.
 
 struct ExprTraceFormatter : public UnifiedStatsReporter::TraceFormatter {
   void traceName(const void *Entity, raw_ostream &OS) const {
-    // Exprs don't have names.
+    if (!Entity)
+      return;
+    const Expr *E = static_cast<const Expr *>(Entity);
+    OS << Expr::getKindName(E->getKind());
   }
   void traceLoc(const void *Entity, SourceManager *SM,
                 clang::SourceManager *CSM, raw_ostream &OS) const {
     if (!Entity)
       return;
-    const Expr *D = static_cast<const Expr *>(Entity);
-    D->getSourceRange().print(OS, *SM, false);
+    const Expr *E = static_cast<const Expr *>(Entity);
+    E->getSourceRange().print(OS, *SM, false);
   }
 };
 

lib/AST/Module.cpp

@@ -34,6 +34,7 @@
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/Basic/Compiler.h"
 #include "swift/Basic/SourceManager.h"
+#include "swift/Basic/Statistic.h"
 #include "swift/Parse/Token.h"
 #include "swift/Syntax/SyntaxNodes.h"
 #include "clang/Basic/Module.h"
@@ -1545,3 +1546,28 @@ const ModuleDecl* ModuleEntity::getAsSwiftModule() const {
     return SwiftMod;
   return nullptr;
 }
+
+// See swift/Basic/Statistic.h for declaration: this enables tracing SourceFiles, is
+// defined here to avoid too much layering violation / circular linkage
+// dependency.
+
+struct SourceFileTraceFormatter : public UnifiedStatsReporter::TraceFormatter {
+  void traceName(const void *Entity, raw_ostream &OS) const {
+    if (!Entity)
+      return;
+    const SourceFile *SF = static_cast<const SourceFile *>(Entity);
+    OS << llvm::sys::path::filename(SF->getFilename());
+  }
+  void traceLoc(const void *Entity, SourceManager *SM,
+                clang::SourceManager *CSM, raw_ostream &OS) const {
+    // SourceFiles don't have SourceLocs of their own; they contain them.
+  }
+};
+
+static SourceFileTraceFormatter TF;
+
+template<>
+const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const SourceFile *>() {
+  return &TF;
+}

lib/AST/NameLookup.cpp

@@ -1552,7 +1552,6 @@ TinyPtrVector<ValueDecl *> NominalTypeDecl::lookupDirect(
                                                   DeclName name,
                                                   bool ignoreNewExtensions) {
   ASTContext &ctx = getASTContext();
-  FrontendStatsTracer tracer(ctx.Stats, "lookup-direct", this);
   if (auto s = ctx.Stats) {
     ++s->getFrontendCounters().NominalTypeLookupDirectCount;
   }

lib/AST/Pattern.cpp

@@ -20,6 +20,7 @@
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/TypeLoc.h"
+#include "swift/Basic/Statistic.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace swift;
@@ -431,4 +432,33 @@ SourceLoc ExprPattern::getLoc() const {
 SourceRange ExprPattern::getSourceRange() const {
   return getSubExpr()->getSourceRange();
 }
-  
+
+// See swift/Basic/Statistic.h for declaration: this enables tracing Patterns, is
+// defined here to avoid too much layering violation / circular linkage
+// dependency.
+
+struct PatternTraceFormatter : public UnifiedStatsReporter::TraceFormatter {
+  void traceName(const void *Entity, raw_ostream &OS) const {
+    if (!Entity)
+      return;
+    const Pattern *P = static_cast<const Pattern *>(Entity);
+    if (const NamedPattern *NP = dyn_cast<NamedPattern>(P)) {
+      OS << NP->getBoundName();
+    }
+  }
+  void traceLoc(const void *Entity, SourceManager *SM,
+                clang::SourceManager *CSM, raw_ostream &OS) const {
+    if (!Entity)
+      return;
+    const Pattern *P = static_cast<const Pattern *>(Entity);
+    P->getSourceRange().print(OS, *SM, false);
+  }
+};
+
+static PatternTraceFormatter TF;
+
+template<>
+const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const Pattern *>() {
+  return &TF;
+}

lib/AST/Stmt.cpp

@@ -19,6 +19,7 @@
 #include "swift/AST/Decl.h"
 #include "swift/AST/Expr.h"
 #include "swift/AST/Pattern.h"
+#include "swift/Basic/Statistic.h"
 #include "llvm/ADT/PointerUnion.h"
 
 using namespace swift;
@@ -431,3 +432,31 @@ SwitchStmt *SwitchStmt::create(LabeledStmtInfo LabelInfo, SourceLoc SwitchLoc,
                           theSwitch->getTrailingObjects<ASTNode>());
   return theSwitch;
 }
+
+// See swift/Basic/Statistic.h for declaration: this enables tracing Stmts, is
+// defined here to avoid too much layering violation / circular linkage
+// dependency.
+
+struct StmtTraceFormatter : public UnifiedStatsReporter::TraceFormatter {
+  void traceName(const void *Entity, raw_ostream &OS) const {
+    if (!Entity)
+      return;
+    const Stmt *S = static_cast<const Stmt *>(Entity);
+    OS << Stmt::getKindName(S->getKind());
+  }
+  void traceLoc(const void *Entity, SourceManager *SM,
+                clang::SourceManager *CSM, raw_ostream &OS) const {
+    if (!Entity)
+      return;
+    const Stmt *S = static_cast<const Stmt *>(Entity);
+    S->getSourceRange().print(OS, *SM, false);
+  }
+};
+
+static StmtTraceFormatter TF;
+
+template<>
+const UnifiedStatsReporter::TraceFormatter*
+FrontendStatsTracer::getTraceFormatter<const Stmt *>() {
+  return &TF;
+}