From f778bdefdd9663aa78c31ffc7773e31bcae4fb39 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Wed, 16 May 2018 20:59:27 +1000
Subject: [PATCH] Avoid repeated HashMap lookups in
 `opt_normalize_projection_type`.

There is a hot path through `opt_normalize_projection_type`:
- `try_start` does a cache lookup (#1).
- The result is a `NormalizedTy`.
- There are no unresolved type vars, so we call `complete`.
- `complete` does *another* cache lookup (#2), then calls
  `SnapshotMap::insert`.
- `insert` does *another* cache lookup (#3), inserting the same value
  that's already in the cache.

This patch optimizes this hot path by introducing `complete_normalized`,
for use when the value is known in advance to be a `NormalizedTy`. It
always avoids lookup #2. Furthermore, if the `NormalizedTy`'s
obligations are empty (the common case), we know that lookup #3 would be
a no-op, so we avoid it, while inserting a Noop into the `SnapshotMap`'s
undo log.
---
 src/librustc/traits/project.rs                | 19 ++++++++++++++++++-
 .../snapshot_map/mod.rs                       |  6 ++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs
index bfa32f8e7faf3..da8f086b9c55d 100644
--- a/src/librustc/traits/project.rs
+++ b/src/librustc/traits/project.rs
@@ -596,7 +596,7 @@ fn opt_normalize_projection_type<'a, 'b, 'gcx, 'tcx>(
             // Once we have inferred everything we need to know, we
             // can ignore the `obligations` from that point on.
             if !infcx.any_unresolved_type_vars(&ty.value) {
-                infcx.projection_cache.borrow_mut().complete(cache_key);
+                infcx.projection_cache.borrow_mut().complete_normalized(cache_key, &ty);
                 ty.obligations = vec![];
             }
 
@@ -1682,6 +1682,23 @@ impl<'tcx> ProjectionCache<'tcx> {
         }));
     }
 
+    /// A specialized version of `complete` for when the key's value is known
+    /// to be a NormalizedTy.
+    pub fn complete_normalized(&mut self, key: ProjectionCacheKey<'tcx>, ty: &NormalizedTy<'tcx>) {
+        // We want to insert `ty` with no obligations. If the existing value
+        // already has no obligations (as is common) we can use `insert_noop`
+        // to do a minimal amount of work -- the HashMap insertion is skipped,
+        // and minimal changes are made to the undo log.
+        if ty.obligations.is_empty() {
+            self.map.insert_noop();
+        } else {
+            self.map.insert(key, ProjectionCacheEntry::NormalizedTy(Normalized {
+                value: ty.value,
+                obligations: vec![]
+            }));
+        }
+    }
+
     /// Indicates that trying to normalize `key` resulted in
     /// ambiguity. No point in trying it again then until we gain more
     /// type information (in which case, the "fully resolved" key will
diff --git a/src/librustc_data_structures/snapshot_map/mod.rs b/src/librustc_data_structures/snapshot_map/mod.rs
index cede6f147821b..6ee8c3579f543 100644
--- a/src/librustc_data_structures/snapshot_map/mod.rs
+++ b/src/librustc_data_structures/snapshot_map/mod.rs
@@ -67,6 +67,12 @@ impl<K, V> SnapshotMap<K, V>
         }
     }
 
+    pub fn insert_noop(&mut self) {
+        if !self.undo_log.is_empty() {
+            self.undo_log.push(UndoLog::Noop);
+        }
+    }
+
     pub fn remove(&mut self, key: K) -> bool {
         match self.map.remove(&key) {
             Some(old_value) => {