From 5a58ca41444f30d42eb0fda57dea688a92be7f32 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Wed, 16 Jan 2019 20:17:00 -0800 Subject: [PATCH] Fabric: New non-blocking treading model for ShadowTree Summary: Instead of the whole family of commit* and complete* methods, now we have one single `commit` method which performs pre- and post-commit operations and swap pointers in a thread-safe manner. The `commit` operation is also exposing `revision` number and allows perform multiple commit attempts. `completeByReplacingShadowNode`, `measure` and `constraintLayout` are also going away to RootShadowNode class in the next commits. Why? * Nicer API; * No more recursive_mutex, no more problems with thread jumps; * All mutex locks are now leaf-locks, so no more deadlocks possible; * Exposing `revision` should help with debugging races. Reviewed By: sahrens Differential Revision: D13613942 fbshipit-source-id: 94e797d2f7860717847e823b5d97c4f7b35f08df --- ReactCommon/fabric/uimanager/Scheduler.cpp | 31 +++- ReactCommon/fabric/uimanager/ShadowTree.cpp | 161 +++++++++++--------- ReactCommon/fabric/uimanager/ShadowTree.h | 39 ++--- 3 files changed, 124 insertions(+), 107 deletions(-) diff --git a/ReactCommon/fabric/uimanager/Scheduler.cpp b/ReactCommon/fabric/uimanager/Scheduler.cpp index 8c285dea029..0efafbd4803 100644 --- a/ReactCommon/fabric/uimanager/Scheduler.cpp +++ b/ReactCommon/fabric/uimanager/Scheduler.cpp @@ -104,8 +104,14 @@ void Scheduler::renderTemplateToSurface( reactNativeConfig_); shadowTreeRegistry_.visit(surfaceId, [=](const ShadowTree &shadowTree) { - shadowTree.complete( - std::make_shared(SharedShadowNodeList{tree})); + return shadowTree.commit( + [&](const SharedRootShadowNode &oldRootShadowNode) { + return std::make_shared( + *oldRootShadowNode, + ShadowNodeFragment{.children = + std::make_shared( + SharedShadowNodeList{tree})}); + }); }); } catch (const std::exception &e) { LOG(ERROR) << " >>>> EXCEPTION <<< rendering uiTemplate in " @@ -118,8 +124,13 @@ void Scheduler::stopSurface(SurfaceId surfaceId) const { shadowTreeRegistry_.visit(surfaceId, [](const ShadowTree &shadowTree) { // As part of stopping the Surface, we have to commit an empty tree. - shadowTree.complete(std::const_pointer_cast( - ShadowNode::emptySharedShadowNodeSharedList())); + return shadowTree.commit( + [&](const SharedRootShadowNode &oldRootShadowNode) { + return std::make_shared( + *oldRootShadowNode, + ShadowNodeFragment{ + .children = ShadowNode::emptySharedShadowNodeSharedList()}); + }); }); auto shadowTree = shadowTreeRegistry_.remove(surfaceId); @@ -152,9 +163,7 @@ void Scheduler::constraintSurfaceLayout( SystraceSection s("Scheduler::constraintSurfaceLayout"); shadowTreeRegistry_.visit(surfaceId, [&](const ShadowTree &shadowTree) { - shadowTree.synchronize([&]() { - shadowTree.constraintLayout(layoutConstraints, layoutContext); - }); + return shadowTree.constraintLayout(layoutConstraints, layoutContext); }); } @@ -189,7 +198,13 @@ void Scheduler::uiManagerDidFinishTransaction( SystraceSection s("Scheduler::uiManagerDidFinishTransaction"); shadowTreeRegistry_.visit(surfaceId, [&](const ShadowTree &shadowTree) { - shadowTree.synchronize([&]() { shadowTree.complete(rootChildNodes); }); + shadowTree.commit( + [&](const SharedRootShadowNode &oldRootShadowNode) { + return std::make_shared( + *oldRootShadowNode, + ShadowNodeFragment{.children = rootChildNodes}); + }, + std::numeric_limits::max()); }); } diff --git a/ReactCommon/fabric/uimanager/ShadowTree.cpp b/ReactCommon/fabric/uimanager/ShadowTree.cpp index 134e9a3d1c5..0dda345fdcc 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.cpp +++ b/ReactCommon/fabric/uimanager/ShadowTree.cpp @@ -38,7 +38,12 @@ ShadowTree::ShadowTree( } ShadowTree::~ShadowTree() { - complete(std::make_shared(SharedShadowNodeList{})); + commit([](const SharedRootShadowNode &oldRootShadowNode) { + return std::make_shared( + *oldRootShadowNode, + ShadowNodeFragment{.children = + ShadowNode::emptySharedShadowNodeSharedList()}); + }); } Tag ShadowTree::getSurfaceId() const { @@ -46,15 +51,10 @@ Tag ShadowTree::getSurfaceId() const { } SharedRootShadowNode ShadowTree::getRootShadowNode() const { - std::lock_guard lock(commitMutex_); + std::shared_lock lock(commitMutex_); return rootShadowNode_; } -void ShadowTree::synchronize(std::function function) const { - std::lock_guard lock(commitMutex_); - function(); -} - #pragma mark - Layout Size ShadowTree::measure( @@ -69,14 +69,12 @@ Size ShadowTree::measure( bool ShadowTree::constraintLayout( const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const { - auto oldRootShadowNode = getRootShadowNode(); - auto newRootShadowNode = - cloneRootShadowNode(oldRootShadowNode, layoutConstraints, layoutContext); - return complete(oldRootShadowNode, newRootShadowNode); + return commit([&](const SharedRootShadowNode &oldRootShadowNode) { + return cloneRootShadowNode( + oldRootShadowNode, layoutConstraints, layoutContext); + }); } -#pragma mark - Commiting - UnsharedRootShadowNode ShadowTree::cloneRootShadowNode( const SharedRootShadowNode &oldRootShadowNode, const LayoutConstraints &layoutConstraints, @@ -88,85 +86,98 @@ UnsharedRootShadowNode ShadowTree::cloneRootShadowNode( return newRootShadowNode; } -bool ShadowTree::complete( - const SharedShadowNodeUnsharedList &rootChildNodes) const { - auto oldRootShadowNode = getRootShadowNode(); - auto newRootShadowNode = std::make_shared( - *oldRootShadowNode, - ShadowNodeFragment{.children = - SharedShadowNodeSharedList(rootChildNodes)}); - - return complete(oldRootShadowNode, newRootShadowNode); -} - bool ShadowTree::completeByReplacingShadowNode( const SharedShadowNode &oldShadowNode, const SharedShadowNode &newShadowNode) const { - auto rootShadowNode = getRootShadowNode(); - std::vector> ancestors; - oldShadowNode->constructAncestorPath(*rootShadowNode, ancestors); + return commit([&](const SharedRootShadowNode &oldRootShadowNode) { + std::vector> ancestors; + oldShadowNode->constructAncestorPath(*oldRootShadowNode, ancestors); - if (ancestors.size() == 0) { - return false; - } + if (ancestors.size() == 0) { + return UnsharedRootShadowNode{nullptr}; + } - auto oldChild = oldShadowNode; - auto newChild = newShadowNode; + auto oldChild = oldShadowNode; + auto newChild = newShadowNode; - SharedShadowNodeUnsharedList sharedChildren; + SharedShadowNodeUnsharedList sharedChildren; - for (const auto &ancestor : ancestors) { - auto children = ancestor.get().getChildren(); - std::replace(children.begin(), children.end(), oldChild, newChild); + for (const auto &ancestor : ancestors) { + auto children = ancestor.get().getChildren(); + std::replace(children.begin(), children.end(), oldChild, newChild); - sharedChildren = std::make_shared(children); + sharedChildren = std::make_shared(children); - oldChild = ancestor.get().shared_from_this(); - newChild = oldChild->clone(ShadowNodeFragment{.children = sharedChildren}); - } + oldChild = ancestor.get().shared_from_this(); + newChild = + oldChild->clone(ShadowNodeFragment{.children = sharedChildren}); + } - return complete(sharedChildren); -} - -bool ShadowTree::complete( - const SharedRootShadowNode &oldRootShadowNode, - const UnsharedRootShadowNode &newRootShadowNode) const { - SystraceSection s("ShadowTree::complete"); - newRootShadowNode->layout(); - newRootShadowNode->sealRecursive(); - - auto mutations = - calculateShadowViewMutations(*oldRootShadowNode, *newRootShadowNode); - - if (!commit(oldRootShadowNode, newRootShadowNode, mutations)) { - return false; - } - - emitLayoutEvents(mutations); - - if (delegate_) { - delegate_->shadowTreeDidCommit(*this, mutations); - } - - return true; + return std::make_shared( + *oldRootShadowNode, ShadowNodeFragment{.children = sharedChildren}); + }); } bool ShadowTree::commit( - const SharedRootShadowNode &oldRootShadowNode, - const SharedRootShadowNode &newRootShadowNode, - const ShadowViewMutationList &mutations) const { + std::function transaction, + int attempts, + int *revision) const { SystraceSection s("ShadowTree::commit"); - std::lock_guard lock(commitMutex_); - if (oldRootShadowNode != rootShadowNode_) { - return false; + while (attempts) { + attempts--; + + SharedRootShadowNode oldRootShadowNode; + + { + // Reading `rootShadowNode_` in shared manner. + std::shared_lock lock(commitMutex_); + oldRootShadowNode = rootShadowNode_; + } + + UnsharedRootShadowNode newRootShadowNode = transaction(oldRootShadowNode); + + if (!newRootShadowNode) { + break; + } + + newRootShadowNode->layout(); + newRootShadowNode->sealRecursive(); + + auto mutations = + calculateShadowViewMutations(*oldRootShadowNode, *newRootShadowNode); + + { + // Updating `rootShadowNode_` in unique manner if it hasn't changed. + std::unique_lock lock(commitMutex_); + + if (rootShadowNode_ != oldRootShadowNode) { + continue; + } + + rootShadowNode_ = newRootShadowNode; + + toggleEventEmitters(mutations); + + revision_++; + + // Returning last revision if requested. + if (revision) { + *revision = revision_; + } + } + + emitLayoutEvents(mutations); + + if (delegate_) { + delegate_->shadowTreeDidCommit(*this, mutations); + } + + return true; } - rootShadowNode_ = newRootShadowNode; - - toggleEventEmitters(mutations); - - return true; + return false; } void ShadowTree::emitLayoutEvents( diff --git a/ReactCommon/fabric/uimanager/ShadowTree.h b/ReactCommon/fabric/uimanager/ShadowTree.h index 0f8c6397ed3..0dc31a58a74 100644 --- a/ReactCommon/fabric/uimanager/ShadowTree.h +++ b/ReactCommon/fabric/uimanager/ShadowTree.h @@ -5,8 +5,9 @@ #pragma once +#include #include -#include +#include #include #include @@ -38,16 +39,6 @@ class ShadowTree final { */ SurfaceId getSurfaceId() const; - /* - * Synchronously runs `function` when `commitMutex_` is acquired. - * It is useful in cases when transactional consistency and/or successful - * commit are required. E.g. you might want to run `measure` and - * `constraintLayout` as part of a single congious transaction. - * Use this only if it is necessary. All public methods of the class are - * already thread-safe. - */ - void synchronize(std::function function) const; - #pragma mark - Layout /* @@ -71,11 +62,19 @@ class ShadowTree final { #pragma mark - Application /* - * Create a new shadow tree with given `rootChildNodes` and commit. - * Can be called from any thread. + * 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. + * Specify `attempts` to allow performing multiple tries. * Returns `true` if the operation finished successfully. */ - bool complete(const SharedShadowNodeUnsharedList &rootChildNodes) const; + bool commit( + std::function transaction, + int attempts = 1, + int *revision = nullptr) const; /* * Replaces a given old shadow node with a new one in the tree by cloning all @@ -108,22 +107,14 @@ class ShadowTree final { const LayoutConstraints &layoutConstraints, const LayoutContext &layoutContext) const; - bool complete( - const SharedRootShadowNode &oldRootShadowNode, - const UnsharedRootShadowNode &newRootShadowNode) const; - - bool commit( - const SharedRootShadowNode &oldRootShadowNode, - const SharedRootShadowNode &newRootShadowNode, - const ShadowViewMutationList &mutations) const; - void toggleEventEmitters(const ShadowViewMutationList &mutations) const; void emitLayoutEvents(const ShadowViewMutationList &mutations) const; const SurfaceId surfaceId_; + mutable folly::SharedMutex commitMutex_; mutable SharedRootShadowNode rootShadowNode_; // Protected by `commitMutex_`. + mutable int revision_{1}; // Protected by `commitMutex_`. ShadowTreeDelegate const *delegate_; - mutable std::recursive_mutex commitMutex_; }; } // namespace react