Commit 64fd47a2 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook GitHub Bot

Use relaxed_atomic in IOThreadPoolExecutor

Summary:
[Folly] Use `relaxed_atomic` in `IOThreadPoolExecutor`.

For the `nextThread_` field, which may be mutated within `pickThread` in any number of threads concurrently. While `pickThread` is only ever called with a shared lock held, the shared lock does not actually help with concurrent mutations.

Since the mutations of `nextThread_` by `pickThread` is always a `std::atomic::fetch_add` with `std::memory_order_relaxed`, and since that case is a bit more nicely solved with `folly::relaxed_atomic`, switch to that.

Reviewed By: ot

Differential Revision: D6290451

fbshipit-source-id: e473fa6321e462c9b3df9d2bd9650967cca3444a
parent 7d707de4
...@@ -159,7 +159,7 @@ IOThreadPoolExecutor::pickThread() { ...@@ -159,7 +159,7 @@ IOThreadPoolExecutor::pickThread() {
// the second case, `!me` so we'll crash anyway. // the second case, `!me` so we'll crash anyway.
return me; return me;
} }
auto thread = ths[nextThread_.fetch_add(1, std::memory_order_relaxed) % n]; auto thread = ths[nextThread_++ % n];
return std::static_pointer_cast<IOThread>(thread); return std::static_pointer_cast<IOThread>(thread);
} }
......
...@@ -16,12 +16,11 @@ ...@@ -16,12 +16,11 @@
#pragma once #pragma once
#include <atomic>
#include <folly/Portability.h> #include <folly/Portability.h>
#include <folly/executors/IOExecutor.h> #include <folly/executors/IOExecutor.h>
#include <folly/executors/ThreadPoolExecutor.h> #include <folly/executors/ThreadPoolExecutor.h>
#include <folly/io/async/EventBaseManager.h> #include <folly/io/async/EventBaseManager.h>
#include <folly/synchronization/RelaxedAtomic.h>
namespace folly { namespace folly {
...@@ -103,7 +102,7 @@ class IOThreadPoolExecutor : public ThreadPoolExecutor, public IOExecutor { ...@@ -103,7 +102,7 @@ class IOThreadPoolExecutor : public ThreadPoolExecutor, public IOExecutor {
size_t getPendingTaskCountImpl() const override final; size_t getPendingTaskCountImpl() const override final;
const bool isWaitForAll_; // whether to wait till event base loop exits const bool isWaitForAll_; // whether to wait till event base loop exits
std::atomic<size_t> nextThread_; relaxed_atomic<size_t> nextThread_;
folly::ThreadLocal<std::shared_ptr<IOThread>> thisThread_; folly::ThreadLocal<std::shared_ptr<IOThread>> thisThread_;
folly::EventBaseManager* eventBaseManager_; folly::EventBaseManager* eventBaseManager_;
}; };
......
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