Commit 6678c4e6 authored by Phil Willoughby's avatar Phil Willoughby Committed by Facebook Github Bot

Re-align LifoSem

Summary:
Previously it generated a warning if compiled with -Wover-align because its alignment exceeded alignof(std::max_align_t).

This also meant that heap-allocated LifoSem objects were not guaranteed to be on their own cache-line, potentially degrading performance.

This change adds padding before the important part of the LifoSem (to match the existing after-padding) and reduces its alignment requirement.

Reviewed By: yfeldblum

Differential Revision: D5380700

fbshipit-source-id: 7fdd593a58a2e0c7b03df11152ee4bb66f57e717
parent 4203522c
...@@ -25,9 +25,9 @@ ...@@ -25,9 +25,9 @@
#include <folly/AtomicStruct.h> #include <folly/AtomicStruct.h>
#include <folly/Baton.h> #include <folly/Baton.h>
#include <folly/CachelinePadded.h>
#include <folly/IndexedMemPool.h> #include <folly/IndexedMemPool.h>
#include <folly/Likely.h> #include <folly/Likely.h>
#include <folly/concurrency/CacheLocality.h>
namespace folly { namespace folly {
...@@ -322,7 +322,7 @@ struct LifoSemBase { ...@@ -322,7 +322,7 @@ struct LifoSemBase {
/// Constructor /// Constructor
constexpr explicit LifoSemBase(uint32_t initialValue = 0) constexpr explicit LifoSemBase(uint32_t initialValue = 0)
: head_(LifoSemHead::fresh(initialValue)), padding_() {} : head_(LifoSemHead::fresh(initialValue)) {}
LifoSemBase(LifoSemBase const&) = delete; LifoSemBase(LifoSemBase const&) = delete;
LifoSemBase& operator=(LifoSemBase const&) = delete; LifoSemBase& operator=(LifoSemBase const&) = delete;
...@@ -355,7 +355,7 @@ struct LifoSemBase { ...@@ -355,7 +355,7 @@ struct LifoSemBase {
/// Returns true iff shutdown() has been called /// Returns true iff shutdown() has been called
bool isShutdown() const { bool isShutdown() const {
return UNLIKELY(head_.load(std::memory_order_acquire).isShutdown()); return UNLIKELY(head_->load(std::memory_order_acquire).isShutdown());
} }
/// Prevents blocking on this semaphore, causing all blocking wait() /// Prevents blocking on this semaphore, causing all blocking wait()
...@@ -365,9 +365,9 @@ struct LifoSemBase { ...@@ -365,9 +365,9 @@ struct LifoSemBase {
/// has already occurred will proceed normally. /// has already occurred will proceed normally.
void shutdown() { void shutdown() {
// first set the shutdown bit // first set the shutdown bit
auto h = head_.load(std::memory_order_acquire); auto h = head_->load(std::memory_order_acquire);
while (!h.isShutdown()) { while (!h.isShutdown()) {
if (head_.compare_exchange_strong(h, h.withShutdown())) { if (head_->compare_exchange_strong(h, h.withShutdown())) {
// success // success
h = h.withShutdown(); h = h.withShutdown();
break; break;
...@@ -379,7 +379,7 @@ struct LifoSemBase { ...@@ -379,7 +379,7 @@ struct LifoSemBase {
while (h.isNodeIdx()) { while (h.isNodeIdx()) {
auto& node = idxToNode(h.idx()); auto& node = idxToNode(h.idx());
auto repl = h.withPop(node.next); auto repl = h.withPop(node.next);
if (head_.compare_exchange_strong(h, repl)) { if (head_->compare_exchange_strong(h, repl)) {
// successful pop, wake up the waiter and move on. The next // successful pop, wake up the waiter and move on. The next
// field is used to convey that this wakeup didn't consume a value // field is used to convey that this wakeup didn't consume a value
node.setShutdownNotice(); node.setShutdownNotice();
...@@ -463,7 +463,7 @@ struct LifoSemBase { ...@@ -463,7 +463,7 @@ struct LifoSemBase {
// this is actually linearizable, but we don't promise that because // this is actually linearizable, but we don't promise that because
// we may want to add striping in the future to help under heavy // we may want to add striping in the future to help under heavy
// contention // contention
auto h = head_.load(std::memory_order_acquire); auto h = head_->load(std::memory_order_acquire);
return h.isNodeIdx() ? 0 : h.value(); return h.isNodeIdx() ? 0 : h.value();
} }
...@@ -511,11 +511,7 @@ struct LifoSemBase { ...@@ -511,11 +511,7 @@ struct LifoSemBase {
} }
private: private:
CachelinePadded<folly::AtomicStruct<LifoSemHead, Atom>> head_;
FOLLY_ALIGN_TO_AVOID_FALSE_SHARING
folly::AtomicStruct<LifoSemHead,Atom> head_;
char padding_[folly::CacheLocality::kFalseSharingRange - sizeof(LifoSemHead)];
static LifoSemNode<Handoff, Atom>& idxToNode(uint32_t idx) { static LifoSemNode<Handoff, Atom>& idxToNode(uint32_t idx) {
auto raw = &LifoSemRawNode<Atom>::pool()[idx]; auto raw = &LifoSemRawNode<Atom>::pool()[idx];
...@@ -533,16 +529,16 @@ struct LifoSemBase { ...@@ -533,16 +529,16 @@ struct LifoSemBase {
while (true) { while (true) {
assert(n > 0); assert(n > 0);
auto head = head_.load(std::memory_order_acquire); auto head = head_->load(std::memory_order_acquire);
if (head.isNodeIdx()) { if (head.isNodeIdx()) {
auto& node = idxToNode(head.idx()); auto& node = idxToNode(head.idx());
if (head_.compare_exchange_strong(head, head.withPop(node.next))) { if (head_->compare_exchange_strong(head, head.withPop(node.next))) {
// successful pop // successful pop
return head.idx(); return head.idx();
} }
} else { } else {
auto after = head.withValueIncr(n); auto after = head.withValueIncr(n);
if (head_.compare_exchange_strong(head, after)) { if (head_->compare_exchange_strong(head, after)) {
// successful incr // successful incr
return 0; return 0;
} }
...@@ -562,12 +558,12 @@ struct LifoSemBase { ...@@ -562,12 +558,12 @@ struct LifoSemBase {
assert(n > 0); assert(n > 0);
while (true) { while (true) {
auto head = head_.load(std::memory_order_acquire); auto head = head_->load(std::memory_order_acquire);
if (!head.isNodeIdx() && head.value() > 0) { if (!head.isNodeIdx() && head.value() > 0) {
// decr // decr
auto delta = std::min(n, head.value()); auto delta = std::min(n, head.value());
if (head_.compare_exchange_strong(head, head.withValueDecr(delta))) { if (head_->compare_exchange_strong(head, head.withValueDecr(delta))) {
n -= delta; n -= delta;
return WaitResult::DECR; return WaitResult::DECR;
} }
...@@ -583,7 +579,7 @@ struct LifoSemBase { ...@@ -583,7 +579,7 @@ struct LifoSemBase {
auto& node = idxToNode(idx); auto& node = idxToNode(idx);
node.next = head.isNodeIdx() ? head.idx() : 0; node.next = head.isNodeIdx() ? head.idx() : 0;
if (head_.compare_exchange_strong(head, head.withPush(idx))) { if (head_->compare_exchange_strong(head, head.withPush(idx))) {
// push succeeded // push succeeded
return WaitResult::PUSH; return WaitResult::PUSH;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment