From 047764482fcc09ae2b851f331f9db6e23b710c54 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 16 Sep 2020 23:52:42 -0700 Subject: [PATCH] Fabric: Simplifying ShadowTreeRevision implementation Summary: The implementation of this class is too complex for the purpose it serves. Making it simpler will make the code simpler and faster. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23725688 fbshipit-source-id: 5e1ecddb0dd3c4c4f94786e2ba0af9b67e7426ce --- .../renderer/mounting/MountingCoordinator.cpp | 26 +++++++------- .../renderer/mounting/MountingCoordinator.h | 2 +- .../renderer/mounting/ShadowTreeRevision.cpp | 26 -------------- .../renderer/mounting/ShadowTreeRevision.h | 36 +++---------------- 4 files changed, 17 insertions(+), 73 deletions(-) diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 7ad2b08e5e5..1a925ed91db 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -23,13 +23,13 @@ MountingCoordinator::MountingCoordinator( ShadowTreeRevision baseRevision, std::weak_ptr delegate, bool enableReparentingDetection) - : surfaceId_(baseRevision.getRootShadowNode().getSurfaceId()), + : surfaceId_(baseRevision.rootShadowNode->getSurfaceId()), baseRevision_(baseRevision), mountingOverrideDelegate_(delegate), telemetryController_(*this), enableReparentingDetection_(enableReparentingDetection) { #ifdef RN_SHADOW_TREE_INTROSPECTION - stubViewTree_ = stubViewTreeFromShadowNode(baseRevision_.getRootShadowNode()); + stubViewTree_ = stubViewTreeFromShadowNode(*baseRevision_.rootShadowNode); #endif } @@ -37,17 +37,15 @@ SurfaceId MountingCoordinator::getSurfaceId() const { return surfaceId_; } -void MountingCoordinator::push(ShadowTreeRevision &&revision) const { +void MountingCoordinator::push(ShadowTreeRevision const &revision) const { { std::lock_guard lock(mutex_); assert( - !lastRevision_.has_value() || - revision.getNumber() != lastRevision_->getNumber()); + !lastRevision_.has_value() || revision.number != lastRevision_->number); - if (!lastRevision_.has_value() || - lastRevision_->getNumber() < revision.getNumber()) { - lastRevision_ = std::move(revision); + if (!lastRevision_.has_value() || lastRevision_->number < revision.number) { + lastRevision_ = revision; } } @@ -60,7 +58,7 @@ void MountingCoordinator::revoke() const { // 1. We need to stop retaining `ShadowNode`s to not prolong their lifetime // to prevent them from overliving `ComponentDescriptor`s. // 2. A possible call to `pullTransaction()` should return empty optional. - baseRevision_.rootShadowNode_.reset(); + baseRevision_.rootShadowNode.reset(); lastRevision_.reset(); } @@ -90,12 +88,12 @@ better::optional MountingCoordinator::pullTransaction() if (lastRevision_.has_value()) { number_++; - auto telemetry = lastRevision_->getTelemetry(); + auto telemetry = lastRevision_->telemetry; telemetry.willDiff(); auto mutations = calculateShadowViewMutations( - baseRevision_.getRootShadowNode(), lastRevision_->getRootShadowNode()); + *baseRevision_.rootShadowNode, *lastRevision_->rootShadowNode); telemetry.didDiff(); @@ -145,11 +143,11 @@ better::optional MountingCoordinator::pullTransaction() auto line = std::string{}; auto stubViewTree = - stubViewTreeFromShadowNode(baseRevision_.getRootShadowNode()); + stubViewTreeFromShadowNode(*baseRevision_.rootShadowNode); if (stubViewTree_ != stubViewTree) { std::stringstream ssOldTree( - baseRevision_.getRootShadowNode().getDebugDescription()); + baseRevision_.rootShadowNode->getDebugDescription()); while (std::getline(ssOldTree, line, '\n')) { LOG(ERROR) << "Old tree:" << line; } @@ -160,7 +158,7 @@ better::optional MountingCoordinator::pullTransaction() } std::stringstream ssNewTree( - lastRevision_->getRootShadowNode().getDebugDescription()); + lastRevision_->rootShadowNode->getDebugDescription()); while (std::getline(ssNewTree, line, '\n')) { LOG(ERROR) << "New tree:" << line; } diff --git a/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/ReactCommon/react/renderer/mounting/MountingCoordinator.h index 54bb0a3a63d..4563dc7bd0a 100644 --- a/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -91,7 +91,7 @@ class MountingCoordinator final { private: friend class ShadowTree; - void push(ShadowTreeRevision &&revision) const; + void push(ShadowTreeRevision const &revision) const; /* * Revokes the last pushed `ShadowTreeRevision`. diff --git a/ReactCommon/react/renderer/mounting/ShadowTreeRevision.cpp b/ReactCommon/react/renderer/mounting/ShadowTreeRevision.cpp index 998464886ae..f56d03ec202 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTreeRevision.cpp +++ b/ReactCommon/react/renderer/mounting/ShadowTreeRevision.cpp @@ -6,29 +6,3 @@ */ #include "ShadowTreeRevision.h" - -namespace facebook { -namespace react { - -using Number = ShadowTreeRevision::Number; - -ShadowTreeRevision::ShadowTreeRevision( - RootShadowNode::Shared const &rootShadowNode, - Number number, - TransactionTelemetry telemetry) - : rootShadowNode_(rootShadowNode), number_(number), telemetry_(telemetry) {} - -TransactionTelemetry const &ShadowTreeRevision::getTelemetry() const { - return telemetry_; -} - -RootShadowNode const &ShadowTreeRevision::getRootShadowNode() { - return *rootShadowNode_; -} - -Number ShadowTreeRevision::getNumber() const { - return number_; -} - -} // namespace react -} // namespace facebook diff --git a/ReactCommon/react/renderer/mounting/ShadowTreeRevision.h b/ReactCommon/react/renderer/mounting/ShadowTreeRevision.h index d2a63a00aa4..d58dbd64328 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTreeRevision.h +++ b/ReactCommon/react/renderer/mounting/ShadowTreeRevision.h @@ -30,40 +30,12 @@ class ShadowTreeRevision final { */ using Number = int64_t; - /* - * Creates the object with given root shadow node, revision number and - * telemetry. - */ - ShadowTreeRevision( - RootShadowNode::Shared const &rootShadowNode, - Number number, - TransactionTelemetry telemetry); - - /* - * Returns telemetry associated with this revision. - */ - TransactionTelemetry const &getTelemetry() const; - - /* - * Methods from this section are meant to be used by - * `MountingOverrideDelegate` only. - */ - public: - RootShadowNode const &getRootShadowNode(); - - /* - * Methods from this section are meant to be used by `MountingCoordinator` - * only. - */ - private: + friend class ShadowTree; friend class MountingCoordinator; - Number getNumber() const; - - private: - RootShadowNode::Shared rootShadowNode_; - Number number_; - TransactionTelemetry telemetry_; + RootShadowNode::Shared rootShadowNode; + Number number; + TransactionTelemetry telemetry; }; } // namespace react