Skip to content

Commit

Permalink
Move setBridge: off main thread
Browse files Browse the repository at this point in the history
Summary:
Previously, if a module implemented `setBridge:` we assumed that it needs to be initialised on the main thread. This assumption was not really warranted however, and it was a barrier to deferring module initialization.

This diff tweaks the rules so that only modules that override `init` or `constantsToExport**` are assumed to require main thread initialization, and others can be created lazily when they are first used.

WARNING: this will be a breaking change to any 3rd party modules that are assuming `setBridge:` is called on the main thread. Those modules should be rewritten to move any code that requires the main thread into `init` or `constantsToExport` instead.

`**` We will also be examining whether `constantsToExport` can be done lazily, but for now any module that uses it will still be created eagerly when the bridge starts up.

Reviewed By: javache

Differential Revision: D3240682

fb-gh-sync-id: 48f309e3158bbccb52141032baf70def3e609371
fbshipit-source-id: 48f309e3158bbccb52141032baf70def3e609371
  • Loading branch information
nicklockwood authored and Facebook Github Bot 4 committed May 3, 2016
1 parent 55c8158 commit 34ec6a9
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 28 deletions.
12 changes: 9 additions & 3 deletions Examples/UIExplorer/UIExplorerUnitTests/RCTModuleInitTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,16 @@ - (void)testCustomInitModuleInitializedAtBridgeStartup

- (void)testCustomSetBridgeModuleInitializedAtBridgeStartup
{
RUN_RUNLOOP_WHILE(!_customSetBridgeModuleNotificationSent);
XCTAssertFalse(_customSetBridgeModuleNotificationSent);

__block RCTTestCustomSetBridgeModule *module;
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
module = [_bridge moduleForClass:[RCTTestCustomSetBridgeModule class]];
});

RUN_RUNLOOP_WHILE(!module);
XCTAssertTrue(_customSetBridgeModuleNotificationSent);
RCTTestCustomSetBridgeModule *module = [_bridge moduleForClass:[RCTTestCustomSetBridgeModule class]];
XCTAssertTrue(module.setBridgeOnMainThread);
XCTAssertFalse(module.setBridgeOnMainThread);
XCTAssertEqual(module.bridge, _bridge.batchedBridge);
XCTAssertNotNil(module.methodQueue);
}
Expand Down
12 changes: 4 additions & 8 deletions React/Base/RCTModuleData.m
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,15 @@ - (void)setUp
_instanceLock = [NSLock new];

static IMP objectInitMethod;
static SEL setBridgeSelector;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
objectInitMethod = [NSObject instanceMethodForSelector:@selector(init)];
setBridgeSelector = NSSelectorFromString(@"setBridge:");
});

// If a module overrides `init`, `setBridge:` then we must assume that it
// expects for both of those methods to be called on the main thread, because
// they may need to access UIKit.
_requiresMainThreadSetup =
[_moduleClass instancesRespondToSelector:setBridgeSelector] ||
(!_instance && [_moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod);
// If a module overrides `init` then we must assume that it expects to be
// initialized on the main thread, because it may need to access UIKit.
_requiresMainThreadSetup = !_instance &&
[_moduleClass instanceMethodForSelector:@selector(init)] != objectInitMethod;

// If a module overrides `constantsToExport` then we must assume that it
// must be called on the main thread, because it may need to access UIKit.
Expand Down
4 changes: 4 additions & 0 deletions React/Base/RCTUtils.m
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,10 @@ CGFloat RCTScreenScale()

CGSize RCTScreenSize()
{
// FIXME: this caches the bounds at app start, whatever those were, and then
// doesn't update when the device is rotated. We need to find another thread-
// safe way to get the screen size.

static CGSize size;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
Expand Down
13 changes: 10 additions & 3 deletions React/Modules/RCTAppState.m
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,20 @@ @implementation RCTAppState

#pragma mark - Lifecycle

- (instancetype)init
{
if ((self = [super init])) {

// Needs to be called on the main thread, as it accesses UIApplication
_lastKnownState = RCTCurrentAppBackgroundState();
}
return self;
}

- (void)setBridge:(RCTBridge *)bridge
{
_bridge = bridge;

// Is this thread-safe?
_lastKnownState = RCTCurrentAppBackgroundState();

for (NSString *name in @[UIApplicationDidBecomeActiveNotification,
UIApplicationDidEnterBackgroundNotification,
UIApplicationDidFinishLaunchingNotification,
Expand Down
7 changes: 7 additions & 0 deletions React/Modules/RCTDevLoadingView.m
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,13 @@ - (void)dealloc
[[NSNotificationCenter defaultCenter] removeObserver:self];
}

- (instancetype)init
{
// We're only overriding this to ensure the module gets created at startup
// TODO (D3175632): Remove once we have more declarative control over module setup.
return [super init];
}

- (void)setBridge:(RCTBridge *)bridge
{
_bridge = bridge;
Expand Down
2 changes: 1 addition & 1 deletion React/Modules/RCTUIManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ - (void)_amendPendingUIBlocksWithStylePropagationUpdateForShadowView:(RCTShadowV

- (void)_removeChildren:(NSArray<id<RCTComponent>> *)children
fromContainer:(id<RCTComponent>)container
permanent: (BOOL)permanent
permanent:(BOOL)permanent
{
RCTLayoutAnimation *layoutAnimation = _layoutAnimation;
RCTAnimation *deleteAnimation = layoutAnimation.deleteAnimation;
Expand Down
31 changes: 19 additions & 12 deletions React/Profiler/RCTPerfMonitor.m
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,25 @@ @implementation RCTPerfMonitor {

RCT_EXPORT_MODULE()

- (instancetype)init
{
// We're only overriding this to ensure the module gets created at startup
// TODO (D3175632): Remove once we have more declarative control over module setup.
return [super init];
}

- (dispatch_queue_t)methodQueue
{
return dispatch_get_main_queue();
}

- (void)setBridge:(RCTBridge *)bridge
{
_bridge = bridge;

[_bridge.devMenu addItem:self.devMenuItem];
}

- (void)invalidate
{
[self hide];
Expand Down Expand Up @@ -272,18 +291,6 @@ - (UITableView *)metrics
return _metrics;
}

- (dispatch_queue_t)methodQueue
{
return dispatch_get_main_queue();
}

- (void)setBridge:(RCTBridge *)bridge
{
_bridge = bridge;

[_bridge.devMenu addItem:self.devMenuItem];
}

- (void)show
{
if (_container) {
Expand Down
2 changes: 1 addition & 1 deletion React/Views/RCTViewManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ typedef void (^RCTViewManagerUIBlock)(RCTUIManager *uiManager, NSDictionary<NSNu
* allowing the manager (or the views that it manages) to manipulate the view
* hierarchy and send events back to the JS context.
*/
@property (nonatomic, weak, readonly) RCTBridge *bridge;
@property (nonatomic, weak) RCTBridge *bridge;

/**
* This method instantiates a native view to be managed by the module. Override
Expand Down

0 comments on commit 34ec6a9

Please sign in to comment.