From 46a851f49b64fe44c7643eec03e8b16096d9837d Mon Sep 17 00:00:00 2001 From: Phil Willoughby Date: Fri, 14 Jul 2017 02:56:14 -0700 Subject: [PATCH] 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 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 | 48 +++++------ folly/Makefile.am | 1 - folly/detail/CachelinePaddedImpl.h | 57 ------------- folly/test/CachelinePaddedTest.cpp | 123 +++++++++++++++++------------ 4 files changed, 93 insertions(+), 136 deletions(-) delete mode 100644 folly/detail/CachelinePaddedImpl.h diff --git a/folly/CachelinePadded.h b/folly/CachelinePadded.h index caa1fb52..6e30b623 100644 --- a/folly/CachelinePadded.h +++ b/folly/CachelinePadded.h @@ -16,48 +16,36 @@ #pragma once -#include +#include + +#include 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* 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 class CachelinePadded { + static_assert( + alignof(T) <= alignof(std::max_align_t), + "CachelinePadded does not support over-aligned types"); + public: template explicit CachelinePadded(Args&&... args) - : impl_(std::forward(args)...) {} - - CachelinePadded() {} + : inner_(std::forward(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 impl_; + static constexpr size_t paddingSize() noexcept { + return folly::CacheLocality::kFalseSharingRange - + (alignof(T) % folly::CacheLocality::kFalseSharingRange); + } + char paddingPre_[paddingSize()]; + T inner_; + char paddingPost_[paddingSize()]; }; } diff --git a/folly/Makefile.am b/folly/Makefile.am index 36b703fe..f926cdac 100644 --- a/folly/Makefile.am +++ b/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 \ diff --git a/folly/detail/CachelinePaddedImpl.h b/folly/detail/CachelinePaddedImpl.h deleted file mode 100644 index 1acce99d..00000000 --- a/folly/detail/CachelinePaddedImpl.h +++ /dev/null @@ -1,57 +0,0 @@ -/* - * 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 - -namespace folly { - -template -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 -struct alignas(T) alignas(CacheLocality::kFalseSharingRange) - CachelinePaddedImpl { - template - explicit CachelinePaddedImpl(Args&&... args) - : item(std::forward(args)...) {} - T item; -}; - -template -struct alignas(T) alignas(CacheLocality::kFalseSharingRange) - CachelinePaddedImpl { - template - explicit CachelinePaddedImpl(Args&&... args) - : item(std::forward(args)...) {} - - T item; - char padding - [CacheLocality::kFalseSharingRange - - sizeof(T) % CacheLocality::kFalseSharingRange]; -}; -} // namespace detail -} // namespace folly diff --git a/folly/test/CachelinePaddedTest.cpp b/folly/test/CachelinePaddedTest.cpp index 2fe69e88..11cefe33 100644 --- a/folly/test/CachelinePaddedTest.cpp +++ b/folly/test/CachelinePaddedTest.cpp @@ -26,78 +26,99 @@ static_assert( std::is_standard_layout>::value, "CachelinePadded must be standard-layout if T is."); -const int kCachelineSize = folly::CacheLocality::kFalseSharingRange; +static constexpr int kCachelineSize = folly::CacheLocality::kFalseSharingRange; -template -struct SizedData { +template +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(i), data[i]); - ++data[i]; + size_t i = 0; + for (auto& datum : data) { + EXPECT_EQ(static_cast(i++), datum); + ++datum; } } ~SizedData() { - for (unsigned i = 0; i < dataSize; ++i) { - EXPECT_EQ(static_cast(i + 1), data[i]); + size_t i = 1; + for (auto& datum : data) { + EXPECT_EQ(static_cast(i++), datum); } } unsigned char data[dataSize]; }; -using ExactlyCachelineSized = SizedData; -using DoubleCachelineSized = SizedData<2 * kCachelineSize>; -using BelowCachelineSized = SizedData; -using AboveCachelineSized = SizedData; - -TEST(CachelinePadded, Exact) { - EXPECT_EQ(kCachelineSize, sizeof(CachelinePadded)); - CachelinePadded item; - item.get()->doModifications(); - EXPECT_TRUE( - reinterpret_cast*>(item.get()) == - &item); -} - -TEST(CachelinePadded, Double) { - EXPECT_EQ(2 * kCachelineSize, sizeof(CachelinePadded)); - CachelinePadded item; - item.get()->doModifications(); - EXPECT_TRUE( - reinterpret_cast*>(item.get()) == - &item); +template +using SizedDataMimic = SizedData; + +template +struct CachelinePaddedTests : ::testing::Test {}; + +using CachelinePaddedTypes = ::testing::Types< + SizedData, + SizedData<2 * kCachelineSize>, + SizedData, + SizedData, + // Mimic single basic types: + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + // Mimic small arrays of basic types: + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + // Mimic large arrays of basic types: + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic, + SizedDataMimic>; +TYPED_TEST_CASE(CachelinePaddedTests, CachelinePaddedTypes); + +TYPED_TEST(CachelinePaddedTests, alignment) { + EXPECT_EQ(alignof(TypeParam), alignof(CachelinePadded)); } -TEST(CachelinePadded, Below) { - EXPECT_EQ(kCachelineSize, sizeof(CachelinePadded)); - CachelinePadded item; +TYPED_TEST(CachelinePaddedTests, integrity) { + CachelinePadded item; item.get()->doModifications(); - EXPECT_TRUE( - reinterpret_cast*>(item.get()) == - &item); } -TEST(CachelinePadded, Above) { - EXPECT_EQ(2 * kCachelineSize, sizeof(CachelinePadded)); - CachelinePadded item; - item.get()->doModifications(); - EXPECT_TRUE( - reinterpret_cast*>(item.get()) == - &item); -} - -TEST(CachelinePadded, CanBeCastedBack) { - CachelinePadded padded; - CachelinePadded* ptr = - reinterpret_cast*>(padded.get()); - EXPECT_EQ(&padded, ptr); +TYPED_TEST(CachelinePaddedTests, size) { + EXPECT_GT( + sizeof(TypeParam) + 2 * kCachelineSize, + sizeof(CachelinePadded)); + 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)); } TEST(CachelinePadded, PtrOperator) { -- 2.34.1