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

Optimize Futures Core state-dependent fields

Summary:
[Folly] Optimize Futures Core state-dependent fields, controlling when they are constructed and destructed.

* Reduces layout size by removing the `Optional` wrapper for the result.
* Reduces code size by removing checks and destructions in assignment operators which are not in constructors for `Optional`, `Try`, `Function`, and `shared_ptr`.

Reviewed By: andriigrynenko

Differential Revision: D8346365

fbshipit-source-id: 83ac164dc36934065f2fcfb112440869d023b674
parent 7e9d7487
...@@ -212,9 +212,7 @@ class Core final { ...@@ -212,9 +212,7 @@ class Core final {
bool hasCallback() const noexcept { bool hasCallback() const noexcept {
constexpr auto allowed = State::OnlyCallback | State::Done; constexpr auto allowed = State::OnlyCallback | State::Done;
auto const state = state_.load(std::memory_order_acquire); auto const state = state_.load(std::memory_order_acquire);
auto const ans = State() != (state & allowed); return State() != (state & allowed);
assert(!ans || !!callback_); // callback_ must exist in this state
return ans;
} }
/// May call from any thread /// May call from any thread
...@@ -225,9 +223,7 @@ class Core final { ...@@ -225,9 +223,7 @@ class Core final {
bool hasResult() const noexcept { bool hasResult() const noexcept {
constexpr auto allowed = State::OnlyResult | State::Done; constexpr auto allowed = State::OnlyResult | State::Done;
auto const state = state_.load(std::memory_order_acquire); auto const state = state_.load(std::memory_order_acquire);
auto const ans = State() != (state & allowed); return State() != (state & allowed);
assert(!ans || !!result_); // result_ must exist if hasResult() is true
return ans;
} }
/// May call from any thread /// May call from any thread
...@@ -258,10 +254,12 @@ class Core final { ...@@ -258,10 +254,12 @@ class Core final {
/// possibly moved-out, depending on what the callback did; some but not /// possibly moved-out, depending on what the callback did; some but not
/// all callbacks modify (possibly move-out) the result.) /// all callbacks modify (possibly move-out) the result.)
Try<T>& getTry() { Try<T>& getTry() {
return getTryImpl(*this); DCHECK(hasResult());
return result_;
} }
Try<T> const& getTry() const { Try<T> const& getTry() const {
return getTryImpl(*this); DCHECK(hasResult());
return result_;
} }
/// Call only from consumer thread. /// Call only from consumer thread.
...@@ -274,8 +272,11 @@ class Core final { ...@@ -274,8 +272,11 @@ class Core final {
/// executor or if the executor is inline). /// executor or if the executor is inline).
template <typename F> template <typename F>
void setCallback(F&& func) { void setCallback(F&& func) {
callback_ = std::forward<F>(func); DCHECK(!hasCallback());
context_ = RequestContext::saveContext();
// construct callback_ first; if that fails, context_ will not leak
::new (&callback_) Callback(std::forward<F>(func));
::new (&context_) Context(RequestContext::saveContext());
auto state = state_.load(std::memory_order_acquire); auto state = state_.load(std::memory_order_acquire);
while (true) { while (true) {
...@@ -311,7 +312,9 @@ class Core final { ...@@ -311,7 +312,9 @@ class Core final {
/// and might also synchronously execute that callback (e.g., if there is no /// and might also synchronously execute that callback (e.g., if there is no
/// executor or if the executor is inline). /// executor or if the executor is inline).
void setResult(Try<T>&& t) { void setResult(Try<T>&& t) {
result_ = std::move(t); DCHECK(!hasResult());
::new (&result_) Result(std::move(t));
auto state = state_.load(std::memory_order_acquire); auto state = state_.load(std::memory_order_acquire);
while (true) { while (true) {
...@@ -349,7 +352,7 @@ class Core final { ...@@ -349,7 +352,7 @@ class Core final {
/// Calls `delete this` if there are no more references to `this` /// Calls `delete this` if there are no more references to `this`
/// (including if `detachFuture()` is called previously or concurrently). /// (including if `detachFuture()` is called previously or concurrently).
void detachPromise() noexcept { void detachPromise() noexcept {
DCHECK(result_); DCHECK(hasResult());
detachOne(); detachOne();
} }
...@@ -429,7 +432,7 @@ class Core final { ...@@ -429,7 +432,7 @@ class Core final {
} }
private: private:
Core() : result_(), state_(State::Start), attached_(2) {} Core() : state_(State::Start), attached_(2) {}
explicit Core(Try<T>&& t) explicit Core(Try<T>&& t)
: result_(std::move(t)), state_(State::OnlyResult), attached_(1) {} : result_(std::move(t)), state_(State::OnlyResult), attached_(1) {}
...@@ -437,13 +440,14 @@ class Core final { ...@@ -437,13 +440,14 @@ class Core final {
template <typename... Args> template <typename... Args>
explicit Core(in_place_t, Args&&... args) noexcept( explicit Core(in_place_t, Args&&... args) noexcept(
std::is_nothrow_constructible<T, Args&&...>::value) std::is_nothrow_constructible<T, Args&&...>::value)
: result_(in_place, in_place, std::forward<Args>(args)...), : result_(in_place, std::forward<Args>(args)...),
state_(State::OnlyResult), state_(State::OnlyResult),
attached_(1) {} attached_(1) {}
~Core() { ~Core() {
DCHECK(attached_ == 0); DCHECK(attached_ == 0);
DCHECK(result_); DCHECK(hasResult());
result_.~Result();
} }
// Helper class that stores a pointer to the `Core` object and calls // Helper class that stores a pointer to the `Core` object and calls
...@@ -508,7 +512,7 @@ class Core final { ...@@ -508,7 +512,7 @@ class Core final {
auto cr = std::move(core_ref); auto cr = std::move(core_ref);
Core* const core = cr.getCore(); Core* const core = cr.getCore();
RequestContextScopeGuard rctx(core->context_); RequestContextScopeGuard rctx(core->context_);
core->callback_(std::move(*core->result_)); core->callback_(std::move(core->result_));
}); });
} else { } else {
xPtr->addWithPriority( xPtr->addWithPriority(
...@@ -517,7 +521,7 @@ class Core final { ...@@ -517,7 +521,7 @@ class Core final {
auto cr = std::move(core_ref); auto cr = std::move(core_ref);
Core* const core = cr.getCore(); Core* const core = cr.getCore();
RequestContextScopeGuard rctx(core->context_); RequestContextScopeGuard rctx(core->context_);
core->callback_(std::move(*core->result_)); core->callback_(std::move(core->result_));
}, },
priority); priority);
} }
...@@ -529,17 +533,17 @@ class Core final { ...@@ -529,17 +533,17 @@ class Core final {
if (ew) { if (ew) {
RequestContextScopeGuard rctx(context_); RequestContextScopeGuard rctx(context_);
result_ = Try<T>(std::move(ew)); result_ = Try<T>(std::move(ew));
callback_(std::move(*result_)); callback_(std::move(result_));
} }
} else { } else {
attached_++; attached_++;
SCOPE_EXIT { SCOPE_EXIT {
context_ = {}; context_.~Context();
callback_ = {}; callback_.~Callback();
detachOne(); detachOne();
}; };
RequestContextScopeGuard rctx(context_); RequestContextScopeGuard rctx(context_);
callback_(std::move(*result_)); callback_(std::move(result_));
} }
} }
...@@ -553,21 +557,23 @@ class Core final { ...@@ -553,21 +557,23 @@ class Core final {
void derefCallback() noexcept { void derefCallback() noexcept {
if (--callbackReferences_ == 0) { if (--callbackReferences_ == 0) {
context_ = {}; context_.~Context();
callback_ = {}; callback_.~Callback();
} }
} }
template <typename Self> using Result = Try<T>;
static decltype(auto) getTryImpl(Self& self) { using Callback = folly::Function<void(Result&&)>;
assume(self.result_.has_value()); using Context = std::shared_ptr<RequestContext>;
return self.result_.value();
}
folly::Function<void(Try<T>&&)> callback_; union {
Callback callback_;
};
// place result_ next to increase the likelihood that the value will be // place result_ next to increase the likelihood that the value will be
// contained entirely in one cache line // contained entirely in one cache line
folly::Optional<Try<T>> result_; union {
Result result_;
};
std::atomic<State> state_; std::atomic<State> state_;
std::atomic<unsigned char> attached_; std::atomic<unsigned char> attached_;
std::atomic<unsigned char> callbackReferences_{0}; std::atomic<unsigned char> callbackReferences_{0};
...@@ -575,7 +581,9 @@ class Core final { ...@@ -575,7 +581,9 @@ class Core final {
SpinLock interruptLock_; SpinLock interruptLock_;
int8_t priority_ {-1}; int8_t priority_ {-1};
Executor::KeepAlive<> executor_; Executor::KeepAlive<> executor_;
std::shared_ptr<RequestContext> context_ {nullptr}; union {
Context context_;
};
std::unique_ptr<exception_wrapper> interrupt_ {}; std::unique_ptr<exception_wrapper> interrupt_ {};
std::function<void(exception_wrapper const&)> interruptHandler_ {nullptr}; std::function<void(exception_wrapper const&)> interruptHandler_ {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