Update CachelinePadded

Summary:
The C++ standard doesn't guarantee alignment of things bigger than std::max_align_t which is typically 16 bytes. This means that to ensure something is the only thing on a cache-line we need padding on each side of it to exclude anything else.

CachelinePadded<T> will now be larger than it was before, and reinterpret_cast to/from T is no longer valid.

Reviewed By: yfeldblum

Differential Revision: D5380849

fbshipit-source-id: 20f275c875eb4bede4aef19ac7224913ce9b6d51

folly/CachelinePadded.h

@@ -16,48 +16,36 @@
 
 #pragma once
 
-#include <folly/detail/CachelinePaddedImpl.h>
+#include <cstddef>
+
+#include <folly/concurrency/CacheLocality.h>
 
 namespace folly {
 
 /**
- * Holds a type T, in addition to enough padding to round the size up to the
- * next multiple of the false sharing range used by folly.
- *
- * If T is standard-layout, then casting a T* you get from this class to a
- * CachelinePadded<T>* is safe.
- *
- * This class handles padding, but imperfectly handles alignment. (Note that
- * alignment matters for false-sharing: imagine a cacheline size of 64, and two
- * adjacent 64-byte objects, with the first starting at an offset of 32. The
- * last 32 bytes of the first object share a cacheline with the first 32 bytes
- * of the second.). We alignas this class to be at least cacheline-sized, but
- * it's implementation-defined what that means (since a cacheline is almost
- * certainly larger than the maximum natural alignment). The following should be
- * true for recent compilers on common architectures:
- *
- * For heap objects, alignment needs to be handled at the allocator level, such
- * as with posix_memalign (this isn't necessary with jemalloc, which aligns
- * objects that are a multiple of cacheline size to a cacheline).
+ * Holds a type T, in addition to enough padding to ensure that it isn't subject
+ * to false sharing within the range used by folly.
  *
- * For static and stack objects, the alignment should be obeyed, and no specific
- * intervention is necessary.
+ * If `sizeof(T) <= alignof(T)` then the inner `T` will be entirely within one
+ * false sharing range (AKA cache line).
  */
 template <typename T>
 class CachelinePadded {
+  static_assert(
+      alignof(T) <= alignof(std::max_align_t),
+      "CachelinePadded does not support over-aligned types");
+
  public:
   template <typename... Args>
   explicit CachelinePadded(Args&&... args)
-      : impl_(std::forward<Args>(args)...) {}
-
-  CachelinePadded() {}
+      : inner_(std::forward<Args>(args)...) {}
 
   T* get() {
-    return &impl_.item;
+    return &inner_;
   }
 
   const T* get() const {
-    return &impl_.item;
+    return &inner_;
   }
 
   T* operator->() {
@@ -77,6 +65,12 @@ class CachelinePadded {
   }
 
  private:
-  detail::CachelinePaddedImpl<T> impl_;
+  static constexpr size_t paddingSize() noexcept {
+    return folly::CacheLocality::kFalseSharingRange -
+        (alignof(T) % folly::CacheLocality::kFalseSharingRange);
+  }
+  char paddingPre_[paddingSize()];
+  T inner_;
+  char paddingPost_[paddingSize()];
 };
 }

folly/Makefile.am

@@ -62,7 +62,6 @@ nobase_follyinclude_HEADERS = \
 	detail/AtomicUnorderedMapUtils.h \
 	detail/AtomicUtils.h \
 	detail/BitIteratorDetail.h \
-	detail/CachelinePaddedImpl.h \
 	detail/ChecksumDetail.h \
 	detail/DiscriminatedPtrDetail.h \
 	detail/FileUtilDetail.h \

folly/detail/CachelinePaddedImpl.h

-/*
- * Copyright 2017 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#include <folly/concurrency/CacheLocality.h>
-
-namespace folly {
-
-template <typename T>
-class CachelinePadded;
-
-namespace detail {
-
-template <
-    typename T,
-    bool needsPadding = (sizeof(T) % CacheLocality::kFalseSharingRange != 0)>
-struct CachelinePaddedImpl;
-
-// We need alignas(T) alignas(kFalseSharingRange) for the case where alignof(T)
-// > alignof(kFalseSharingRange).
-template <typename T>
-struct alignas(T) alignas(CacheLocality::kFalseSharingRange)
-    CachelinePaddedImpl<T, /* needsPadding = */ false> {
-  template <typename... Args>
-  explicit CachelinePaddedImpl(Args&&... args)
-      : item(std::forward<Args>(args)...) {}
-  T item;
-};
-
-template <typename T>
-struct alignas(T) alignas(CacheLocality::kFalseSharingRange)
-    CachelinePaddedImpl<T, /* needsPadding = */ true> {
-  template <typename... Args>
-  explicit CachelinePaddedImpl(Args&&... args)
-      : item(std::forward<Args>(args)...) {}
-
-  T item;
-  char padding
-      [CacheLocality::kFalseSharingRange -
-       sizeof(T) % CacheLocality::kFalseSharingRange];
-};
-} // namespace detail
-} // namespace folly

folly/test/CachelinePaddedTest.cpp

@@ -26,78 +26,99 @@ static_assert(
     std::is_standard_layout<CachelinePadded<int>>::value,
     "CachelinePadded<T> must be standard-layout if T is.");
 
-const int kCachelineSize = folly::CacheLocality::kFalseSharingRange;
+static constexpr int kCachelineSize = folly::CacheLocality::kFalseSharingRange;
 
-template <int dataSize>
-struct SizedData {
+template <size_t dataSize, size_t alignment = alignof(void*)>
+struct alignas(alignment) SizedData {
   SizedData() {
-    for (unsigned i = 0; i < dataSize; ++i) {
-      data[i] = i;
+    size_t i = 0;
+    for (auto& datum : data) {
+      datum = i++;
     }
   }
 
   void doModifications() {
-    for (unsigned i = 0; i < dataSize; ++i) {
-      EXPECT_EQ(static_cast<unsigned char>(i), data[i]);
-      ++data[i];
+    size_t i = 0;
+    for (auto& datum : data) {
+      EXPECT_EQ(static_cast<unsigned char>(i++), datum);
+      ++datum;
     }
   }
 
   ~SizedData() {
-    for (unsigned i = 0; i < dataSize; ++i) {
-      EXPECT_EQ(static_cast<unsigned char>(i + 1), data[i]);
+    size_t i = 1;
+    for (auto& datum : data) {
+      EXPECT_EQ(static_cast<unsigned char>(i++), datum);
     }
   }
 
   unsigned char data[dataSize];
 };
 
-using ExactlyCachelineSized = SizedData<kCachelineSize>;
-using DoubleCachelineSized = SizedData<2 * kCachelineSize>;
-using BelowCachelineSized = SizedData<kCachelineSize / 2>;
-using AboveCachelineSized = SizedData<kCachelineSize + kCachelineSize / 2>;
-
-TEST(CachelinePadded, Exact) {
-  EXPECT_EQ(kCachelineSize, sizeof(CachelinePadded<ExactlyCachelineSized>));
-  CachelinePadded<ExactlyCachelineSized> item;
-  item.get()->doModifications();
-  EXPECT_TRUE(
-      reinterpret_cast<CachelinePadded<ExactlyCachelineSized>*>(item.get()) ==
-      &item);
-}
-
-TEST(CachelinePadded, Double) {
-  EXPECT_EQ(2 * kCachelineSize, sizeof(CachelinePadded<DoubleCachelineSized>));
-  CachelinePadded<DoubleCachelineSized> item;
-  item.get()->doModifications();
-  EXPECT_TRUE(
-      reinterpret_cast<CachelinePadded<DoubleCachelineSized>*>(item.get()) ==
-      &item);
+template <typename T, size_t N = 1>
+using SizedDataMimic = SizedData<N * sizeof(T), alignof(T)>;
+
+template <typename T>
+struct CachelinePaddedTests : ::testing::Test {};
+
+using CachelinePaddedTypes = ::testing::Types<
+    SizedData<kCachelineSize>,
+    SizedData<2 * kCachelineSize>,
+    SizedData<kCachelineSize / 2>,
+    SizedData<kCachelineSize + kCachelineSize / 2>,
+    // Mimic single basic types:
+    SizedDataMimic<std::max_align_t>,
+    SizedDataMimic<void*>,
+    SizedDataMimic<long double>,
+    SizedDataMimic<double>,
+    SizedDataMimic<float>,
+    SizedDataMimic<long long>,
+    SizedDataMimic<long>,
+    SizedDataMimic<int>,
+    SizedDataMimic<short>,
+    SizedDataMimic<char>,
+    // Mimic small arrays of basic types:
+    SizedDataMimic<std::max_align_t, 3>,
+    SizedDataMimic<void*, 3>,
+    SizedDataMimic<long double, 3>,
+    SizedDataMimic<double, 3>,
+    SizedDataMimic<float, 3>,
+    SizedDataMimic<long long, 3>,
+    SizedDataMimic<long, 3>,
+    SizedDataMimic<int, 3>,
+    SizedDataMimic<short, 3>,
+    SizedDataMimic<char, 3>,
+    // Mimic large arrays of basic types:
+    SizedDataMimic<std::max_align_t, kCachelineSize + 3>,
+    SizedDataMimic<void*, kCachelineSize + 3>,
+    SizedDataMimic<long double, kCachelineSize + 3>,
+    SizedDataMimic<double, kCachelineSize + 3>,
+    SizedDataMimic<float, kCachelineSize + 3>,
+    SizedDataMimic<long long, kCachelineSize + 3>,
+    SizedDataMimic<long, kCachelineSize + 3>,
+    SizedDataMimic<int, kCachelineSize + 3>,
+    SizedDataMimic<short, kCachelineSize + 3>,
+    SizedDataMimic<char, kCachelineSize + 3>>;
+TYPED_TEST_CASE(CachelinePaddedTests, CachelinePaddedTypes);
+
+TYPED_TEST(CachelinePaddedTests, alignment) {
+  EXPECT_EQ(alignof(TypeParam), alignof(CachelinePadded<TypeParam>));
 }
 
-TEST(CachelinePadded, Below) {
-  EXPECT_EQ(kCachelineSize, sizeof(CachelinePadded<BelowCachelineSized>));
-  CachelinePadded<BelowCachelineSized> item;
+TYPED_TEST(CachelinePaddedTests, integrity) {
+  CachelinePadded<TypeParam> item;
   item.get()->doModifications();
-  EXPECT_TRUE(
-      reinterpret_cast<CachelinePadded<BelowCachelineSized>*>(item.get()) ==
-      &item);
 }
 
-TEST(CachelinePadded, Above) {
-  EXPECT_EQ(2 * kCachelineSize, sizeof(CachelinePadded<AboveCachelineSized>));
-  CachelinePadded<AboveCachelineSized> item;
-  item.get()->doModifications();
-  EXPECT_TRUE(
-      reinterpret_cast<CachelinePadded<AboveCachelineSized>*>(item.get()) ==
-      &item);
-}
-
-TEST(CachelinePadded, CanBeCastedBack) {
-  CachelinePadded<int> padded;
-  CachelinePadded<int>* ptr =
-      reinterpret_cast<CachelinePadded<int>*>(padded.get());
-  EXPECT_EQ(&padded, ptr);
+TYPED_TEST(CachelinePaddedTests, size) {
+  EXPECT_GT(
+      sizeof(TypeParam) + 2 * kCachelineSize,
+      sizeof(CachelinePadded<TypeParam>));
+  size_t const rawSize = sizeof(TypeParam);
+  size_t const rawAlign = alignof(TypeParam);
+  size_t const expectedPadding = kCachelineSize - (rawAlign % kCachelineSize);
+  size_t const expectedPaddedSize = rawSize + 2 * expectedPadding;
+  EXPECT_EQ(expectedPaddedSize, sizeof(CachelinePadded<TypeParam>));
 }
 
 TEST(CachelinePadded, PtrOperator) {