Skip to content

Commit

Permalink
Enable inlining of trivial CoreBase methods
Browse files Browse the repository at this point in the history
Summary: D22371899 et seq outlined part of the CoreBase implementation. However, in some cases (at least for opt builds), inlining trivial methods makes the code smaller. This comes from a combination of avoiding register spills and reducing exception handling code.

Reviewed By: ot

Differential Revision: D48133157

fbshipit-source-id: fac62aded9b298c92d8d3b75fd88d069d3388728
  • Loading branch information
bmaurer authored and facebook-github-bot committed Aug 10, 2023
1 parent d969a5c commit 27584ea
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 36 deletions.
32 changes: 0 additions & 32 deletions folly/futures/detail/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,6 @@ void UniqueDeleter::operator()(DeferredExecutor* ptr) {
}
}

KeepAliveOrDeferred::KeepAliveOrDeferred() noexcept : state_(State::Deferred) {
::new (&deferred_) DW{};
}

KeepAliveOrDeferred::KeepAliveOrDeferred(KA ka) noexcept
: state_(State::KeepAlive) {
::new (&keepAlive_) KA{std::move(ka)};
}

KeepAliveOrDeferred::KeepAliveOrDeferred(DW deferred) noexcept
: state_(State::Deferred) {
::new (&deferred_) DW{std::move(deferred)};
}

KeepAliveOrDeferred::KeepAliveOrDeferred(KeepAliveOrDeferred&& other) noexcept
: state_(other.state_) {
switch (state_) {
Expand Down Expand Up @@ -316,21 +302,6 @@ bool CoreBase::hasResult() const noexcept {
return State() != (state & allowed);
}

Executor* CoreBase::getExecutor() const {
if (!executor_.isKeepAlive()) {
return nullptr;
}
return executor_.getKeepAliveExecutor();
}

DeferredExecutor* CoreBase::getDeferredExecutor() const {
if (!executor_.isDeferred()) {
return {};
}

return executor_.getDeferredExecutor();
}

DeferredWrapper CoreBase::stealDeferredExecutor() {
if (executor_.isKeepAlive()) {
return {};
Expand Down Expand Up @@ -436,9 +407,6 @@ class CoreBase::CoreAndCallbackReference {
CoreBase* core_{nullptr};
};

CoreBase::CoreBase(State state, unsigned char attached)
: state_(state), attached_(attached) {}

CoreBase::~CoreBase() {
auto interrupt = interrupt_.load(std::memory_order_acquire);
auto pointer = interrupt & ~InterruptMask;
Expand Down
33 changes: 29 additions & 4 deletions folly/futures/detail/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,18 @@ class KeepAliveOrDeferred {
using DW = DeferredWrapper;

public:
KeepAliveOrDeferred() noexcept;
/* implicit */ KeepAliveOrDeferred(KA ka) noexcept;
/* implicit */ KeepAliveOrDeferred(DW deferred) noexcept;
KeepAliveOrDeferred() noexcept : state_(State::Deferred), deferred_(DW{}) {}

/* implicit */ KeepAliveOrDeferred(KA ka) noexcept
: state_(State::KeepAlive) {
::new (&keepAlive_) KA{std::move(ka)};
}

/* implicit */ KeepAliveOrDeferred(DW deferred) noexcept
: state_(State::Deferred) {
::new (&deferred_) DW{std::move(deferred)};
}

KeepAliveOrDeferred(KeepAliveOrDeferred&& other) noexcept;

~KeepAliveOrDeferred();
Expand Down Expand Up @@ -474,7 +483,8 @@ class CoreBase {
}

protected:
CoreBase(State state, unsigned char attached);
CoreBase(State state, unsigned char attached) noexcept
: state_(state), attached_(attached) {}

virtual ~CoreBase();

Expand Down Expand Up @@ -701,6 +711,21 @@ class Core final : private ResultHolder<T>, public CoreBase {
}
};

inline Executor* CoreBase::getExecutor() const {
if (!executor_.isKeepAlive()) {
return nullptr;
}
return executor_.getKeepAliveExecutor();
}

inline DeferredExecutor* CoreBase::getDeferredExecutor() const {
if (!executor_.isDeferred()) {
return {};
}

return executor_.getDeferredExecutor();
}

#if FOLLY_USE_EXTERN_FUTURE_UNIT
// limited to the instances unconditionally forced by the futures library
extern template class Core<folly::Unit>;
Expand Down

0 comments on commit 27584ea

Please sign in to comment.