From dcea6905c5065b8bf9810558b7363cb64cc5151b Mon Sep 17 00:00:00 2001
From: Jordan Rose <jordan_rose@apple.com>
Date: Mon, 3 Dec 2018 21:47:50 -0800
Subject: [PATCH] Request Evaluator: save malloc traffic by canonicalizing
 AnyRequests

Use the Dependencies map to standardize on a single allocation for
any particular Request. We could probably go all the way to unique
ownership here, but I didn't want to rock the boat.

Results in minor speedups compiling the stdlib. (I was experimenting
with using a new, auto-cached Request for a hot lookup path, which may
or may not turn out to be worth it, but even without that we can take
this.)
---
 include/swift/AST/Evaluator.h | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/swift/AST/Evaluator.h b/include/swift/AST/Evaluator.h
index c842a721253d0..f1440d5a7f090 100644
--- a/include/swift/AST/Evaluator.h
+++ b/include/swift/AST/Evaluator.h
@@ -259,7 +259,7 @@ class Evaluator {
   llvm::Expected<typename Request::OutputType>
   operator()(const Request &request) {
     // Check for a cycle.
-    if (checkDependency(AnyRequest(request))) {
+    if (checkDependency(getCanonicalRequest(request))) {
       return llvm::Error(
         llvm::make_unique<CyclicalRequestError<Request>>(request, *this));
     }
@@ -293,6 +293,17 @@ class Evaluator {
   void clearCache() { cache.clear(); }
 
 private:
+  template <typename Request>
+  const AnyRequest &getCanonicalRequest(const Request &request) {
+    // FIXME: DenseMap ought to let us do this with one hash lookup.
+    auto iter = dependencies.find_as(request);
+    if (iter != dependencies.end())
+      return iter->first;
+    auto insertResult = dependencies.insert({AnyRequest(request), {}});
+    assert(insertResult.second && "just checked if the key was already there");
+    return insertResult.first->first;
+  }
+
   /// Diagnose a cycle detected in the evaluation of the given
   /// request.
   void diagnoseCycle(const AnyRequest &request);
@@ -335,7 +346,7 @@ class Evaluator {
   getResultUncached(const Request &request) {
     // Clear out the dependencies on this request; we're going to recompute
     // them now anyway.
-    dependencies[AnyRequest(request)].clear();
+    dependencies.find_as(request)->second.clear();
 
     PrettyStackTraceRequest<Request> prettyStackTrace(request);
 
@@ -377,7 +388,6 @@ class Evaluator {
       typename std::enable_if<!Request::hasExternalCache>::type * = nullptr>
   llvm::Expected<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_as(request);
     if (known != cache.end()) {
@@ -390,7 +400,7 @@ class Evaluator {
       return result;
 
     // Cache the result.
-    cache.insert({AnyRequest(request), *result});
+    cache.insert({getCanonicalRequest(request), *result});
     return result;
   }