Skip to content

Commit

Permalink
Use std::shared_mutex instead of folly::shared_mutex
Browse files Browse the repository at this point in the history
Summary:
changelog: [internal]

C++17 has implementation of shared_mutex in standard library. Let's use it instead of folly.

Reviewed By: cipolleschi

Differential Revision: D43275493

fbshipit-source-id: d766251226aa230110011aca94b4e697fe0d31a1
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Feb 16, 2023
1 parent cf194ae commit e665a0f
Show file tree
Hide file tree
Showing 28 changed files with 115 additions and 136 deletions.
10 changes: 5 additions & 5 deletions React/Fabric/Mounting/RCTComponentViewFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#import <React/RCTConversions.h>

#import <butter/map.h>
#import <butter/mutex.h>
#import <butter/set.h>
#import <shared_mutex>

#import <react/renderer/componentregistry/ComponentDescriptorProviderRegistry.h>
#import <react/renderer/componentregistry/componentNameByReactViewName.h>
Expand Down Expand Up @@ -61,7 +61,7 @@ @implementation RCTComponentViewFactory {
butter::map<ComponentHandle, RCTComponentViewClassDescriptor> _componentViewClasses;
butter::set<std::string> _registeredComponentsNames;
ComponentDescriptorProviderRegistry _providerRegistry;
butter::shared_mutex _mutex;
std::shared_mutex _mutex;
}

+ (RCTComponentViewFactory *)currentComponentViewFactory
Expand Down Expand Up @@ -155,7 +155,7 @@ - (BOOL)registerComponentIfPossible:(std::string const &)name
- (void)registerComponentViewClass:(Class<RCTComponentViewProtocol>)componentViewClass
{
RCTAssert(componentViewClass, @"RCTComponentViewFactory: Provided `componentViewClass` is `nil`.");
std::unique_lock<butter::shared_mutex> lock(_mutex);
std::unique_lock lock(_mutex);

auto componentDescriptorProvider = [componentViewClass componentDescriptorProvider];
_componentViewClasses[componentDescriptorProvider.handle] =
Expand All @@ -177,7 +177,7 @@ - (void)_addDescriptorToProviderRegistry:(ComponentDescriptorProvider const &)pr
- (RCTComponentViewDescriptor)createComponentViewWithComponentHandle:(facebook::react::ComponentHandle)componentHandle
{
RCTAssertMainQueue();
std::shared_lock<butter::shared_mutex> lock(_mutex);
std::shared_lock lock(_mutex);

auto iterator = _componentViewClasses.find(componentHandle);
RCTAssert(
Expand All @@ -198,7 +198,7 @@ - (RCTComponentViewDescriptor)createComponentViewWithComponentHandle:(facebook::
- (facebook::react::ComponentDescriptorRegistry::Shared)createComponentDescriptorRegistryWithParameters:
(facebook::react::ComponentDescriptorParameters)parameters
{
std::shared_lock<butter::shared_mutex> lock(_mutex);
std::shared_lock lock(_mutex);

return _providerRegistry.createComponentDescriptorRegistry(parameters);
}
Expand Down
11 changes: 6 additions & 5 deletions React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#import "RCTSurfacePresenter.h"

#import <mutex>
#import <shared_mutex>

#import <React/RCTAssert.h>
#import <React/RCTBridge+Private.h>
Expand Down Expand Up @@ -83,7 +84,7 @@ @implementation RCTSurfacePresenter {
RuntimeExecutor _runtimeExecutor; // Protected by `_schedulerLifeCycleMutex`.
std::optional<RuntimeExecutor> _bridgelessBindingsExecutor; // Only used for installing bindings.

butter::shared_mutex _observerListMutex;
std::shared_mutex _observerListMutex;
std::vector<__weak id<RCTSurfacePresenterObserver>> _observers; // Protected by `_observerListMutex`.
}

Expand Down Expand Up @@ -390,13 +391,13 @@ - (void)schedulerDidSetIsJSResponder:(BOOL)isJSResponder

- (void)addObserver:(id<RCTSurfacePresenterObserver>)observer
{
std::unique_lock<butter::shared_mutex> lock(_observerListMutex);
std::unique_lock lock(_observerListMutex);
_observers.push_back(observer);
}

- (void)removeObserver:(id<RCTSurfacePresenterObserver>)observer
{
std::unique_lock<butter::shared_mutex> lock(_observerListMutex);
std::unique_lock lock(_observerListMutex);
std::vector<__weak id<RCTSurfacePresenterObserver>>::const_iterator it =
std::find(_observers.begin(), _observers.end(), observer);
if (it != _observers.end()) {
Expand All @@ -412,7 +413,7 @@ - (void)mountingManager:(RCTMountingManager *)mountingManager willMountComponent

NSArray<id<RCTSurfacePresenterObserver>> *observersCopy;
{
std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
std::shared_lock lock(_observerListMutex);
observersCopy = [self _getObservers];
}

Expand All @@ -429,7 +430,7 @@ - (void)mountingManager:(RCTMountingManager *)mountingManager didMountComponents

NSArray<id<RCTSurfacePresenterObserver>> *observersCopy;
{
std::shared_lock<butter::shared_mutex> lock(_observerListMutex);
std::shared_lock lock(_observerListMutex);
observersCopy = [self _getObservers];
}

Expand Down
11 changes: 5 additions & 6 deletions React/Fabric/RCTSurfaceRegistry.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#import "RCTSurfaceRegistry.h"

#import <butter/mutex.h>
#import <mutex>
#import <shared_mutex>

Expand All @@ -16,7 +15,7 @@
using namespace facebook;

@implementation RCTSurfaceRegistry {
butter::shared_mutex _mutex;
std::shared_mutex _mutex;
NSMapTable<id, RCTFabricSurface *> *_registry;
}

Expand All @@ -32,29 +31,29 @@ - (instancetype)init

- (void)enumerateWithBlock:(RCTSurfaceEnumeratorBlock)block
{
std::shared_lock<butter::shared_mutex> lock(_mutex);
std::shared_lock lock(_mutex);
block([_registry objectEnumerator]);
}

- (void)registerSurface:(RCTFabricSurface *)surface
{
std::unique_lock<butter::shared_mutex> lock(_mutex);
std::unique_lock lock(_mutex);

ReactTag rootTag = surface.rootViewTag.integerValue;
[_registry setObject:surface forKey:(__bridge id)(void *)rootTag];
}

- (void)unregisterSurface:(RCTFabricSurface *)surface
{
std::unique_lock<butter::shared_mutex> lock(_mutex);
std::unique_lock lock(_mutex);

ReactTag rootTag = surface.rootViewTag.integerValue;
[_registry removeObjectForKey:(__bridge id)(void *)rootTag];
}

- (RCTFabricSurface *)surfaceForRootTag:(ReactTag)rootTag
{
std::shared_lock<butter::shared_mutex> lock(_mutex);
std::shared_lock lock(_mutex);

return [_registry objectForKey:(__bridge id)(void *)rootTag];
}
Expand Down
16 changes: 8 additions & 8 deletions ReactAndroid/src/main/jni/react/fabric/Binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jni::local_ref<Binding::jhybriddata> Binding::initHybrid(

// Thread-safe getter
std::shared_ptr<Scheduler> Binding::getScheduler() {
std::shared_lock<butter::shared_mutex> lock(installMutex_);
std::shared_lock lock(installMutex_);
return scheduler_;
}

Expand Down Expand Up @@ -132,7 +132,7 @@ void Binding::startSurface(

{
SystraceSection s2("FabricUIManagerBinding::startSurface::surfaceId::lock");
std::unique_lock<butter::shared_mutex> lock(surfaceHandlerRegistryMutex_);
std::unique_lock lock(surfaceHandlerRegistryMutex_);
SystraceSection s3("FabricUIManagerBinding::startSurface::surfaceId");
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
}
Expand Down Expand Up @@ -202,7 +202,7 @@ void Binding::startSurfaceWithConstraints(
{
SystraceSection s2(
"FabricUIManagerBinding::startSurfaceWithConstraints::surfaceId::lock");
std::unique_lock<butter::shared_mutex> lock(surfaceHandlerRegistryMutex_);
std::unique_lock lock(surfaceHandlerRegistryMutex_);
SystraceSection s3(
"FabricUIManagerBinding::startSurfaceWithConstraints::surfaceId");
surfaceHandlerRegistry_.emplace(surfaceId, std::move(surfaceHandler));
Expand Down Expand Up @@ -246,7 +246,7 @@ void Binding::stopSurface(jint surfaceId) {
}

{
std::unique_lock<butter::shared_mutex> lock(surfaceHandlerRegistryMutex_);
std::unique_lock lock(surfaceHandlerRegistryMutex_);

auto iterator = surfaceHandlerRegistry_.find(surfaceId);

Expand Down Expand Up @@ -338,7 +338,7 @@ void Binding::setConstraints(
isRTL ? LayoutDirection::RightToLeft : LayoutDirection::LeftToRight;

{
std::shared_lock<butter::shared_mutex> lock(surfaceHandlerRegistryMutex_);
std::shared_lock lock(surfaceHandlerRegistryMutex_);

auto iterator = surfaceHandlerRegistry_.find(surfaceId);

Expand Down Expand Up @@ -377,7 +377,7 @@ void Binding::installFabricUIManager(

// Use std::lock and std::adopt_lock to prevent deadlocks by locking mutexes
// at the same time
std::unique_lock<butter::shared_mutex> lock(installMutex_);
std::unique_lock lock(installMutex_);

auto globalJavaUiManager = make_global(javaUIManager);
mountingManager_ =
Expand Down Expand Up @@ -467,7 +467,7 @@ void Binding::uninstallFabricUIManager() {
<< this << ").";
}

std::unique_lock<butter::shared_mutex> lock(installMutex_);
std::unique_lock lock(installMutex_);
animationDriver_ = nullptr;
scheduler_ = nullptr;
mountingManager_ = nullptr;
Expand All @@ -476,7 +476,7 @@ void Binding::uninstallFabricUIManager() {

std::shared_ptr<FabricMountingManager> Binding::verifyMountingManager(
std::string const &hint) {
std::shared_lock<butter::shared_mutex> lock(installMutex_);
std::shared_lock lock(installMutex_);
if (!mountingManager_) {
LOG(ERROR) << hint << " mounting manager disappeared.";
}
Expand Down
6 changes: 3 additions & 3 deletions ReactAndroid/src/main/jni/react/fabric/Binding.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "FabricMountingManager.h"

#include <memory>
#include <mutex>
#include <shared_mutex>

#include <fbjni/fbjni.h>
#include <react/jni/JRuntimeExecutor.h>
Expand Down Expand Up @@ -123,7 +123,7 @@ class Binding : public jni::HybridClass<Binding>,
void uninstallFabricUIManager();

// Private member variables
butter::shared_mutex installMutex_;
std::shared_mutex installMutex_;
std::shared_ptr<FabricMountingManager> mountingManager_;
std::shared_ptr<Scheduler> scheduler_;

Expand All @@ -139,7 +139,7 @@ class Binding : public jni::HybridClass<Binding>,
BackgroundExecutor backgroundExecutor_;

butter::map<SurfaceId, SurfaceHandler> surfaceHandlerRegistry_{};
butter::shared_mutex
std::shared_mutex
surfaceHandlerRegistryMutex_; // Protects `surfaceHandlerRegistry_`.

float pointScaleFactor_ = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ void SurfaceHandlerBinding::setDisplayMode(jint mode) {
}

void SurfaceHandlerBinding::start() {
std::unique_lock<butter::shared_mutex> lock(lifecycleMutex_);
std::unique_lock lock(lifecycleMutex_);

if (surfaceHandler_.getStatus() != SurfaceHandler::Status::Running) {
surfaceHandler_.start();
}
}

void SurfaceHandlerBinding::stop() {
std::unique_lock<butter::shared_mutex> lock(lifecycleMutex_);
std::unique_lock lock(lifecycleMutex_);

if (surfaceHandler_.getStatus() == SurfaceHandler::Status::Running) {
surfaceHandler_.stop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#pragma once

#include <shared_mutex>

#include <fbjni/fbjni.h>
#include <react/jni/ReadableNativeMap.h>
#include <react/renderer/scheduler/SurfaceHandler.h>
Expand Down Expand Up @@ -50,7 +52,7 @@ class SurfaceHandlerBinding : public jni::HybridClass<SurfaceHandlerBinding> {
SurfaceHandler const &getSurfaceHandler();

private:
mutable butter::shared_mutex lifecycleMutex_;
mutable std::shared_mutex lifecycleMutex_;
const SurfaceHandler surfaceHandler_;

jni::alias_ref<SurfaceHandlerBinding::jhybriddata> jhybridobject_;
Expand Down
20 changes: 0 additions & 20 deletions ReactCommon/butter/mutex.h

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace facebook::react {

void ComponentDescriptorProviderRegistry::add(
const ComponentDescriptorProvider &provider) const {
std::unique_lock<butter::shared_mutex> lock(mutex_);
std::unique_lock lock(mutex_);

/*
// TODO: T57583139
Expand Down Expand Up @@ -44,7 +44,7 @@ void ComponentDescriptorProviderRegistry::add(
void ComponentDescriptorProviderRegistry::setComponentDescriptorProviderRequest(
ComponentDescriptorProviderRequest componentDescriptorProviderRequest)
const {
std::shared_lock<butter::shared_mutex> lock(mutex_);
std::shared_lock lock(mutex_);
componentDescriptorProviderRequest_ =
std::move(componentDescriptorProviderRequest);
}
Expand All @@ -54,7 +54,7 @@ void ComponentDescriptorProviderRegistry::request(
ComponentDescriptorProviderRequest componentDescriptorProviderRequest;

{
std::shared_lock<butter::shared_mutex> lock(mutex_);
std::shared_lock lock(mutex_);
componentDescriptorProviderRequest = componentDescriptorProviderRequest_;
}

Expand All @@ -66,7 +66,7 @@ void ComponentDescriptorProviderRegistry::request(
ComponentDescriptorRegistry::Shared
ComponentDescriptorProviderRegistry::createComponentDescriptorRegistry(
ComponentDescriptorParameters const &parameters) const {
std::shared_lock<butter::shared_mutex> lock(mutex_);
std::shared_lock lock(mutex_);

auto registry = std::make_shared<ComponentDescriptorRegistry const>(
parameters, *this, parameters.contextContainer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#pragma once

#include <butter/mutex.h>
#include <shared_mutex>

#include <react/renderer/componentregistry/ComponentDescriptorProvider.h>
#include <react/renderer/componentregistry/ComponentDescriptorRegistry.h>
Expand Down Expand Up @@ -58,7 +58,7 @@ class ComponentDescriptorProviderRegistry final {

void request(ComponentName componentName) const;

mutable butter::shared_mutex mutex_;
mutable std::shared_mutex mutex_;
mutable std::vector<std::weak_ptr<ComponentDescriptorRegistry const>>
componentDescriptorRegistries_;
mutable butter::map<ComponentHandle, ComponentDescriptorProvider const>
Expand Down
Loading

0 comments on commit e665a0f

Please sign in to comment.