Skip to content

Commit

Permalink
[react-packager] Switch from Q to Bluebird as promises library
Browse files Browse the repository at this point in the history
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
  • Loading branch information
pilwon committed Apr 1, 2015
1 parent 6638713 commit 87599bf
Showing 14 changed files with 77 additions and 78 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -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",
5 changes: 5 additions & 0 deletions packager/react-packager/__mocks__/bluebird.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

jest.autoMockOff();
module.exports = require.requireActual('bluebird');
jest.autoMockOn();
Original file line number Diff line number Diff line change
@@ -10,7 +10,6 @@

jest
.dontMock('../index')
.dontMock('q')
.dontMock('path')
.dontMock('absolute-path')
.dontMock('../docblock')
Original file line number Diff line number Diff line change
@@ -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) {
Original file line number Diff line number Diff line change
@@ -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 })
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@
*/
'use strict';

var Promise = require('q').Promise;
var Promise = require('bluebird');
var ModuleDescriptor = require('../ModuleDescriptor');

var mdeps = require('module-deps');
10 changes: 4 additions & 6 deletions packager/react-packager/src/FileWatcher/index.js
Original file line number Diff line number Diff line change
@@ -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;
};
10 changes: 4 additions & 6 deletions packager/react-packager/src/JSTransformer/Cache.js
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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();
13 changes: 7 additions & 6 deletions packager/react-packager/src/JSTransformer/index.js
Original file line number Diff line number Diff line change
@@ -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(
Loading

0 comments on commit 87599bf

Please sign in to comment.