[react-packager] Switch from Q to Bluebird as promises library

Summary: This PR improves performance of `react-packager` by switching the promises library from the [Q](https://github.com/kriskowal/q) to [Bluebird](https://github.com/petkaantonov/bluebird). [Here is the test result](facebook#361 (comment)) showing a noticeable difference. (2x speed improvement) Please refer to [this issue](facebook#361) for more details. Closes facebook#516 Github Author: Pilwon Huh <pilwon@gmail.com> Test Plan: ./runJestTests start app and click around
vjeux · Apr 12, 2015 · a8f8fa1 · a8f8fa1
1 parent dcc228a
commit a8f8fa1
Show file tree

Hide file tree

Showing 14 changed files with 77 additions and 78 deletions.
diff --git a/package.json b/package.json
@@ -53,13 +53,13 @@
     "joi": "~5.1.0",
     "module-deps": "3.5.6",
     "optimist": "0.6.1",
-    "q": "1.0.1",
     "sane": "1.0.1",
     "uglify-js": "~2.4.16",
     "underscore": "1.7.0",
     "worker-farm": "1.1.0",
     "yargs": "1.3.2",
-    "ws": "0.4.31"
+    "ws": "0.4.31",
+    "bluebird": "^2.9.21"
   },
   "devDependencies": {
     "jest-cli": "0.2.1",

diff --git a/packager/react-packager/__mocks__/bluebird.js b/packager/react-packager/__mocks__/bluebird.js
@@ -0,0 +1,5 @@
+'use strict';
+
+jest.autoMockOff();
+module.exports = require.requireActual('bluebird');
+jest.autoMockOn();
diff --git a/...t-packager/src/DependencyResolver/haste/DependencyGraph/__tests__/DependencyGraph-test.js b/...t-packager/src/DependencyResolver/haste/DependencyGraph/__tests__/DependencyGraph-test.js
@@ -10,7 +10,6 @@
 
 jest
   .dontMock('../index')
-  .dontMock('q')
   .dontMock('path')
   .dontMock('absolute-path')
   .dontMock('../docblock')

diff --git a/packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js b/packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js
@@ -9,7 +9,7 @@
 'use strict';
 
 var ModuleDescriptor = require('../../ModuleDescriptor');
-var q = require('q');
+var Promise = require('bluebird');
 var fs = require('fs');
 var docblock = require('./docblock');
 var requirePattern = require('../requirePattern');
@@ -19,10 +19,10 @@ var debug = require('debug')('DependecyGraph');
 var util = require('util');
 var declareOpts = require('../../../lib/declareOpts');
 
-var readFile = q.nfbind(fs.readFile);
-var readDir = q.nfbind(fs.readdir);
-var lstat = q.nfbind(fs.lstat);
-var realpath = q.nfbind(fs.realpath);
+var readFile = Promise.promisify(fs.readFile);
+var readDir = Promise.promisify(fs.readdir);
+var lstat = Promise.promisify(fs.lstat);
+var realpath = Promise.promisify(fs.realpath);
 
 var validateOpts = declareOpts({
   roots: {
@@ -73,7 +73,7 @@ DependecyGraph.prototype.load = function() {
     return this._loading;
   }
 
-  this._loading = q.all([
+  this._loading = Promise.all([
     this._search(),
     this._buildAssetMap(),
   ]);
@@ -263,7 +263,7 @@ DependecyGraph.prototype._search = function() {
   var dir = this._queue.shift();
 
   if (dir == null) {
-    return q.Promise.resolve(this._graph);
+    return Promise.resolve(this._graph);
   }
 
   // Steps:
@@ -292,10 +292,10 @@ DependecyGraph.prototype._search = function() {
 
       var processing = self._findAndProcessPackage(files, dir)
         .then(function() {
-          return q.all(modulePaths.map(self._processModule.bind(self)));
+          return Promise.all(modulePaths.map(self._processModule.bind(self)));
         });
 
-      return q.all([
+      return Promise.all([
         processing,
         self._search()
       ]);
@@ -324,7 +324,7 @@ DependecyGraph.prototype._findAndProcessPackage = function(files, root) {
   if (packagePath != null) {
     return this._processPackage(packagePath);
   } else {
-    return q();
+    return Promise.resolve();
   }
 };
 
@@ -338,15 +338,15 @@ DependecyGraph.prototype._processPackage = function(packagePath) {
         packageJson = JSON.parse(content);
       } catch (e) {
         debug('WARNING: malformed package.json: ', packagePath);
-        return q();
+        return Promise.resolve();
       }
 
       if (packageJson.name == null) {
         debug(
           'WARNING: package.json `%s` is missing a name field',
           packagePath
         );
-        return q();
+        return Promise.resolve();
       }
 
       packageJson._root = packageRoot;
@@ -556,7 +556,7 @@ DependecyGraph.prototype._getAbsolutePath = function(filePath) {
 
 DependecyGraph.prototype._buildAssetMap = function() {
   if (this._assetRoots == null || this._assetRoots.length === 0) {
-    return q();
+    return Promise.resolve();
   }
 
   this._assetMap = Object.create(null);
@@ -640,13 +640,13 @@ function withExtJs(file) {
 
 function handleBrokenLink(e) {
   debug('WARNING: error stating, possibly broken symlink', e.message);
-  return q();
+  return Promise.resolve();
 }
 
 function readAndStatDir(dir) {
   return readDir(dir)
     .then(function(files){
-      return q.all(files.map(function(filePath) {
+      return Promise.all(files.map(function(filePath) {
         return realpath(path.join(dir, filePath)).catch(handleBrokenLink);
       }));
     }).then(function(files) {
@@ -660,7 +660,7 @@ function readAndStatDir(dir) {
 
       return [
         files,
-        q.all(stats),
+        Promise.all(stats),
       ];
     });
 }
@@ -676,7 +676,7 @@ function buildAssetMap(roots, processAsset) {
     var root = queue.shift();
 
     if (root == null) {
-      return q();
+      return Promise.resolve();
     }
 
     return readAndStatDir(root).spread(function(files, stats) {

diff --git a/...ger/react-packager/src/DependencyResolver/haste/__tests__/HasteDependencyResolver-test.js b/...ger/react-packager/src/DependencyResolver/haste/__tests__/HasteDependencyResolver-test.js
@@ -13,7 +13,7 @@ jest.dontMock('../')
     .dontMock('../requirePattern')
     .setMock('../../ModuleDescriptor', function(data) {return data;});
 
-var q = require('q');
+var Promise = require('bluebird');
 
 describe('HasteDependencyResolver', function() {
   var HasteDependencyResolver;
@@ -41,7 +41,7 @@ describe('HasteDependencyResolver', function() {
         return deps;
       });
       depGraph.load.mockImpl(function() {
-        return q();
+        return Promise.resolve();
       });
 
       return depResolver.getDependencies('/root/index.js', { dev: false })
@@ -101,7 +101,7 @@ describe('HasteDependencyResolver', function() {
         return deps;
       });
       depGraph.load.mockImpl(function() {
-        return q();
+        return Promise.resolve();
       });
 
       return depResolver.getDependencies('/root/index.js', { dev: true })
@@ -162,7 +162,7 @@ describe('HasteDependencyResolver', function() {
         return deps;
       });
       depGraph.load.mockImpl(function() {
-        return q();
+        return Promise.resolve();
       });
 
       return depResolver.getDependencies('/root/index.js', { dev: false })

diff --git a/packager/react-packager/src/DependencyResolver/node/index.js b/packager/react-packager/src/DependencyResolver/node/index.js
@@ -8,7 +8,7 @@
  */
 'use strict';
 
-var Promise = require('q').Promise;
+var Promise = require('bluebird');
 var ModuleDescriptor = require('../ModuleDescriptor');
 
 var mdeps = require('module-deps');

diff --git a/packager/react-packager/src/FileWatcher/index.js b/packager/react-packager/src/FileWatcher/index.js
@@ -10,12 +10,10 @@
 
 var EventEmitter  = require('events').EventEmitter;
 var sane = require('sane');
-var q = require('q');
+var Promise = require('bluebird');
 var util = require('util');
 var exec = require('child_process').exec;
 
-var Promise = q.Promise;
-
 var detectingWatcherClass = new Promise(function(resolve) {
   exec('which watchman', function(err, out) {
     if (err || out.length === 0) {
@@ -41,7 +39,7 @@ function FileWatcher(rootConfigs) {
 
   fileWatcher = this;
 
-  this._loading = q.all(
+  this._loading = Promise.all(
     rootConfigs.map(createWatcher)
   ).then(function(watchers) {
     watchers.forEach(function(watcher) {
@@ -59,7 +57,7 @@ util.inherits(FileWatcher, EventEmitter);
 FileWatcher.prototype.end = function() {
   return this._loading.then(function(watchers) {
     watchers.forEach(function(watcher) {
-      return q.ninvoke(watcher, 'close');
+      return Promise.promisify(watcher.close, watcher)();
     });
   });
 };
@@ -88,7 +86,7 @@ function createWatcher(rootConfig) {
 FileWatcher.createDummyWatcher = function() {
   var ev = new EventEmitter();
   ev.end = function() {
-    return q();
+    return Promise.resolve();
   };
   return ev;
 };
diff --git a/packager/react-packager/src/JSTransformer/Cache.js b/packager/react-packager/src/JSTransformer/Cache.js
@@ -14,12 +14,10 @@ var declareOpts = require('../lib/declareOpts');
 var fs = require('fs');
 var isAbsolutePath = require('absolute-path');
 var path = require('path');
-var q = require('q');
+var Promise = require('bluebird');
 var tmpdir = require('os').tmpDir();
 var version = require('../../../../package.json').version;
 
-var Promise = q.Promise;
-
 var validateOpts = declareOpts({
   resetCache: {
     type: 'boolean',
@@ -74,7 +72,7 @@ Cache.prototype._set = function(filepath, loaderPromise) {
   this._data[filepath] = loaderPromise.then(function(data) {
     return [
       data,
-      q.nfbind(fs.stat)(filepath)
+      Promise.promisify(fs.stat)(filepath)
     ];
   }).spread(function(data, stat) {
     this._persistEventually();
@@ -105,13 +103,13 @@ Cache.prototype._persistCache = function() {
   var data = this._data;
   var cacheFilepath = this._cacheFilePath;
 
-  this._persisting = q.all(_.values(data))
+  this._persisting = Promise.all(_.values(data))
     .then(function(values) {
       var json = Object.create(null);
       Object.keys(data).forEach(function(key, i) {
         json[key] = values[i];
       });
-      return q.nfbind(fs.writeFile)(cacheFilepath, JSON.stringify(json));
+      return Promise.promisify(fs.writeFile)(cacheFilepath, JSON.stringify(json));
     })
     .then(function() {
       this._persisting = null;

diff --git a/packager/react-packager/src/JSTransformer/__tests__/Cache-test.js b/packager/react-packager/src/JSTransformer/__tests__/Cache-test.js
@@ -15,7 +15,7 @@ jest
   .dontMock('crypto')
   .dontMock('../Cache');
 
-var q = require('q');
+var Promise = require('bluebird');
 
 describe('JSTransformer Cache', function() {
   var Cache;
@@ -32,7 +32,7 @@ describe('JSTransformer Cache', function() {
     it('calls loader callback for uncached file', function() {
       var cache = new Cache({projectRoots: ['/rootDir']});
       var loaderCb = jest.genMockFn().mockImpl(function() {
-        return q();
+        return Promise.resolve();
       });
       cache.get('/rootDir/someFile', loaderCb);
       expect(loaderCb).toBeCalledWith('/rootDir/someFile');
@@ -48,7 +48,7 @@ describe('JSTransformer Cache', function() {
       });
       var cache = new Cache({projectRoots: ['/rootDir']});
       var loaderCb = jest.genMockFn().mockImpl(function() {
-        return q('lol');
+        return Promise.resolve('lol');
       });
       return cache.get('/rootDir/someFile', loaderCb).then(function(value) {
         expect(value).toBe('lol');
@@ -65,7 +65,7 @@ describe('JSTransformer Cache', function() {
       });
       var cache = new Cache({projectRoots: ['/rootDir']});
       var loaderCb = jest.genMockFn().mockImpl(function() {
-        return q('lol');
+        return Promise.resolve('lol');
       });
       return cache.get('/rootDir/someFile', loaderCb).then(function() {
         var shouldNotBeCalled = jest.genMockFn();
@@ -152,7 +152,7 @@ describe('JSTransformer Cache', function() {
 
       var cache = new Cache({projectRoots: ['/rootDir']});
       var loaderCb = jest.genMockFn().mockImpl(function() {
-        return q('new value');
+        return Promise.resolve('new value');
       });
 
       return cache.get('/rootDir/someFile', loaderCb).then(function(value) {
@@ -193,13 +193,13 @@ describe('JSTransformer Cache', function() {
 
       var cache = new Cache({projectRoots: ['/rootDir']});
       cache.get('/rootDir/bar', function() {
-        return q('bar value');
+        return Promise.resolve('bar value');
       });
       cache.get('/rootDir/foo', function() {
-        return q('foo value');
+        return Promise.resolve('foo value');
       });
       cache.get('/rootDir/baz', function() {
-        return q('baz value');
+        return Promise.resolve('baz value');
       });
 
       jest.runAllTicks();

diff --git a/packager/react-packager/src/JSTransformer/index.js b/packager/react-packager/src/JSTransformer/index.js
@@ -9,14 +9,13 @@
 'use strict';
 
 var fs = require('fs');
-var q = require('q');
+var Promise = require('bluebird');
 var Cache = require('./Cache');
-var _ = require('underscore');
 var workerFarm = require('worker-farm');
 var declareOpts = require('../lib/declareOpts');
 var util = require('util');
 
-var readFile = q.nfbind(fs.readFile);
+var readFile = Promise.promisify(fs.readFile);
 
 module.exports = Transformer;
 Transformer.TransformError = TransformError;
@@ -63,12 +62,14 @@ function Transformer(options) {
     });
 
   if (options.transformModulePath == null) {
-    this._failedToStart = q.Promise.reject(new Error('No transfrom module'));
+    this._failedToStart = Promise.reject(new Error('No transfrom module'));
   } else {
     this._workers = workerFarm(
       {autoStart: true, maxConcurrentCallsPerWorker: 1},
       options.transformModulePath
     );
+
+    this._transform = Promise.promisify(this._workers);
   }
 }
 
@@ -86,13 +87,13 @@ Transformer.prototype.loadFileAndTransform = function(filePath) {
     return this._failedToStart;
   }
 
-  var workers = this._workers;
+  var transform = this._transform;
   return this._cache.get(filePath, function() {
     return readFile(filePath)
       .then(function(buffer) {
         var sourceCode = buffer.toString();
 
-        return q.nfbind(workers)({
+        return transform({
           sourceCode: sourceCode,
           filename: filePath,
         }).then(