From af0daaf583289ce6a6b8d565b4bd3666a013f834 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sat, 20 Apr 2019 10:49:53 -0700 Subject: [PATCH] Fabric: Introducing MountingTransaction Summary: `MountingTransaction` encapsulates all artifacts of `ShadowTree` commit, particularly list of mutations and meta-data. We will rely on this heavily in the coming diffs. Reviewed By: JoshuaGross Differential Revision: D15021795 fbshipit-source-id: 811da7afd7b929a34a81aa66566193d46bbc34f8 --- React/Fabric/Mounting/RCTMountingManager.h | 6 +- React/Fabric/Mounting/RCTMountingManager.mm | 10 +- React/Fabric/RCTScheduler.h | 4 +- React/Fabric/RCTScheduler.mm | 8 +- React/Fabric/RCTSurfacePresenter.mm | 7 +- .../facebook/react/fabric/jsi/jni/Binding.cpp | 13 +-- .../facebook/react/fabric/jsi/jni/Binding.h | 5 +- .../fabric/mounting/MountingTransaction.cpp | 47 +++++++++ .../fabric/mounting/MountingTransaction.h | 95 +++++++++++++++++++ ReactCommon/fabric/mounting/ShadowTree.cpp | 21 ++-- ReactCommon/fabric/mounting/ShadowTree.h | 14 +-- .../fabric/mounting/ShadowTreeDelegate.h | 6 +- ReactCommon/fabric/mounting/ShadowView.h | 2 + ReactCommon/fabric/uimanager/Scheduler.cpp | 7 +- ReactCommon/fabric/uimanager/Scheduler.h | 4 +- .../fabric/uimanager/SchedulerDelegate.h | 6 +- 16 files changed, 188 insertions(+), 67 deletions(-) create mode 100644 ReactCommon/fabric/mounting/MountingTransaction.cpp create mode 100644 ReactCommon/fabric/mounting/MountingTransaction.h diff --git a/React/Fabric/Mounting/RCTMountingManager.h b/React/Fabric/Mounting/RCTMountingManager.h index 28f7858028..8233bbb7d9 100644 --- a/React/Fabric/Mounting/RCTMountingManager.h +++ b/React/Fabric/Mounting/RCTMountingManager.h @@ -11,8 +11,8 @@ #import #import #import +#import #import -#import NS_ASSUME_NONNULL_BEGIN @@ -27,10 +27,10 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, strong) RCTComponentViewRegistry *componentViewRegistry; /** - * Schedule mutations to be performed on the main thread. + * Schedule a mounting transaction to be performed on the main thread. * Can be called from any thread. */ -- (void)scheduleMutations:(facebook::react::ShadowViewMutationList const &)mutations rootTag:(ReactTag)rootTag; +- (void)scheduleTransaction:(facebook::react::MountingTransaction &&)mountingTransaction; /** * Suggests preliminary creation of a component view of given type. diff --git a/React/Fabric/Mounting/RCTMountingManager.mm b/React/Fabric/Mounting/RCTMountingManager.mm index 1dc7423555..333e2c5568 100644 --- a/React/Fabric/Mounting/RCTMountingManager.mm +++ b/React/Fabric/Mounting/RCTMountingManager.mm @@ -211,23 +211,23 @@ static void RNPerformMountInstructions(ShadowViewMutationList const &mutations, return self; } -- (void)scheduleMutations:(ShadowViewMutationList const &)mutations rootTag:(ReactTag)rootTag +- (void)scheduleTransaction:(facebook::react::MountingTransaction &&)mountingTransaction; { if (RCTIsMainQueue()) { // Already on the proper thread, so: // * No need to do a thread jump; // * No need to do expensive copy of all mutations; // * No need to allocate a block. - [self mountMutations:mutations rootTag:rootTag]; + [self mountMutations:mountingTransaction.getMutations() rootTag:mountingTransaction.getSurfaceId()]; return; } - // We need a non-reference for `mutations` to allow copy semantic. - auto mutationsCopy = mutations; + // We need a non-reference for `mountingTransaction` to allow copy semantic. + __block auto mountingTransactionCopy = MountingTransaction{mountingTransaction}; RCTExecuteOnMainQueue(^{ RCTAssertMainQueue(); - [self mountMutations:mutationsCopy rootTag:rootTag]; + [self mountMutations:mountingTransactionCopy.getMutations() rootTag:mountingTransactionCopy.getSurfaceId()]; }); } diff --git a/React/Fabric/RCTScheduler.h b/React/Fabric/RCTScheduler.h index 14fa84679e..04d412a9c2 100644 --- a/React/Fabric/RCTScheduler.h +++ b/React/Fabric/RCTScheduler.h @@ -12,6 +12,7 @@ #import #import #import +#import #import #import #import @@ -25,8 +26,7 @@ NS_ASSUME_NONNULL_BEGIN */ @protocol RCTSchedulerDelegate -- (void)schedulerDidFinishTransaction:(facebook::react::ShadowViewMutationList const &)mutations - rootTag:(ReactTag)rootTag; +- (void)schedulerDidFinishTransaction:(facebook::react::MountingTransaction &&)mountingTransaction; - (void)schedulerOptimisticallyCreateComponentViewWithComponentHandle:(facebook::react::ComponentHandle)componentHandle; diff --git a/React/Fabric/RCTScheduler.mm b/React/Fabric/RCTScheduler.mm index 0eeebf3413..53fd67fc55 100644 --- a/React/Fabric/RCTScheduler.mm +++ b/React/Fabric/RCTScheduler.mm @@ -22,14 +22,10 @@ class SchedulerDelegateProxy : public SchedulerDelegate { public: SchedulerDelegateProxy(void *scheduler) : scheduler_(scheduler) {} - void schedulerDidFinishTransaction( - Tag rootTag, - const ShadowViewMutationList &mutations, - const long commitStartTime, - const long layoutTime) override + void schedulerDidFinishTransaction(MountingTransaction &&mountingTransaction) override { RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_; - [scheduler.delegate schedulerDidFinishTransaction:mutations rootTag:rootTag]; + [scheduler.delegate schedulerDidFinishTransaction:std::move(mountingTransaction)]; } void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowView &shadowView) override diff --git a/React/Fabric/RCTSurfacePresenter.mm b/React/Fabric/RCTSurfacePresenter.mm index cb8666ad73..d37c0c9b7d 100644 --- a/React/Fabric/RCTSurfacePresenter.mm +++ b/React/Fabric/RCTSurfacePresenter.mm @@ -309,14 +309,13 @@ using namespace facebook::react; #pragma mark - RCTSchedulerDelegate -- (void)schedulerDidFinishTransaction:(facebook::react::ShadowViewMutationList const &)mutations - rootTag:(ReactTag)rootTag +- (void)schedulerDidFinishTransaction:(facebook::react::MountingTransaction &&)mountingTransaction { - RCTFabricSurface *surface = [_surfaceRegistry surfaceForRootTag:rootTag]; + RCTFabricSurface *surface = [_surfaceRegistry surfaceForRootTag:mountingTransaction.getSurfaceId()]; [surface _setStage:RCTSurfaceStagePrepared]; - [_mountingManager scheduleMutations:mutations rootTag:rootTag]; + [_mountingManager scheduleTransaction:std::move(mountingTransaction)]; } - (void)schedulerOptimisticallyCreateComponentViewWithComponentHandle:(ComponentHandle)componentHandle diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp index 1e89d4dc42..d5e716be12 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.cpp @@ -329,11 +329,12 @@ local_ref createDeleteMountItem( } void Binding::schedulerDidFinishTransaction( - const Tag rootTag, - const ShadowViewMutationList& mutations, - const long commitStartTime, - const long layoutTime) { + MountingTransaction &&mountingTransaction) { SystraceSection s("FabricUIManager::schedulerDidFinishTransaction"); + + auto telemetry = mountingTransaction.getTelemetry(); + auto mutations = mountingTransaction.getMutations(); + std::vector> queue; // Upper bound estimation of mount items to be delivered to Java side. int size = mutations.size() * 3 + 42; @@ -466,8 +467,8 @@ void Binding::schedulerDidFinishTransaction( scheduleMountItems( javaUIManager_, batch.get(), - commitStartTime, - layoutTime, + telemetry.commitStartTime, + telemetry.layoutTime, finishTransactionStartTime, finishTransactionEndTime); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h index b17e3f3522..535245c1ba 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jsi/jni/Binding.h @@ -57,10 +57,7 @@ class Binding : public jni::HybridClass, public SchedulerDelegate { void stopSurface(jint surfaceId); void schedulerDidFinishTransaction( - const Tag rootTag, - const ShadowViewMutationList& mutations, - const long commitStartTime, - const long layoutTime); + MountingTransaction &&mountingTransaction); void schedulerDidRequestPreliminaryViewAllocation( const SurfaceId surfaceId, diff --git a/ReactCommon/fabric/mounting/MountingTransaction.cpp b/ReactCommon/fabric/mounting/MountingTransaction.cpp new file mode 100644 index 0000000000..1e9d801ccf --- /dev/null +++ b/ReactCommon/fabric/mounting/MountingTransaction.cpp @@ -0,0 +1,47 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include "MountingTransaction.h" + +namespace facebook { +namespace react { + +using Telemetry = MountingTransaction::Telemetry; +using Revision = MountingTransaction::Revision; + +MountingTransaction::MountingTransaction( + SurfaceId surfaceId, + Revision revision, + ShadowViewMutationList &&mutations, + Telemetry telemetry) + : surfaceId_(surfaceId), + revision_(revision), + mutations_(std::move(mutations)), + telemetry_(std::move(telemetry)) {} + +ShadowViewMutationList const &MountingTransaction::getMutations() const & { + return mutations_; +} + +ShadowViewMutationList MountingTransaction::getMutations() && { + return std::move(mutations_); +} + +Telemetry const &MountingTransaction::getTelemetry() const { + return telemetry_; +} + +SurfaceId MountingTransaction::getSurfaceId() const { + return surfaceId_; +} + +Revision MountingTransaction::getRevision() const { + return revision_; +} + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/mounting/MountingTransaction.h b/ReactCommon/fabric/mounting/MountingTransaction.h new file mode 100644 index 0000000000..80ff2e2568 --- /dev/null +++ b/ReactCommon/fabric/mounting/MountingTransaction.h @@ -0,0 +1,95 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include + +namespace facebook { +namespace react { + +/* + * Encapsulates all artifacts of `ShadowTree` commit, particularly list of + * mutations and meta-data. + * Movable and copyable, but moving is strongly preferred. + * A moved-from object of this type has unspecified value and accessing that is + * UB. + */ +class MountingTransaction final { + public: + /* + * Revision grows continuously starting from `1`. Value `0` represents the + * state before the very first transaction happens. + */ + using Revision = int64_t; + + /* + * Represent arbitrary telementry data that can be associated with the + * particular transaction. + */ + struct Telemetry final { + long commitStartTime{}; + long layoutTime{}; + }; + + /* + * Copying a list of `ShadowViewMutation` is expensive, so the constructor + * accepts it as rvalue reference to discourage copying. + */ + MountingTransaction( + SurfaceId surfaceId, + Revision revision, + ShadowViewMutationList &&mutations, + Telemetry telemetry); + + /* + * Copy semantic. + * Copying of MountingTransaction is expensive, so copy-constructor is + * explicit and copy-assignment is deleted to prevent accidental copying. + */ + explicit MountingTransaction(const MountingTransaction &mountingTransaction) = + default; + MountingTransaction &operator=(const MountingTransaction &other) = delete; + + /* + * Move semantic. + */ + MountingTransaction(MountingTransaction &&mountingTransaction) noexcept = + default; + MountingTransaction &operator=(MountingTransaction &&other) = default; + + /* + * Returns a list of mutations that represent the transaction. The list can be + * empty (theoretically). + */ + ShadowViewMutationList const &getMutations() const &; + ShadowViewMutationList getMutations() &&; + + /* + * Returns telemetry associated with this transaction. + */ + Telemetry const &getTelemetry() const; + + /* + * Returns the id of the surface that the transaction belongs to. + */ + SurfaceId getSurfaceId() const; + + /* + * Returns the revision of the ShadowTree that this transaction represents. + */ + Revision getRevision() const; + + private: + SurfaceId surfaceId_; + Revision revision_; + ShadowViewMutationList mutations_; + Telemetry telemetry_; +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/fabric/mounting/ShadowTree.cpp b/ReactCommon/fabric/mounting/ShadowTree.cpp index 9bc002cb68..c2ec749e82 100644 --- a/ReactCommon/fabric/mounting/ShadowTree.cpp +++ b/ReactCommon/fabric/mounting/ShadowTree.cpp @@ -127,15 +127,14 @@ Tag ShadowTree::getSurfaceId() const { void ShadowTree::commit( ShadowTreeCommitTransaction transaction, - long commitStartTime, - int *revision) const { + long commitStartTime) const { SystraceSection s("ShadowTree::commit"); int attempts = 0; while (true) { attempts++; - if (tryCommit(transaction, commitStartTime, revision)) { + if (tryCommit(transaction, commitStartTime)) { return; } @@ -147,8 +146,7 @@ void ShadowTree::commit( bool ShadowTree::tryCommit( ShadowTreeCommitTransaction transaction, - long commitStartTime, - int *revision) const { + long commitStartTime) const { SystraceSection s("ShadowTree::tryCommit"); SharedRootShadowNode oldRootShadowNode; @@ -170,6 +168,7 @@ bool ShadowTree::tryCommit( layoutTime = getTime() - layoutTime; newRootShadowNode->sealRecursive(); + int revision; auto mutations = calculateShadowViewMutations(*oldRootShadowNode, *newRootShadowNode); @@ -190,13 +189,9 @@ bool ShadowTree::tryCommit( oldRootShadowNode->getChildren(), newRootShadowNode->getChildren()); } + revision = revision_; revision_++; - // Returning last revision if requested. - if (revision) { - *revision = revision_; - } - #ifdef RN_SHADOW_TREE_INTROSPECTION stubViewTree_.mutate(mutations); auto stubViewTree = stubViewTreeFromShadowNode(*rootShadowNode_); @@ -219,7 +214,11 @@ bool ShadowTree::tryCommit( if (delegate_) { delegate_->shadowTreeDidCommit( - *this, mutations, commitStartTime, layoutTime); + *this, + {surfaceId_, + revision, + std::move(mutations), + {commitStartTime, layoutTime}}); } return true; diff --git a/ReactCommon/fabric/mounting/ShadowTree.h b/ReactCommon/fabric/mounting/ShadowTree.h index 5d5b83edb4..cc328c4195 100644 --- a/ReactCommon/fabric/mounting/ShadowTree.h +++ b/ReactCommon/fabric/mounting/ShadowTree.h @@ -55,22 +55,16 @@ class ShadowTree final { * Performs commit calling `transaction` function with a `oldRootShadowNode` * and expecting a `newRootShadowNode` as a return value. * The `transaction` function can abort commit returning `nullptr`. - * If a `revision` pointer is not null, the method will store there a - * contiguous revision number of the successfully performed transaction. * Returns `true` if the operation finished successfully. */ - bool tryCommit( - ShadowTreeCommitTransaction transaction, - long commitStartTime, - int *revision = nullptr) const; + bool tryCommit(ShadowTreeCommitTransaction transaction, long commitStartTime) + const; /* * Calls `tryCommit` in a loop until it finishes successfully. */ - void commit( - ShadowTreeCommitTransaction transaction, - long commitStartTime, - int *revision = nullptr) const; + void commit(ShadowTreeCommitTransaction transaction, long commitStartTime) + const; #pragma mark - Delegate diff --git a/ReactCommon/fabric/mounting/ShadowTreeDelegate.h b/ReactCommon/fabric/mounting/ShadowTreeDelegate.h index 8ab9abf5d6..0e17cd2349 100644 --- a/ReactCommon/fabric/mounting/ShadowTreeDelegate.h +++ b/ReactCommon/fabric/mounting/ShadowTreeDelegate.h @@ -5,7 +5,7 @@ #pragma once -#include +#include namespace facebook { namespace react { @@ -22,9 +22,7 @@ class ShadowTreeDelegate { */ virtual void shadowTreeDidCommit( const ShadowTree &shadowTree, - const ShadowViewMutationList &mutations, - long commitStartTime, - long layoutTime) const = 0; + MountingTransaction &&transaction) const = 0; virtual ~ShadowTreeDelegate() noexcept = default; }; diff --git a/ReactCommon/fabric/mounting/ShadowView.h b/ReactCommon/fabric/mounting/ShadowView.h index a854421abf..97ece0ae88 100644 --- a/ReactCommon/fabric/mounting/ShadowView.h +++ b/ReactCommon/fabric/mounting/ShadowView.h @@ -23,6 +23,7 @@ namespace react { struct ShadowView final { ShadowView() = default; ShadowView(const ShadowView &shadowView) = default; + ShadowView(ShadowView &&shadowView) noexcept = default; ~ShadowView(){}; @@ -32,6 +33,7 @@ struct ShadowView final { explicit ShadowView(const ShadowNode &shadowNode); ShadowView &operator=(const ShadowView &other) = default; + ShadowView &operator=(ShadowView &&other) = default; bool operator==(const ShadowView &rhs) const; bool operator!=(const ShadowView &rhs) const; diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index be018de2bc..c663933376 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -242,14 +242,11 @@ SchedulerDelegate *Scheduler::getDelegate() const { void Scheduler::shadowTreeDidCommit( const ShadowTree &shadowTree, - const ShadowViewMutationList &mutations, - long commitStartTime, - long layoutTime) const { + MountingTransaction &&transaction) const { SystraceSection s("Scheduler::shadowTreeDidCommit"); if (delegate_) { - delegate_->schedulerDidFinishTransaction( - shadowTree.getSurfaceId(), mutations, commitStartTime, layoutTime); + delegate_->schedulerDidFinishTransaction(std::move(transaction)); } } diff --git a/ReactCommon/fabric/uimanager/Scheduler.h b/ReactCommon/fabric/uimanager/Scheduler.h index 358e5999cb..f164d50deb 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.h +++ b/ReactCommon/fabric/uimanager/Scheduler.h @@ -93,9 +93,7 @@ class Scheduler final : public UIManagerDelegate, public ShadowTreeDelegate { void shadowTreeDidCommit( const ShadowTree &shadowTree, - const ShadowViewMutationList &mutations, - long commitStartTime, - long layoutTime) const override; + MountingTransaction &&transaction) const override; private: SchedulerDelegate *delegate_; diff --git a/ReactCommon/fabric/uimanager/SchedulerDelegate.h b/ReactCommon/fabric/uimanager/SchedulerDelegate.h index cc5fe052b6..39996053c5 100644 --- a/ReactCommon/fabric/uimanager/SchedulerDelegate.h +++ b/ReactCommon/fabric/uimanager/SchedulerDelegate.h @@ -9,6 +9,7 @@ #include #include +#include #include namespace facebook { @@ -25,10 +26,7 @@ class SchedulerDelegate { * to construct a new one. */ virtual void schedulerDidFinishTransaction( - Tag rootTag, - ShadowViewMutationList const &mutations, - const long commitStartTime, - const long layoutTime) = 0; + MountingTransaction &&mountingTransaction) = 0; /* * Called right after a new ShadowNode was created.