Forward iterator for F14VectorMap

Summary:
This diff adds a reverse iterator to F14VectorMap/Set; this is mainly useful because
if there have been no calls to erase(), the reverse iterator will visit the entries in
insertion order.

Reviewed By: nbronson

Differential Revision: D7173038

fbshipit-source-id: 0a64c4e753c9c36c6ac8b6141654b6f29f410580

folly/container/F14Map.h

@@ -869,6 +869,8 @@ class F14VectorMap
   using typename Super::const_iterator;
   using typename Super::iterator;
   using typename Super::key_type;
+  using reverse_iterator = typename Policy::ReverseIter;
+  using const_reverse_iterator = typename Policy::ConstReverseIter;
 
   F14VectorMap() noexcept(
       f14::detail::F14Table<Policy>::kDefaultConstructIsNoexcept)
@@ -882,6 +884,27 @@ class F14VectorMap
     this->table_.swap(rhs.table_);
   }
 
+  // ITERATION ORDER
+  //
+  // Deterministic iteration order for insert-only workloads is part of
+  // F14VectorMap's supported API: iterator is LIFO and reverse_iterator
+  // is FIFO.
+  //
+  // If there have been no calls to erase() then iterator and
+  // const_iterator enumerate entries in the opposite of insertion order.
+  // begin()->first is the key most recently inserted.  reverse_iterator
+  // and reverse_const_iterator, therefore, enumerate in LIFO (insertion)
+  // order for insert-only workloads.  Deterministic iteration order is
+  // only guaranteed if no keys were removed since the last time the
+  // map was empty.  Iteration order is preserved across rehashes and
+  // F14VectorMap copies and moves.
+  //
+  // iterator uses LIFO order so that erasing while iterating with begin()
+  // and end() is safe using the erase(it++) idiom, which is supported
+  // by std::map and std::unordered_map.  erase(iter) invalidates iter
+  // and all iterators before iter in the non-reverse iteration order.
+  // Every successful erase invalidates all reverse iterators.
+
   iterator begin() {
     return this->table_.linearBegin(this->size());
   }
@@ -902,6 +925,41 @@ class F14VectorMap
     return this->table_.linearEnd();
   }
 
+  reverse_iterator rbegin() {
+    return this->table_.values_;
+  }
+  const_reverse_iterator rbegin() const {
+    return crbegin();
+  }
+  const_reverse_iterator crbegin() const {
+    return this->table_.values_;
+  }
+
+  reverse_iterator rend() {
+    return this->table_.values_ + this->table_.size();
+  }
+  const_reverse_iterator rend() const {
+    return crend();
+  }
+  const_reverse_iterator crend() const {
+    return this->table_.values_ + this->table_.size();
+  }
+
+  // explicit conversions between iterator and reverse_iterator
+  iterator iter(reverse_iterator riter) {
+    return this->table_.iter(riter);
+  }
+  const_iterator iter(const_reverse_iterator riter) const {
+    return this->table_.iter(riter);
+  }
+
+  reverse_iterator riter(iterator it) {
+    return this->table_.riter(it);
+  }
+  const_reverse_iterator riter(const_iterator it) const {
+    return this->table_.riter(it);
+  }
+
  private:
   void eraseUnderlying(typename Policy::ItemIter underlying) {
     Alloc& a = this->table_.alloc();

folly/container/F14Set.h

@@ -631,6 +631,8 @@ class F14VectorSet
   using typename Super::const_iterator;
   using typename Super::iterator;
   using typename Super::key_type;
+  using reverse_iterator = typename Policy::ReverseIter;
+  using const_reverse_iterator = typename Policy::ConstReverseIter;
 
   F14VectorSet() noexcept(
       f14::detail::F14Table<Policy>::kDefaultConstructIsNoexcept)
@@ -644,6 +646,27 @@ class F14VectorSet
     this->table_.swap(rhs.table_);
   }
 
+  // ITERATION ORDER
+  //
+  // Deterministic iteration order for insert-only workloads is part of
+  // F14VectorSet's supported API: iterator is LIFO and reverse_iterator
+  // is FIFO.
+  //
+  // If there have been no calls to erase() then iterator and
+  // const_iterator enumerate entries in the opposite of insertion order.
+  // begin()->first is the key most recently inserted.  reverse_iterator
+  // and reverse_const_iterator, therefore, enumerate in LIFO (insertion)
+  // order for insert-only workloads.  Deterministic iteration order is
+  // only guaranteed if no keys were removed since the last time the
+  // set was empty.  Iteration order is preserved across rehashes and
+  // F14VectorSet copies and moves.
+  //
+  // iterator uses LIFO order so that erasing while iterating with begin()
+  // and end() is safe using the erase(it++) idiom, which is supported
+  // by std::set and std::unordered_set.  erase(iter) invalidates iter
+  // and all iterators before iter in the non-reverse iteration order.
+  // Every successful erase invalidates all reverse iterators.
+
   iterator begin() {
     return cbegin();
   }
@@ -664,6 +687,41 @@ class F14VectorSet
     return this->table_.linearEnd();
   }
 
+  reverse_iterator rbegin() {
+    return this->table_.values_;
+  }
+  const_reverse_iterator rbegin() const {
+    return crbegin();
+  }
+  const_reverse_iterator crbegin() const {
+    return this->table_.values_;
+  }
+
+  reverse_iterator rend() {
+    return this->table_.values_ + this->table_.size();
+  }
+  const_reverse_iterator rend() const {
+    return crend();
+  }
+  const_reverse_iterator crend() const {
+    return this->table_.values_ + this->table_.size();
+  }
+
+  // explicit conversions between iterator and reverse_iterator
+  iterator iter(reverse_iterator riter) {
+    return this->table_.iter(riter);
+  }
+  const_iterator iter(const_reverse_iterator riter) const {
+    return this->table_.iter(riter);
+  }
+
+  reverse_iterator riter(iterator it) {
+    return this->table_.riter(it);
+  }
+  const_reverse_iterator riter(const_iterator it) const {
+    return this->table_.riter(it);
+  }
+
  private:
   void eraseUnderlying(typename Policy::ItemIter underlying) {
     Alloc& a = this->table_.alloc();
@@ -682,10 +740,7 @@ class F14VectorSet
       tail.item() = index;
       auto p = std::addressof(values[index]);
       folly::assume(p != nullptr);
-      std::allocator_traits<Alloc>::construct(
-          a, p, std::move(values[tailIndex]));
-      std::allocator_traits<Alloc>::destroy(
-          a, std::addressof(values[tailIndex]));
+      this->table_.transfer(a, std::addressof(values[tailIndex]), p, 1);
     }
   }
 

folly/container/detail/F14Policy.h

@@ -728,8 +728,6 @@ class VectorContainerIterator : public BaseIter<ValuePtr, uint32_t> {
   ~VectorContainerIterator() = default;
 
   /*implicit*/ operator VectorContainerIterator<ValueConstPtr>() const {
-    // can we trust that fancy pointers are implicitly convertible to
-    // fancy const pointers?
     return VectorContainerIterator<ValueConstPtr>{current_, lowest_};
   }
 
@@ -826,6 +824,9 @@ class VectorContainerPolicy : public BasePolicy<
       kIsMap,
       VectorContainerIterator<typename AllocTraits::pointer>,
       ConstIter>;
+  using ConstReverseIter = typename AllocTraits::const_pointer;
+  using ReverseIter = std::
+      conditional_t<kIsMap, typename AllocTraits::pointer, ConstReverseIter>;
 
   using ValuePtr = typename AllocTraits::pointer;
 
@@ -1151,6 +1152,22 @@ class VectorContainerPolicy : public BasePolicy<
     return Iter{values_ + index, values_};
   }
 
+  Iter iter(ReverseIter it) {
+    return Iter{it, values_};
+  }
+
+  ConstIter iter(ConstReverseIter it) const {
+    return ConstIter{it, values_};
+  }
+
+  ReverseIter riter(Iter it) {
+    return it.current_;
+  }
+
+  ConstReverseIter riter(ConstIter it) const {
+    return it.current_;
+  }
+
   ValuePtr values_{nullptr};
 };
 

folly/container/test/F14MapTest.cpp

@@ -410,6 +410,47 @@ TEST(F14FastMap, simple) {
   runSimple<F14FastMap<std::string, std::string>>();
 }
 
+TEST(F14VectorMap, reverse_iterator) {
+  using TMap = F14VectorMap<uint64_t, uint64_t>;
+  TMap h;
+  auto populate = [](TMap& h, uint64_t lo, uint64_t hi) {
+    for (auto i = lo; i < hi; ++i) {
+      h.emplace(i, i);
+    }
+  };
+  auto verify = [](TMap const& h, uint64_t lo, uint64_t hi) {
+    auto loIt = h.find(lo);
+    EXPECT_NE(h.end(), loIt);
+    uint64_t val = lo;
+    for (auto rit = h.riter(loIt); rit != h.rend(); ++rit) {
+      EXPECT_EQ(val, rit->first);
+      EXPECT_EQ(val, rit->second);
+      TMap::const_iterator it = h.iter(rit);
+      EXPECT_EQ(val, it->first);
+      EXPECT_EQ(val, it->second);
+      val++;
+    }
+    EXPECT_EQ(hi, val);
+  };
+
+  size_t prevSize = 0;
+  size_t newSize = 1;
+  // verify iteration order across rehashes, copies, and moves
+  while (newSize < 10'000) {
+    populate(h, prevSize, newSize);
+    verify(h, 0, newSize);
+    verify(h, newSize / 2, newSize);
+
+    TMap h2{h};
+    verify(h2, 0, newSize);
+
+    h = std::move(h2);
+    verify(h, 0, newSize);
+    prevSize = newSize;
+    newSize *= 10;
+  }
+}
+
 TEST(F14ValueMap, rehash) {
   runRehash<F14ValueMap<std::string, std::string>>();
 }

folly/container/test/F14SetTest.cpp

@@ -374,6 +374,45 @@ TEST(F14FastSet, simple) {
   runSimple<F14FastSet<std::string>>();
 }
 
+TEST(F14VectorMap, reverse_iterator) {
+  using TSet = F14VectorSet<uint64_t>;
+  TSet h;
+  auto populate = [](TSet& h, uint64_t lo, uint64_t hi) {
+    for (auto i = lo; i < hi; ++i) {
+      h.insert(i);
+    }
+  };
+  auto verify = [](TSet const& h, uint64_t lo, uint64_t hi) {
+    auto loIt = h.find(lo);
+    EXPECT_NE(h.end(), loIt);
+    uint64_t val = lo;
+    for (auto rit = h.riter(loIt); rit != h.rend(); ++rit) {
+      EXPECT_EQ(val, *rit);
+      TSet::const_iterator it = h.iter(rit);
+      EXPECT_EQ(val, *it);
+      val++;
+    }
+    EXPECT_EQ(hi, val);
+  };
+
+  size_t prevSize = 0;
+  size_t newSize = 1;
+  // verify iteration order across rehashes, copies, and moves
+  while (newSize < 10'000) {
+    populate(h, prevSize, newSize);
+    verify(h, 0, newSize);
+    verify(h, newSize / 2, newSize);
+
+    TSet h2{h};
+    verify(h2, 0, newSize);
+
+    h = std::move(h2);
+    verify(h, 0, newSize);
+    prevSize = newSize;
+    newSize *= 10;
+  }
+}
+
 TEST(F14ValueSet, rehash) {
   runRehash<F14ValueSet<std::string>>();
 }