F14 fallback mode cleanups

Summary:
This diff adds operator=(initializer_list) to the F14 fallback
implementations, relaxes the guarantees provided by visitAllocationClasses
for fallbacks (so that the current implementation can be considered
correct), enables additional tests, and avoids a bit of logic repetition.
Changes should only be made to the fallback mode and tests, and this diff
should not change the set of platforms for which fallback mode is enabled.

Reviewed By: shixiao

Differential Revision: D9664700

fbshipit-source-id: 1f3fc6f0c48009bb3e5fd249506b42f54d93df43

folly/container/F14Map.h

@@ -54,31 +54,39 @@ class F14BasicMap : public std::unordered_map<K, M, H, E, A> {
   using Super = std::unordered_map<K, M, H, E, A>;
 
  public:
+  using typename Super::pointer;
+  using typename Super::value_type;
+
+  F14BasicMap() = default;
+
   using Super::Super;
-  F14BasicMap() : Super() {}
 
   //// PUBLIC - F14 Extensions
 
-  typename Super::size_type getAllocatedMemorySize() const {
-    auto bc = this->bucket_count();
-    return (bc == 1 ? 0 : bc) * sizeof(typename Super::pointer) +
-        this->size() * sizeof(StdNodeReplica<K, typename Super::value_type, H>);
+  // exact for libstdc++, approximate for others
+  std::size_t getAllocatedMemorySize() const {
+    std::size_t rv = 0;
+    visitAllocationClasses(
+        [&](std::size_t bytes, std::size_t n) { rv += bytes * n; });
+    return rv;
   }
 
+  // exact for libstdc++, approximate for others
   template <typename V>
   void visitAllocationClasses(V&& visitor) const {
     auto bc = this->bucket_count();
     if (bc > 1) {
-      visitor(bc * sizeof(typename Super::pointer), 1);
+      visitor(bc * sizeof(pointer), 1);
+    }
+    if (this->size() > 0) {
+      visitor(sizeof(StdNodeReplica<K, value_type, H>), this->size());
     }
-    visitor(
-        sizeof(StdNodeReplica<K, typename Super::value_type, H>), this->size());
   }
 
   template <typename V>
   void visitContiguousRanges(V&& visitor) const {
-    for (typename Super::value_type const& entry : *this) {
-      typename Super::value_type const* b = std::addressof(entry);
+    for (value_type const& entry : *this) {
+      value_type const* b = std::addressof(entry);
       visitor(b, b + 1);
     }
   }
@@ -91,8 +99,16 @@ class F14ValueMap : public f14::detail::F14BasicMap<K, M, H, E, A> {
   using Super = f14::detail::F14BasicMap<K, M, H, E, A>;
 
  public:
+  using typename Super::value_type;
+
+  F14ValueMap() = default;
+
   using Super::Super;
-  F14ValueMap() : Super() {}
+
+  F14ValueMap& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 template <typename K, typename M, typename H, typename E, typename A>
@@ -100,8 +116,16 @@ class F14NodeMap : public f14::detail::F14BasicMap<K, M, H, E, A> {
   using Super = f14::detail::F14BasicMap<K, M, H, E, A>;
 
  public:
+  using typename Super::value_type;
+
+  F14NodeMap() = default;
+
   using Super::Super;
-  F14NodeMap() : Super() {}
+
+  F14NodeMap& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 template <typename K, typename M, typename H, typename E, typename A>
@@ -109,8 +133,16 @@ class F14VectorMap : public f14::detail::F14BasicMap<K, M, H, E, A> {
   using Super = f14::detail::F14BasicMap<K, M, H, E, A>;
 
  public:
+  using typename Super::value_type;
+
+  F14VectorMap() = default;
+
   using Super::Super;
-  F14VectorMap() : Super() {}
+
+  F14VectorMap& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 } // namespace folly

folly/container/F14Set.h

@@ -50,33 +50,39 @@ class F14BasicSet : public std::unordered_set<K, H, E, A> {
   using Super = std::unordered_set<K, H, E, A>;
 
  public:
+  using typename Super::pointer;
+  using typename Super::value_type;
+
+  F14BasicSet() = default;
+
   using Super::Super;
-  F14BasicSet() : Super() {}
 
   //// PUBLIC - F14 Extensions
 
-  typename Super::size_type getAllocatedMemorySize() const {
-    auto bc = this->bucket_count();
-    return (bc == 1 ? 0 : bc) * sizeof(typename Super::pointer) +
-        this->size() * sizeof(StdNodeReplica<K, typename Super::value_type, H>);
+  // exact for libstdc++, approximate for others
+  std::size_t getAllocatedMemorySize() const {
+    std::size_t rv = 0;
+    visitAllocationClasses(
+        [&](std::size_t bytes, std::size_t n) { rv += bytes * n; });
+    return rv;
   }
 
-  // TODO(T33376370): this implementation is incorrect for android. Fix it or
-  // disable it.
+  // exact for libstdc++, approximate for others
   template <typename V>
   void visitAllocationClasses(V&& visitor) const {
     auto bc = this->bucket_count();
     if (bc > 1) {
-      visitor(bc * sizeof(typename Super::pointer), 1);
+      visitor(bc * sizeof(pointer), 1);
+    }
+    if (this->size() > 0) {
+      visitor(sizeof(StdNodeReplica<K, value_type, H>), this->size());
     }
-    visitor(
-        sizeof(StdNodeReplica<K, typename Super::value_type, H>), this->size());
   }
 
   template <typename V>
   void visitContiguousRanges(V&& visitor) const {
-    for (typename Super::value_type const& entry : *this) {
-      typename Super::value_type const* b = std::addressof(entry);
+    for (value_type const& entry : *this) {
+      value_type const* b = std::addressof(entry);
       visitor(b, b + 1);
     }
   }
@@ -89,8 +95,16 @@ class F14NodeSet : public f14::detail::F14BasicSet<K, H, E, A> {
   using Super = f14::detail::F14BasicSet<K, H, E, A>;
 
  public:
+  using typename Super::value_type;
+
+  F14NodeSet() = default;
+
   using Super::Super;
-  F14NodeSet() : Super() {}
+
+  F14NodeSet& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 template <typename K, typename H, typename E, typename A>
@@ -98,8 +112,16 @@ class F14ValueSet : public f14::detail::F14BasicSet<K, H, E, A> {
   using Super = f14::detail::F14BasicSet<K, H, E, A>;
 
  public:
-  using Super::Super;
+  using typename Super::value_type;
+
   F14ValueSet() : Super() {}
+
+  using Super::Super;
+
+  F14ValueSet& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 template <typename K, typename H, typename E, typename A>
@@ -107,8 +129,16 @@ class F14VectorSet : public f14::detail::F14BasicSet<K, H, E, A> {
   using Super = f14::detail::F14BasicSet<K, H, E, A>;
 
  public:
+  using typename Super::value_type;
+
+  F14VectorSet() = default;
+
   using Super::Super;
-  F14VectorSet() : Super() {}
+
+  F14VectorSet& operator=(std::initializer_list<value_type> ilist) {
+    Super::operator=(ilist);
+    return *this;
+  }
 };
 
 } // namespace folly

folly/container/detail/F14IntrinsicsAvailability.h

@@ -18,16 +18,21 @@
 
 #include <folly/Portability.h>
 
-// F14 has been implemented for SSE2 and NEON (so far). The NEON version is only
-// enabled on aarch64 as it is difficult to ensure that dependent targets on
-// other Android ARM platforms set the NEON compilation flags consistently. If
-// dependent targets don't consistently build with NEON, due to C++ templates
-// and ODR, the NEON version may be linked in where a non-NEON version is
-// expected.
+// F14 has been implemented for SSE2 and NEON (so far).
 //
-// TODO: enable for android (T33376370)
-// TODO: enable for iOS (T33470422)
-#if ((FOLLY_SSE >= 2) || (FOLLY_NEON && FOLLY_AARCH64)) && (!FOLLY_MOBILE)
+// This platform detection is a bit of a mess because it combines the
+// detection of supported platforms (FOLLY_SSE >= 2 || FOLLY_NEON) with
+// the selection of platforms on which we want to use it.
+//
+// Currently no 32-bit ARM versions are desired because we don't want to
+// need a separate build for chips that don't have NEON.  AARCH64 support
+// is enabled for non-mobile platforms, but on mobile platforms there
+// are downstream iteration order effects that have not yet been resolved.
+//
+// If FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE differs across compilation
+// units the program will fail to link due to a missing definition of
+// folly::container::detail::F14LinkCheck<X>::check() for some X.
+#if (FOLLY_SSE >= 2 || (FOLLY_NEON && FOLLY_AARCH64)) && !FOLLY_MOBILE
 #define FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE 1
 #else
 #define FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE 0

folly/container/test/F14InterprocessTest.cpp

@@ -23,6 +23,7 @@
 
 #include <boost/interprocess/allocators/adaptive_pool.hpp>
 #include <boost/interprocess/managed_shared_memory.hpp>
+
 #include <folly/Format.h>
 #include <folly/Random.h>
 #include <folly/Traits.h>
@@ -174,29 +175,34 @@ TEST(ShmF14VectorSet, simple) {
 
 template <typename M>
 void runSimultaneousAccessMapTest() {
-  auto name = makeRandomName();
-  auto segment1 = makeShmSegment(8192, name);
-  auto segment2 =
-      std::make_shared<managed_shared_memory>(open_only, name.c_str());
-
-  auto m1 = segment1->construct<M>("m")(
-      typename M::allocator_type{segment1->get_segment_manager()});
-  auto m2 = segment2->find<M>("m").first;
-
-  std::cout << "m in segment1 @ " << (uintptr_t)m1 << "\n";
-  std::cout << "m in segment2 @ " << (uintptr_t)m2 << "\n";
-
-  EXPECT_NE(&*m1, &*m2);
-
-  (*m1)[1] = 10;
-  EXPECT_EQ(m2->count(0), 0);
-  EXPECT_EQ((*m2)[1], 10);
-  (*m2)[2] = 20;
-  EXPECT_EQ(m1->size(), 2);
-  EXPECT_EQ(m1->find(2)->second, 20);
-  (*m1)[3] = 30;
-  EXPECT_EQ(m2->size(), 3);
-  EXPECT_FALSE(m2->try_emplace(3, 33).second);
+  using namespace folly::f14::detail;
+
+  // fallback std::unordered_map on libstdc++ doesn't pass this test
+  if (getF14IntrinsicsMode() != F14IntrinsicsMode::None) {
+    auto name = makeRandomName();
+    auto segment1 = makeShmSegment(8192, name);
+    auto segment2 =
+        std::make_shared<managed_shared_memory>(open_only, name.c_str());
+
+    auto m1 = segment1->construct<M>("m")(
+        typename M::allocator_type{segment1->get_segment_manager()});
+    auto m2 = segment2->find<M>("m").first;
+
+    std::cout << "m in segment1 @ " << (uintptr_t)m1 << "\n";
+    std::cout << "m in segment2 @ " << (uintptr_t)m2 << "\n";
+
+    EXPECT_NE(&*m1, &*m2);
+
+    (*m1)[1] = 10;
+    EXPECT_EQ(m2->count(0), 0);
+    EXPECT_EQ((*m2)[1], 10);
+    (*m2)[2] = 20;
+    EXPECT_EQ(m1->size(), 2);
+    EXPECT_EQ(m1->find(2)->second, 20);
+    (*m1)[3] = 30;
+    EXPECT_EQ(m2->size(), 3);
+    EXPECT_FALSE(m2->emplace(std::make_pair(3, 33)).second);
+  }
 }
 
 TEST(ShmF14ValueMap, simultaneous) {

folly/container/test/F14MapTest.cpp

@@ -57,19 +57,32 @@ template <
     typename V>
 void runAllocatedMemorySizeTest() {
   using namespace folly::f14;
+  using namespace folly::f14::detail;
   using A = SwapTrackingAlloc<std::pair<const K, V>>;
 
   resetTracking();
   {
     TMap<K, V, DefaultHasher<K>, DefaultKeyEqual<K>, A> m;
-    EXPECT_EQ(testAllocatedMemorySize, m.getAllocatedMemorySize());
+
+    // if F14 intrinsics are not available then we fall back to using
+    // std::unordered_map underneath, but in that case the allocation
+    // info is only best effort
+    bool preciseAllocInfo = getF14IntrinsicsMode() != F14IntrinsicsMode::None;
+
+    if (preciseAllocInfo) {
+      EXPECT_EQ(testAllocatedMemorySize, 0);
+      EXPECT_EQ(m.getAllocatedMemorySize(), 0);
+    }
     auto emptyMapAllocatedMemorySize = testAllocatedMemorySize;
     auto emptyMapAllocatedBlockCount = testAllocatedBlockCount;
 
     for (size_t i = 0; i < 1000; ++i) {
       m.insert(std::make_pair(folly::to<K>(i), V{}));
       m.erase(folly::to<K>(i / 10 + 2));
-      EXPECT_EQ(testAllocatedMemorySize, m.getAllocatedMemorySize());
+      if (preciseAllocInfo) {
+        EXPECT_EQ(testAllocatedMemorySize, m.getAllocatedMemorySize());
+      }
+      EXPECT_GE(m.getAllocatedMemorySize(), sizeof(std::pair<K, V>) * m.size());
       std::size_t size = 0;
       std::size_t count = 0;
       m.visitAllocationClasses([&](std::size_t, std::size_t) mutable {});
@@ -77,13 +90,20 @@ void runAllocatedMemorySizeTest() {
         size += bytes * n;
         count += n;
       });
-      EXPECT_EQ(testAllocatedMemorySize, size);
-      EXPECT_EQ(testAllocatedBlockCount, count);
+      if (preciseAllocInfo) {
+        EXPECT_EQ(testAllocatedMemorySize, size);
+        EXPECT_EQ(testAllocatedBlockCount, count);
+      }
     }
 
     m = decltype(m){};
     EXPECT_EQ(testAllocatedMemorySize, emptyMapAllocatedMemorySize);
     EXPECT_EQ(testAllocatedBlockCount, emptyMapAllocatedBlockCount);
+
+    m.reserve(5);
+    EXPECT_GT(testAllocatedMemorySize, 0);
+    m = {};
+    EXPECT_GT(testAllocatedMemorySize, 0);
   }
   EXPECT_EQ(testAllocatedMemorySize, 0);
   EXPECT_EQ(testAllocatedBlockCount, 0);

folly/container/test/F14SetTest.cpp

@@ -53,37 +53,53 @@ template <
     typename K>
 void runAllocatedMemorySizeTest() {
   using namespace folly::f14;
+  using namespace folly::f14::detail;
   using A = SwapTrackingAlloc<K>;
 
   resetTracking();
   {
-    TSet<K, DefaultHasher<K>, DefaultKeyEqual<K>, A> m;
-    EXPECT_EQ(testAllocatedMemorySize, m.getAllocatedMemorySize());
+    TSet<K, DefaultHasher<K>, DefaultKeyEqual<K>, A> s;
+
+    // if F14 intrinsics are not available then we fall back to using
+    // std::unordered_set underneath, but in that case the allocation
+    // info is only best effort
+    bool preciseAllocInfo = getF14IntrinsicsMode() != F14IntrinsicsMode::None;
+
+    if (preciseAllocInfo) {
+      EXPECT_EQ(testAllocatedMemorySize, 0);
+      EXPECT_EQ(s.getAllocatedMemorySize(), 0);
+    }
     auto emptySetAllocatedMemorySize = testAllocatedMemorySize;
     auto emptySetAllocatedBlockCount = testAllocatedBlockCount;
 
-    // TODO(T33426422): check unordered_set impl in Android NDK for allocation
-    // behaviors
-#if (!FOLLY_MOBILE || defined(__APPLE__))
     for (size_t i = 0; i < 1000; ++i) {
-      m.insert(folly::to<K>(i));
-      m.erase(folly::to<K>(i / 10 + 2));
-      EXPECT_EQ(testAllocatedMemorySize, m.getAllocatedMemorySize());
+      s.insert(folly::to<K>(i));
+      s.erase(folly::to<K>(i / 10 + 2));
+      if (preciseAllocInfo) {
+        EXPECT_EQ(testAllocatedMemorySize, s.getAllocatedMemorySize());
+      }
+      EXPECT_GE(s.getAllocatedMemorySize(), sizeof(K) * s.size());
       std::size_t size = 0;
       std::size_t count = 0;
-      m.visitAllocationClasses([&](std::size_t, std::size_t) mutable {});
-      m.visitAllocationClasses([&](std::size_t bytes, std::size_t n) {
+      s.visitAllocationClasses([&](std::size_t, std::size_t) mutable {});
+      s.visitAllocationClasses([&](std::size_t bytes, std::size_t n) {
         size += bytes * n;
         count += n;
       });
-      EXPECT_EQ(testAllocatedMemorySize, size);
-      EXPECT_EQ(testAllocatedBlockCount, count);
+      if (preciseAllocInfo) {
+        EXPECT_EQ(testAllocatedMemorySize, size);
+        EXPECT_EQ(testAllocatedBlockCount, count);
+      }
     }
-#endif
 
-    m = decltype(m){};
+    s = decltype(s){};
     EXPECT_EQ(testAllocatedMemorySize, emptySetAllocatedMemorySize);
     EXPECT_EQ(testAllocatedBlockCount, emptySetAllocatedBlockCount);
+
+    s.reserve(5);
+    EXPECT_GT(testAllocatedMemorySize, emptySetAllocatedMemorySize);
+    s = {};
+    EXPECT_GT(testAllocatedMemorySize, emptySetAllocatedMemorySize);
   }
   EXPECT_EQ(testAllocatedMemorySize, 0);
   EXPECT_EQ(testAllocatedBlockCount, 0);