Fabric: Storing data as a shared pointer inside State object

Summary:
Before this change, the concrete component-specific data/payload object associated with a State was stored inside a templated subclass as a normal instance variable; after the change, it's stored as a shared pointer inside the base class. The original motivation was that storing that inside subclass saves us one shared pointer and one heap allocation.

This approach overcomplicated a lot of things and all possible savings are probably compensated with additional complexity (we have to have templated state-update lamdas in subclasses and so on). And to update the data in the previous approach we need to create a shared pointer to data anyway.

This change will allow future improvements in the coming diff.

 Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D19799014

fbshipit-source-id: 287ed939353ba58d9e434d1502ecfbb208c6daa5
This commit is contained in:
Valentin Shergin
2020-02-09 22:28:39 -08:00
committed by Facebook Github Bot
parent c525ab5e04
commit 085c6d2675
6 changed files with 54 additions and 28 deletions
@@ -126,14 +126,15 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
}
return std::make_shared<ConcreteState>(
ConcreteShadowNode::initialStateData(
fragment, family->getSurfaceId(), *this),
std::make_shared<ConcreteStateData const>(
ConcreteShadowNode::initialStateData(
fragment, family->getSurfaceId(), *this)),
family);
}
virtual State::Shared createState(
ShadowNodeFamily::Shared const &family,
const StateData::Shared &data) const override {
StateData::Shared const &data) const override {
if (std::is_same<ConcreteStateData, StateData>::value) {
// Default case: Returning `null` for nodes that don't use `State`.
return nullptr;
@@ -141,8 +142,8 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
assert(data && "Provided `data` is nullptr.");
return std::make_shared<const ConcreteState>(
std::move(*std::static_pointer_cast<ConcreteStateData>(data)), family);
return std::make_shared<ConcreteState const>(
std::static_pointer_cast<ConcreteStateData const>(data), family);
}
virtual ShadowNodeFamily::Shared createFamily(
@@ -106,7 +106,8 @@ class ConcreteShadowNode : public ShadowNode {
*/
void setStateData(ConcreteStateData &&data) {
ensureUnsealed();
state_ = std::make_shared<ConcreteState const>(std::move(data), *state_);
state_ = std::make_shared<ConcreteState const>(
std::make_shared<ConcreteStateData const>(std::move(data)), *state_);
}
/*
+19 -12
View File
@@ -26,20 +26,30 @@ class ConcreteState : public State {
public:
using Shared = std::shared_ptr<const ConcreteState>;
using Data = DataT;
using SharedData = std::shared_ptr<Data const>;
explicit ConcreteState(Data &&data, ShadowNodeFamily::Shared const &family)
: State(family), data_(std::move(data)) {}
/*
* Creates an updated `State` object with given previous one and `data`.
*/
explicit ConcreteState(SharedData const &data, State const &state)
: State(data, state) {}
explicit ConcreteState(Data &&data, State const &other)
: State(other), data_(std::move(data)) {}
/*
* Creates a first-of-its-family `State` object with given `family` and
* `data`.
*/
explicit ConcreteState(
SharedData const &data,
ShadowNodeFamily::Shared const &family)
: State(data, family) {}
virtual ~ConcreteState() = default;
/*
* Returns stored data.
*/
const Data &getData() const {
return data_;
Data const &getData() const {
return *std::static_pointer_cast<Data const>(data_);
}
/*
@@ -67,7 +77,7 @@ class ConcreteState : public State {
* of conflict.
*/
void updateState(
std::function<Data(const Data &oldData)> callback,
std::function<Data(Data const &oldData)> callback,
EventPriority priority = EventPriority::AsynchronousBatched) const {
family_->dispatchRawState(
{[family = family_, callback = std::move(callback)]()
@@ -84,16 +94,13 @@ class ConcreteState : public State {
#ifdef ANDROID
folly::dynamic getDynamic() const override {
return data_.getDynamic();
return getData().getDynamic();
}
void updateState(folly::dynamic data) const override {
updateState(std::move(Data(data_, data)));
updateState(std::move(Data(getData(), data)));
}
#endif
private:
DataT data_;
};
} // namespace react
+7 -4
View File
@@ -11,6 +11,7 @@
#include <react/core/ShadowNode.h>
#include <react/core/ShadowNodeFragment.h>
#include <react/core/State.h>
#include <react/core/StateData.h>
#include <react/core/StateTarget.h>
#ifdef ANDROID
@@ -20,11 +21,13 @@
namespace facebook {
namespace react {
State::State(State const &state)
: family_(state.family_), revision_(state.revision_ + 1){};
State::State(StateData::Shared const &data, State const &state)
: family_(state.family_), data_(data), revision_(state.revision_ + 1){};
State::State(ShadowNodeFamily::Shared const &family)
: family_(family), revision_{1} {};
State::State(
StateData::Shared const &data,
ShadowNodeFamily::Shared const &family)
: family_(family), data_(data), revision_{1} {};
void State::commit(std::shared_ptr<ShadowNode const> const &shadowNode) const {
family_->setTarget(StateTarget{shadowNode});
+19 -5
View File
@@ -24,8 +24,17 @@ class State {
public:
using Shared = std::shared_ptr<const State>;
explicit State(State const &state);
explicit State(ShadowNodeFamily::Shared const &family);
protected:
/*
* Constructors are protected to make calling them directly with
* type-erasured arguments impossible.
*/
explicit State(StateData::Shared const &data, State const &state);
explicit State(
StateData::Shared const &data,
ShadowNodeFamily::Shared const &family);
public:
virtual ~State() = default;
/*
@@ -53,12 +62,17 @@ class State {
void commit(std::shared_ptr<ShadowNode const> const &shadowNode) const;
protected:
ShadowNodeFamily::Shared family_;
private:
friend class StateCoordinator;
friend class ShadowNodeFamily;
ShadowNodeFamily::Shared family_;
/*
* Type-erasured pointer to arbitrary component-specific data held by the
* `State`.
*/
StateData::Shared data_;
/*
* Indicates that the state was committed once and then was replaced by a
* newer one.
+1 -1
View File
@@ -21,7 +21,7 @@ namespace react {
* don't have a state.
*/
struct StateData final {
using Shared = std::shared_ptr<void>;
using Shared = std::shared_ptr<void const>;
#ifdef ANDROID
StateData() = default;