Commit 5623085a authored by Chao Yang's avatar Chao Yang Committed by Praveen Kumar Ramakrishnan

Tag dispatch for enqueue/dequeue implementation

Summary:
clang (>=3.6?) reports potential object slicing bug when MPMCQueue is used for
polymorphic class as the queue item, e.g. as in P19814469. This can be false
positive however, since the choice is based on the type trait already.  This
diff uses tag dispatch to selectively compile the overload that will be
executed, therefore if there is no-throw move ctor supplied clang will not
examine the simulated relocation code.

This doesn't avoid object slicing bug however if the client insists to use
MPMCQueue to hold base class while enqueue and dequeue with subclassed item.

Test Plan: compile with --clang

Reviewed By: tudorb@fb.com

Subscribers: folly-diffs@, yfeldblum, chalfant

FB internal diff: D2029949

Signature: t1:2029949:1430264357:af479117adf90bc1915c071e7376a30aacb72f46
parent c7138e7c
...@@ -782,7 +782,7 @@ struct SingleElementQueue { ...@@ -782,7 +782,7 @@ struct SingleElementQueue {
/// enqueue using in-place noexcept construction /// enqueue using in-place noexcept construction
template <typename ...Args, template <typename ...Args,
typename = typename std::enable_if< typename = typename std::enable_if<
std::is_nothrow_constructible<T,Args...>::value>::type> std::is_nothrow_constructible<T,Args...>::value>::type>
void enqueue(const uint32_t turn, void enqueue(const uint32_t turn,
Atom<uint32_t>& spinCutoff, Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff, const bool updateSpinCutoff,
...@@ -803,19 +803,13 @@ struct SingleElementQueue { ...@@ -803,19 +803,13 @@ struct SingleElementQueue {
Atom<uint32_t>& spinCutoff, Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff, const bool updateSpinCutoff,
T&& goner) noexcept { T&& goner) noexcept {
if (std::is_nothrow_constructible<T,T&&>::value) { enqueueImpl(
// this is preferred turn,
sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff); spinCutoff,
new (&contents_) T(std::move(goner)); updateSpinCutoff,
sequencer_.completeTurn(turn * 2); std::move(goner),
} else { typename std::conditional<std::is_nothrow_constructible<T,T&&>::value,
// simulate nothrow move with relocation, followed by default ImplByMove, ImplByRelocation>::type());
// construction to fill the gap we created
sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
memcpy(&contents_, &goner, sizeof(T));
sequencer_.completeTurn(turn * 2);
new (&goner) T();
}
} }
bool mayEnqueue(const uint32_t turn) const noexcept { bool mayEnqueue(const uint32_t turn) const noexcept {
...@@ -826,24 +820,13 @@ struct SingleElementQueue { ...@@ -826,24 +820,13 @@ struct SingleElementQueue {
Atom<uint32_t>& spinCutoff, Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff, const bool updateSpinCutoff,
T& elem) noexcept { T& elem) noexcept {
if (folly::IsRelocatable<T>::value) { dequeueImpl(turn,
// this version is preferred, because we do as much work as possible spinCutoff,
// before waiting updateSpinCutoff,
try { elem,
elem.~T(); typename std::conditional<folly::IsRelocatable<T>::value,
} catch (...) { ImplByRelocation,
// unlikely, but if we don't complete our turn the queue will die ImplByMove>::type());
}
sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
memcpy(&elem, &contents_, sizeof(T));
sequencer_.completeTurn(turn * 2 + 1);
} else {
// use nothrow move assignment
sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
elem = std::move(*ptr());
destroyContents();
sequencer_.completeTurn(turn * 2 + 1);
}
} }
bool mayDequeue(const uint32_t turn) const noexcept { bool mayDequeue(const uint32_t turn) const noexcept {
...@@ -871,6 +854,63 @@ struct SingleElementQueue { ...@@ -871,6 +854,63 @@ struct SingleElementQueue {
memset(&contents_, 'Q', sizeof(T)); memset(&contents_, 'Q', sizeof(T));
#endif #endif
} }
/// Tag classes for dispatching to enqueue/dequeue implementation.
struct ImplByRelocation {};
struct ImplByMove {};
/// enqueue using nothrow move construction.
void enqueueImpl(const uint32_t turn,
Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff,
T&& goner,
ImplByMove) noexcept {
sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
new (&contents_) T(std::move(goner));
sequencer_.completeTurn(turn * 2);
}
/// enqueue by simulating nothrow move with relocation, followed by
/// default construction to a noexcept relocation.
void enqueueImpl(const uint32_t turn,
Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff,
T&& goner,
ImplByRelocation) noexcept {
sequencer_.waitForTurn(turn * 2, spinCutoff, updateSpinCutoff);
memcpy(&contents_, &goner, sizeof(T));
sequencer_.completeTurn(turn * 2);
new (&goner) T();
}
/// dequeue by destructing followed by relocation. This version is preferred,
/// because as much work as possible can be done before waiting.
void dequeueImpl(uint32_t turn,
Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff,
T& elem,
ImplByRelocation) noexcept {
try {
elem.~T();
} catch (...) {
// unlikely, but if we don't complete our turn the queue will die
}
sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
memcpy(&elem, &contents_, sizeof(T));
sequencer_.completeTurn(turn * 2 + 1);
}
/// dequeue by nothrow move assignment.
void dequeueImpl(uint32_t turn,
Atom<uint32_t>& spinCutoff,
const bool updateSpinCutoff,
T& elem,
ImplByMove) noexcept {
sequencer_.waitForTurn(turn * 2 + 1, spinCutoff, updateSpinCutoff);
elem = std::move(*ptr());
destroyContents();
sequencer_.completeTurn(turn * 2 + 1);
}
}; };
} // namespace detail } // namespace detail
......
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