EvictingCacheMap::erase(const_iterator)

Summary: Allows erase without a re-hash into index_ when we already have an iterator

Reviewed By: aary

Differential Revision: D13928691

fbshipit-source-id: d662dc72cd8a07cdbd874cb6a3517f68ef320d80

folly/container/EvictingCacheMap.h

@@ -120,10 +120,24 @@ class EvictingCacheMap {
                             TIterator,
                             Value,
                             boost::bidirectional_traversal_tag> {
+   private:
+    struct Enabler {};
+
    public:
     iterator_base() {}
+
     explicit iterator_base(TIterator it)
         : iterator_base::iterator_adaptor_(it) {}
+
+    template <typename V, typename I>
+    /* implicit */ iterator_base(
+        iterator_base<V, I> const& other,
+        std::enable_if_t<
+            std::is_convertible<V, Value>::value &&
+                std::is_convertible<I, TIterator>::value,
+            Enabler> = Enabler())
+        : iterator_base::iterator_adaptor_(other.base()) {}
+
     Value& dereference() const {
       return this->base_reference()->pr;
     }
@@ -292,14 +306,24 @@ class EvictingCacheMap {
    */
   bool erase(const TKey& key) {
     auto it = findInIndex(key);
-    if (it == index_.end()) {
+    if (it != index_.end()) {
+      erase(const_iterator(lru_.iterator_to(*it)));
+      return true;
+    }
     return false;
   }
-    auto node = &(*it);
+
+  /**
+   * Erase the key-value pair associated with pos
+   * @param pos iterator to the element to be erased
+   * @return iterator to the following element or end() if pos was the last
+   *     element
+   */
+  iterator erase(const_iterator pos) {
+    auto* node = const_cast<Node*>(&(*pos.base()));
     std::unique_ptr<Node> nptr(node);
-    lru_.erase(lru_.iterator_to(*node));
-    index_.erase(it);
-    return true;
+    index_.erase(index_.iterator_to(*node));
+    return iterator(lru_.erase(pos.base()));
   }
 
   /**

folly/container/test/EvictingCacheMapTest.cpp

@@ -438,6 +438,18 @@ TEST(EvictingCacheMap, DestructorInvocationTest) {
   sum = 0;
   map.prune(100);
   EXPECT_EQ((90 * 91) / 2 + (10 * 189) / 2, sum);
+
+  sum = 0;
+  map.set(3, SumInt(3, &sum));
+  map.set(2, SumInt(2, &sum));
+  map.set(1, SumInt(1, &sum));
+  EXPECT_EQ(0, sum);
+  EXPECT_EQ(2, map.erase(map.find(1))->second.val);
+  EXPECT_EQ(1, sum);
+  EXPECT_EQ(map.end(), map.erase(map.findWithoutPromotion(3)));
+  EXPECT_EQ(4, sum);
+  map.prune(1);
+  EXPECT_EQ(6, sum);
 }
 
 TEST(EvictingCacheMap, LruSanityTest) {