Commit 05cdf111 authored by Alejandro's avatar Alejandro Committed by Facebook Github Bot 4

Promote memory_order_consume to memory_order_acquire

Summary:
The current use case of `std::memory_order_consume` in this project was intended to provide the appropriate synchronization in cases where a consumer spins on `while( spsc_queue.empty() ) {} `, and then attempts to use an element of the queue since the loop was broken out of, according to comments [here](https://reviews.facebook.net/D48141). Consume semantics do not provide this guarantee according to the standard since there is no data dependency from the producer that can be carried to the consumer by doing a load-consume from the corresponding functions. What is needed is a load-acquire. Current compilers promote `memory_order_consume` to `memory_order_acquire`. Thus, this example appears to work simply due to the promotion from consume to acquire, but would fail to generate the right synchronization instructions on weaker architectures once `memory_order_consume` is implemented as intended.  Therefore, the `memory_order` should be tightened to `memory_order_acquire` to guarantee visibility to the consumer
Closes https://github.com/facebook/folly/pull/381

Reviewed By: djwatson

Differential Revision: D3173574

Pulled By: nbronson

fbshipit-source-id: b109be2e74f016750a3fe1f848e3fc66e609b9d2
parent a56a988a
...@@ -134,16 +134,16 @@ struct ProducerConsumerQueue { ...@@ -134,16 +134,16 @@ struct ProducerConsumerQueue {
} }
bool isEmpty() const { bool isEmpty() const {
return readIndex_.load(std::memory_order_consume) == return readIndex_.load(std::memory_order_acquire) ==
writeIndex_.load(std::memory_order_consume); writeIndex_.load(std::memory_order_acquire);
} }
bool isFull() const { bool isFull() const {
auto nextRecord = writeIndex_.load(std::memory_order_consume) + 1; auto nextRecord = writeIndex_.load(std::memory_order_acquire) + 1;
if (nextRecord == size_) { if (nextRecord == size_) {
nextRecord = 0; nextRecord = 0;
} }
if (nextRecord != readIndex_.load(std::memory_order_consume)) { if (nextRecord != readIndex_.load(std::memory_order_acquire)) {
return false; return false;
} }
// queue is full // queue is full
...@@ -156,8 +156,8 @@ struct ProducerConsumerQueue { ...@@ -156,8 +156,8 @@ struct ProducerConsumerQueue {
// be removing items concurrently). // be removing items concurrently).
// * It is undefined to call this from any other thread. // * It is undefined to call this from any other thread.
size_t sizeGuess() const { size_t sizeGuess() const {
int ret = writeIndex_.load(std::memory_order_consume) - int ret = writeIndex_.load(std::memory_order_acquire) -
readIndex_.load(std::memory_order_consume); readIndex_.load(std::memory_order_acquire);
if (ret < 0) { if (ret < 0) {
ret += size_; ret += size_;
} }
......
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