Merge pull request swiftlang#18617 from jrose-apple/any-requestions

Request evaluator: Cut down on the cost of AnyRequest in two ways

Author: jrose-apple

include/swift/AST/AnyRequest.h

@@ -20,6 +20,7 @@
 #include "swift/Basic/TypeID.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include <string>
 
 namespace llvm {
@@ -49,8 +50,14 @@ class DiagnosticEngine;
 ///       void noteCycleStep(DiagnosticEngine &diags) const;
 ///
 class AnyRequest {
+  friend llvm::DenseMapInfo<swift::AnyRequest>;
+
+  static hash_code hashForHolder(uint64_t typeID, hash_code requestHash) {
+    return hash_combine(hash_value(typeID), requestHash);
+  }
+
   /// Abstract base class used to hold the specific request kind.
-  class HolderBase {
+  class HolderBase : public llvm::RefCountedBase<HolderBase> {
   public:
     /// The type ID of the request being stored.
     const uint64_t typeID;
@@ -61,7 +68,7 @@ class AnyRequest {
   protected:
     /// Initialize base with type ID and hash code.
     HolderBase(uint64_t typeID, hash_code hash)
-      : typeID(typeID), hash(hash_combine(hash_value(typeID), hash)) { }
+      : typeID(typeID), hash(AnyRequest::hashForHolder(typeID, hash)) { }
 
   public:
     virtual ~HolderBase();
@@ -128,7 +135,7 @@ class AnyRequest {
   } storageKind = StorageKind::Normal;
 
   /// The data stored in this value.
-  std::shared_ptr<HolderBase> stored;
+  llvm::IntrusiveRefCntPtr<HolderBase> stored;
 
   AnyRequest(StorageKind storageKind) : storageKind(storageKind) {
     assert(storageKind != StorageKind::Normal);
@@ -159,17 +166,18 @@ class AnyRequest {
 
   /// Construct a new instance with the given value.
   template<typename T>
-  AnyRequest(T&& value) : storageKind(StorageKind::Normal) {
+  explicit AnyRequest(T&& value) : storageKind(StorageKind::Normal) {
     using ValueType =
-      typename std::remove_cv<typename std::remove_reference<T>::type>::type;
-    stored.reset(new Holder<ValueType>(std::forward<T>(value)));
+        typename std::remove_cv<typename std::remove_reference<T>::type>::type;
+    stored = llvm::IntrusiveRefCntPtr<HolderBase>(
+        new Holder<ValueType>(std::forward<T>(value)));
   }
 
   /// Cast to a specific (known) type.
   template<typename Request>
   const Request &castTo() const {
     assert(stored->typeID == TypeID<Request>::value && "wrong type in cast");
-    return static_cast<const Holder<Request> *>(stored.get())->value;
+    return static_cast<const Holder<Request> *>(stored.get())->request;
   }
 
   /// Try casting to a specific (known) type, returning \c nullptr on
@@ -179,7 +187,7 @@ class AnyRequest {
     if (stored->typeID != TypeID<Request>::value)
       return nullptr;
 
-    return &static_cast<const Holder<Request> *>(stored.get())->value;
+    return &static_cast<const Holder<Request> *>(stored.get())->request;
   }
 
   /// Diagnose a cycle detected for this request.
@@ -248,10 +256,25 @@ namespace llvm {
     static unsigned getHashValue(const swift::AnyRequest &request) {
       return hash_value(request);
     }
+    template <typename Request>
+    static unsigned getHashValue(const Request &request) {
+      return swift::AnyRequest::hashForHolder(swift::TypeID<Request>::value,
+                                              hash_value(request));
+    }
     static bool isEqual(const swift::AnyRequest &lhs,
                         const swift::AnyRequest &rhs) {
       return lhs == rhs;
     }
+    template <typename Request>
+    static bool isEqual(const Request &lhs,
+                        const swift::AnyRequest &rhs) {
+      if (rhs == getEmptyKey() || rhs == getTombstoneKey())
+        return false;
+      const Request *rhsRequest = rhs.getAs<Request>();
+      if (!rhsRequest)
+        return false;
+      return lhs == *rhsRequest;
+    }
   };
 
 } // end namespace llvm

include/swift/AST/Evaluator.h

@@ -213,13 +213,13 @@ class Evaluator {
   template<typename Request>
   typename Request::OutputType operator()(const Request &request) {
     // Check for a cycle.
-    if (checkDependency(request))
+    if (checkDependency(AnyRequest(request)))
       return request.breakCycle();
 
     // Make sure we remove this from the set of active requests once we're
     // done.
     SWIFT_DEFER {
-      assert(activeRequests.back() == request);
+      assert(activeRequests.back().castTo<Request>() == request);
       activeRequests.pop_back();
     };
 
@@ -283,7 +283,7 @@ class Evaluator {
   typename Request::OutputType getResultUncached(const Request &request) {
     // Clear out the dependencies on this request; we're going to recompute
     // them now anyway.
-    dependencies[request].clear();
+    dependencies[AnyRequest(request)].clear();
 
     PrettyStackTraceRequest<Request> prettyStackTrace(request);
 
@@ -319,19 +319,17 @@ class Evaluator {
       typename Request,
       typename std::enable_if<!Request::hasExternalCache>::type * = nullptr>
   typename Request::OutputType getResultCached(const Request &request) {
-    AnyRequest anyRequest{request};
-
     // If we already have an entry for this request in the cache, return it.
-    auto known = cache.find(anyRequest);
+    auto known = cache.find_as(request);
     if (known != cache.end()) {
-      return known->second.castTo<typename Request::OutputType>();
+      return known->second.template castTo<typename Request::OutputType>();
     }
 
     // Compute the result.
     auto result = getResultUncached(request);
 
     // Cache the result.
-    cache.insert({anyRequest, result});
+    cache.insert({AnyRequest(request), result});
     return result;
   }
 
@@ -349,8 +347,14 @@ class Evaluator {
                          bool lastChild) const;
 
   /// Print the dependencies of the given request as a tree.
-  void printDependencies(const AnyRequest &request,
-                         llvm::raw_ostream &out) const;
+  template <typename Request>
+  void printDependencies(const Request &request, llvm::raw_ostream &out) const {
+    std::string prefixStr;
+    llvm::DenseSet<AnyRequest> visitedAnywhere;
+    llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
+    printDependencies(AnyRequest(request), out, visitedAnywhere,
+                      visitedAlongPath, { }, prefixStr, /*lastChild=*/true);
+  }
 
   /// Dump the dependencies of the given request to the debugging stream
   /// as a tree.

lib/AST/Evaluator.cpp

@@ -191,15 +191,6 @@ void Evaluator::printDependencies(
   }
 }
 
-void Evaluator::printDependencies(const AnyRequest &request,
-                                  llvm::raw_ostream &out) const {
-  std::string prefixStr;
-  llvm::DenseSet<AnyRequest> visitedAnywhere;
-  llvm::SmallVector<AnyRequest, 4> visitedAlongPath;
-  printDependencies(request, out, visitedAnywhere, visitedAlongPath, { },
-                    prefixStr, /*lastChild=*/true);
-}
-
 void Evaluator::dumpDependencies(const AnyRequest &request) const {
   printDependencies(request, llvm::dbgs());
 }