Commit f9fb46d1 authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook Github Bot

Remove the spin-lock protecting Future's executor

Summary:
[Folly] Remove the spin-lock protecting `Future`'s executor.

The executor can be set in a few places:
* When constructing a new `Promise` and `Future` pair before handing off the `Promise` in `Future::thenImplementation`.
* After invoking `SemiFuture::wait` and `Future::wait` in the non-timeout case.
* In `SemiFuture` dtor, if it has a core.

In the first case, the executor is set in the future thread before being read by the promise thread. In the second case, the executor is set after being read by the promise thread. In the third case, if there is a core, then no callback could have been attached. But there is no race between setting and reading.

Reviewed By: djwatson

Differential Revision: D7115700

fbshipit-source-id: 5a0f90a9f01a7dcf295f39d323959e91aeae26da
parent fa45ee46
......@@ -295,7 +295,7 @@ FutureBase<T>::thenImplementation(
// grab the Future now before we lose our handle on the Promise
auto sf = p.getSemiFuture();
sf.core_->setExecutorNoLock(this->getExecutor());
sf.core_->setExecutor(this->getExecutor());
auto f = Future<B>(sf.core_);
sf.core_ = nullptr;
......@@ -359,7 +359,7 @@ FutureBase<T>::thenImplementation(
// grab the Future now before we lose our handle on the Promise
auto sf = p.getSemiFuture();
sf.core_->setExecutorNoLock(this->getExecutor());
sf.core_->setExecutor(this->getExecutor());
auto f = Future<B>(sf.core_);
sf.core_ = nullptr;
......
......@@ -226,17 +226,12 @@ class Core final {
/// May call from any thread
bool isActive() { return active_.load(std::memory_order_acquire); }
/// Call only from Future thread
/// Call only from Future thread, either before attaching a callback or after
/// the callback has already been invoked, but not concurrently with anything
/// which might trigger invocation of the callback
void setExecutor(Executor* x, int8_t priority = Executor::MID_PRI) {
if (!executorLock_.try_lock()) {
executorLock_.lock();
}
executor_ = x;
priority_ = priority;
executorLock_.unlock();
}
void setExecutorNoLock(Executor* x, int8_t priority = Executor::MID_PRI) {
auto s = fsm_.getState();
DCHECK(s == State::Start || s == State::OnlyResult || s == State::Done);
executor_ = x;
priority_ = priority;
}
......@@ -337,16 +332,7 @@ class Core final {
void doCallback() {
Executor* x = executor_;
// initialize, solely to appease clang's -Wconditional-uninitialized
int8_t priority = 0;
if (x) {
if (!executorLock_.try_lock()) {
executorLock_.lock();
}
x = executor_;
priority = priority_;
executorLock_.unlock();
}
int8_t priority = priority_;
if (x) {
exception_wrapper ew;
......@@ -426,7 +412,6 @@ class Core final {
std::atomic<bool> active_ {true};
std::atomic<bool> interruptHandlerSet_ {false};
folly::MicroSpinLock interruptLock_ {0};
folly::MicroSpinLock executorLock_ {0};
int8_t priority_ {-1};
Executor* executor_ {nullptr};
std::shared_ptr<RequestContext> context_ {nullptr};
......
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