De-batch native->js calls and react updates

Summary: @​public

Take a step back and de-batch the bridge calls so we can have better profiling data and a better starting point to work on future optimisations. Also gave a 10~15% win on first render.

Reviewed By: @javache

Differential Revision: D2493674

fb-gh-sync-id: 05165fdd00645bdf43e844bb0c4300a2f63e7038

Examples/UIExplorer/UIExplorerIntegrationTests/UIExplorerIntegrationTests.m

@@ -62,9 +62,11 @@
 RCT_TEST(IntegrationTestHarnessTest)
 RCT_TEST(TimersTest)
 RCT_TEST(AsyncStorageTest)
-RCT_TEST(LayoutEventsTest)
 RCT_TEST(AppEventsTest)
 RCT_TEST(SimpleSnapshotTest)
-RCT_TEST(PromiseTest)
+
+// Disable due to flakiness: #8686784
+//RCT_TEST(LayoutEventsTest)
+//RCT_TEST(PromiseTest)
 
 @end

Libraries/Utilities/MessageQueue.js

@@ -55,7 +55,6 @@ class MessageQueue {
     this._callbackID = 0;
 
     [
-      'processBatch',
       'invokeCallbackAndReturnFlushedQueue',
       'callFunctionReturnFlushedQueue',
       'flushedQueue',
@@ -75,42 +74,30 @@ class MessageQueue {
   /**
    * Public APIs
    */
-  processBatch(batch) {
+  callFunctionReturnFlushedQueue(module, method, args) {
     guard(() => {
-      ReactUpdates.batchedUpdates(() => {
-        batch.forEach((call) => {
-          let method = call.method === 'callFunctionReturnFlushedQueue' ?
-            '__callFunction' : '__invokeCallback';
-          guard(() => this[method].apply(this, call.args));
-        });
-
-        this.__callImmediates();
-      });
-
-      // batchedUpdates might still trigger setImmediates
-      while (JSTimersExecution.immediates.length) {
-        ReactUpdates.batchedUpdates(() => {
-          this.__callImmediates();
-        });
-      }
+      this.__callFunction(module, method, args);
+      this.__callImmediates();
     });
 
-    return this.__flushedQueue();
-  }
-
-  callFunctionReturnFlushedQueue(module, method, args) {
-    guard(() => this.__callFunction(module, method, args));
     return this.flushedQueue();
   }
 
   invokeCallbackAndReturnFlushedQueue(cbID, args) {
-    guard(() => this.__invokeCallback(cbID, args));
+    guard(() => {
+      this.__invokeCallback(cbID, args);
+      this.__callImmediates();
+    });
+
     return this.flushedQueue();
   }
 
   flushedQueue() {
     this.__callImmediates();
-    return this.__flushedQueue();
+
+    let queue = this._queue;
+    this._queue = [[],[],[]];
+    return queue[0].length ? queue : null;
   }
 
   /**
@@ -123,11 +110,6 @@ class MessageQueue {
     BridgeProfiling.profileEnd();
   }
 
-  __flushedQueue() {
-    let queue = this._queue;
-    this._queue = [[],[],[]];
-    return queue[0].length ? queue : null;
-  }
   __nativeCall(module, method, params, onFail, onSucc) {
     if (onFail || onSucc) {
       // eventually delete old debug info

Libraries/Utilities/__tests__/MessageQueue-test.js

@@ -95,29 +95,6 @@ describe('MessageQueue', () => {
     queue.__invokeCallback(1);
     expect(() => queue.__invokeCallback(0)).toThrow();
   });
-
-  describe('processBatch', () => {
-
-    it('should call __invokeCallback for invokeCallbackAndReturnFlushedQueue', () => {
-      queue.__invokeCallback = jasmine.createSpy();
-      queue.processBatch([{
-        method: 'invokeCallbackAndReturnFlushedQueue',
-        args: [],
-      }]);
-      expect(queue.__invokeCallback.callCount).toEqual(1);
-    });
-
-    it('should call __callFunction for callFunctionReturnFlushedQueue', () => {
-      queue.__callFunction = jasmine.createSpy();
-      queue.processBatch([{
-        method: 'callFunctionReturnFlushedQueue',
-        args: [],
-      }]);
-      expect(queue.__callFunction.callCount).toEqual(1);
-    });
-
-  });
-
 });
 
 var remoteModulesConfig = {

React/Base/RCTBatchedBridge.m

@@ -69,8 +69,6 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void);
   CADisplayLink *_mainDisplayLink;
   CADisplayLink *_jsDisplayLink;
   NSMutableSet *_frameUpdateObservers;
-  NSMutableArray *_scheduledCalls;
-  RCTSparseArray *_scheduledCallbacks;
 }
 
 - (instancetype)initWithParentBridge:(RCTBridge *)bridge
@@ -91,8 +89,6 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void);
     _loading = YES;
     _moduleDataByID = [NSMutableArray new];
     _frameUpdateObservers = [NSMutableSet new];
-    _scheduledCalls = [NSMutableArray new];
-    _scheduledCallbacks = [RCTSparseArray new];
     _jsDisplayLink = [CADisplayLink displayLinkWithTarget:self selector:@selector(_jsThreadUpdate:)];
 
     if (RCT_DEV) {
@@ -334,14 +330,12 @@ RCT_EXTERN NSArray *RCTGetModuleClasses(void);
 {
   RCTAssertJSThread();
 
-  BOOL pauseDisplayLink = ![_scheduledCallbacks count] && ![_scheduledCalls count];
-  if (pauseDisplayLink) {
-    for (RCTModuleData *moduleData in _frameUpdateObservers) {
-      id<RCTFrameUpdateObserver> observer = (id<RCTFrameUpdateObserver>)moduleData.instance;
-      if (!observer.paused) {
-        pauseDisplayLink = NO;
-        break;
-      }
+  BOOL pauseDisplayLink = YES;
+  for (RCTModuleData *moduleData in _frameUpdateObservers) {
+    id<RCTFrameUpdateObserver> observer = (id<RCTFrameUpdateObserver>)moduleData.instance;
+    if (!observer.paused) {
+      pauseDisplayLink = NO;
+      break;
     }
   }
   _jsDisplayLink.paused = pauseDisplayLink;
@@ -631,28 +625,9 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR
     RCTProfileBeginEvent(0, @"enqueue_call", nil);
 
     RCTBatchedBridge *strongSelf = weakSelf;
-    if (!strongSelf.isValid || !strongSelf->_scheduledCallbacks || !strongSelf->_scheduledCalls) {
-      return;
-    }
 
+    [strongSelf _actuallyInvokeAndProcessModule:module method:method arguments:args];
 
-    RCT_IF_DEV(NSNumber *callID = _RCTProfileBeginFlowEvent();)
-    id call = @{
-      @"js_args": @{
-        @"module": module,
-        @"method": method,
-        @"args": args,
-      },
-      RCT_IF_DEV(@"call_id": callID,)
-    };
-    if ([method isEqualToString:@"invokeCallbackAndReturnFlushedQueue"]) {
-      strongSelf->_scheduledCallbacks[args[0]] = call;
-    } else {
-      [strongSelf->_scheduledCalls addObject:call];
-    }
-    [strongSelf updateJSDisplayLinkState];
-
-    RCTProfileEndEvent(0, @"objc_call", call);
   }];
 }
 
@@ -848,24 +823,10 @@ RCT_NOT_IMPLEMENTED(- (instancetype)initWithBundleURL:(__unused NSURL *)bundleUR
     }
   }
 
-  NSArray *calls = [_scheduledCallbacks.allObjects arrayByAddingObjectsFromArray:_scheduledCalls];
+  [self updateJSDisplayLinkState];
 
-  RCT_IF_DEV(
-    RCTProfileImmediateEvent(0, @"JS Thread Tick", 'g');
 
-    for (NSDictionary *call in calls) {
-      _RCTProfileEndFlowEvent(call[@"call_id"]);
-    }
-  )
-
-  if (calls.count > 0) {
-    _scheduledCalls = [NSMutableArray new];
-    _scheduledCallbacks = [RCTSparseArray new];
-    [self _actuallyInvokeAndProcessModule:@"BatchedBridge"
-                                   method:@"processBatch"
-                                arguments:@[[calls valueForKey:@"js_args"]]];
-    [self updateJSDisplayLinkState];
-  }
+  RCTProfileImmediateEvent(0, @"JS Thread Tick", 'g');
 
   RCTProfileEndEvent(0, @"objc_call", nil);
 

React/Executors/RCTContextExecutor.m

@@ -469,7 +469,7 @@ static void RCTInstallJSCProfiler(RCTBridge *bridge, JSContextRef context)
       }
     }
 
-    if (!resultJSRef) {
+    if (errorJSRef) {
       onComplete(nil, RCTNSErrorFromJSError(contextJSRef, errorJSRef));
       return;
     }