From f5c246d50e46f57edc52ec06c5fba9e5ea9f8eb7 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Wed, 9 Sep 2020 09:28:37 -0700 Subject: [PATCH] Use reader/writer lock for TurboModule cache access Summary: Right now, when two threads require two NativeModules, both threads fight for the same `std::mutex`. Why? Because NativeModule require can read from/write to the shared `std::unordered_map`. **A Few Thoughts:** - All threads should be able to read from the TurboModule cache concurrently, without issue. Only writes into the cache require exclusive access. *With our current setup, both reads and writes acquire exclusive access.* - During the lifetime of an application, there will only be tens of NativeModule create events (i.e: writes into the TurboModule cache). However, there may be hundreds, if not thousands of TurboModule cache lookups. *We don't need to serialize those hundreds/thousands of TurboModule cache reads.* This is a potential optimization opportunity for the TurboModule infra. ## Changes This diff introduces a `std::shared_mutex` inside `RCTTurboModuleManager`. We use either the regular `std::mutex` or the `std::shared_mutex`, depending on whether `RCTTurboModuleSharedMutexInitEnabled()` is `YES`. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D23413118 fbshipit-source-id: 0880413c691b141db98a2715648f0c3e05983307 --- React/Base/RCTBridge.h | 4 ++ React/Base/RCTBridge.m | 11 ++++ .../platform/ios/RCTTurboModuleManager.mm | 60 +++++++++++++++---- 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index c83c40fa9d0d9e..5981e1faa0ae0a 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -157,6 +157,10 @@ RCT_EXTERN void RCTEnableTurboModule(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleEagerInitEnabled(void); RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled); +// Turn on TurboModule shared mutex initialization +RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void); +RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled); + /** * Async batched bridge used to communicate with the JavaScript application. */ diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index a30a273df32f26..4c5bc035c03165 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -125,6 +125,17 @@ void RCTEnableTurboModuleEagerInit(BOOL enabled) turboModuleEagerInitEnabled = enabled; } +static BOOL turboModuleSharedMutexInitEnabled = NO; +BOOL RCTTurboModuleSharedMutexInitEnabled(void) +{ + return turboModuleSharedMutexInitEnabled; +} + +void RCTEnableTurboModuleSharedMutexInit(BOOL enabled) +{ + turboModuleSharedMutexInitEnabled = enabled; +} + @interface RCTBridge () @end diff --git a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModuleManager.mm b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModuleManager.mm index 0d89875a25bb56..147c076fc07670 100644 --- a/ReactCommon/turbomodule/core/platform/ios/RCTTurboModuleManager.mm +++ b/ReactCommon/turbomodule/core/platform/ios/RCTTurboModuleManager.mm @@ -11,6 +11,7 @@ #import #import #import +#import #import @@ -168,6 +169,7 @@ @implementation RCTTurboModuleManager { std::mutex _turboModuleManagerDelegateMutex; // Enforce synchronous access to _invalidating and _turboModuleHolders + std::shared_timed_mutex _turboModuleHoldersSharedMutex; std::mutex _turboModuleHoldersMutex; std::atomic _invalidating; } @@ -314,6 +316,33 @@ - (void)notifyAboutTurboModuleSetup:(const char *)name return turboModule; } +- (TurboModuleHolder *)_getOrCreateTurboModuleHolder:(const char *)moduleName +{ + if (RCTTurboModuleSharedMutexInitEnabled()) { + { + std::shared_lock guard(_turboModuleHoldersSharedMutex); + if (_invalidating) { + return nullptr; + } + + auto it = _turboModuleHolders.find(moduleName); + if (it != _turboModuleHolders.end()) { + return &it->second; + } + } + + std::unique_lock guard(_turboModuleHoldersSharedMutex); + return &_turboModuleHolders[moduleName]; + } + + std::lock_guard guard(_turboModuleHoldersMutex); + if (_invalidating) { + return nullptr; + } + + return &_turboModuleHolders[moduleName]; +} + /** * Given a name for a TurboModule, return an ObjC object which is the instance * of that TurboModule ObjC class. If no TurboModule exist with the provided name, @@ -324,15 +353,10 @@ - (void)notifyAboutTurboModuleSetup:(const char *)name */ - (id)provideRCTTurboModule:(const char *)moduleName { - TurboModuleHolder *moduleHolder; + TurboModuleHolder *moduleHolder = [self _getOrCreateTurboModuleHolder:moduleName]; - { - std::lock_guard guard(_turboModuleHoldersMutex); - if (_invalidating) { - return nil; - } - - moduleHolder = &_turboModuleHolders[moduleName]; + if (!moduleHolder) { + return nil; } TurboModulePerfLogger::moduleCreateStart(moduleName, moduleHolder->getModuleId()); @@ -719,6 +743,11 @@ - (id)moduleForName:(const char *)moduleName warnOnLookupFailure:(BOOL)warnOnLoo - (BOOL)moduleIsInitialized:(const char *)moduleName { + if (RCTTurboModuleSharedMutexInitEnabled()) { + std::shared_lock guard(_turboModuleHoldersSharedMutex); + return _turboModuleHolders.find(moduleName) != _turboModuleHolders.end(); + } + std::unique_lock guard(_turboModuleHoldersMutex); return _turboModuleHolders.find(moduleName) != _turboModuleHolders.end(); } @@ -750,10 +779,14 @@ - (void)bridgeWillInvalidateModules:(NSNotification *)notification return; } - std::lock_guard guard(_turboModuleHoldersMutex); - // This should halt all insertions into _turboModuleHolders - _invalidating = true; + if (RCTTurboModuleSharedMutexInitEnabled()) { + std::unique_lock guard(_turboModuleHoldersSharedMutex); + _invalidating = true; + } else { + std::lock_guard guard(_turboModuleHoldersMutex); + _invalidating = true; + } } - (void)bridgeDidInvalidateModules:(NSNotification *)notification @@ -807,7 +840,10 @@ - (void)bridgeDidInvalidateModules:(NSNotification *)notification - (void)invalidate { - { + if (RCTTurboModuleSharedMutexInitEnabled()) { + std::unique_lock guard(_turboModuleHoldersSharedMutex); + _invalidating = true; + } else { std::lock_guard guard(_turboModuleHoldersMutex); _invalidating = true; }