Skip to content

Commit a8f8fa1

Browse files
pilwonoss sync
authored andcommitted
[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
1 parent dcc228a commit a8f8fa1

File tree

14 files changed

+77
-78
lines changed

14 files changed

+77
-78
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@
5353
"joi": "~5.1.0",
5454
"module-deps": "3.5.6",
5555
"optimist": "0.6.1",
56-
"q": "1.0.1",
5756
"sane": "1.0.1",
5857
"uglify-js": "~2.4.16",
5958
"underscore": "1.7.0",
6059
"worker-farm": "1.1.0",
6160
"yargs": "1.3.2",
62-
"ws": "0.4.31"
61+
"ws": "0.4.31",
62+
"bluebird": "^2.9.21"
6363
},
6464
"devDependencies": {
6565
"jest-cli": "0.2.1",
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
'use strict';
2+
3+
jest.autoMockOff();
4+
module.exports = require.requireActual('bluebird');
5+
jest.autoMockOn();

packager/react-packager/src/DependencyResolver/haste/DependencyGraph/__tests__/DependencyGraph-test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
jest
1212
.dontMock('../index')
13-
.dontMock('q')
1413
.dontMock('path')
1514
.dontMock('absolute-path')
1615
.dontMock('../docblock')

packager/react-packager/src/DependencyResolver/haste/DependencyGraph/index.js

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
'use strict';
1010

1111
var ModuleDescriptor = require('../../ModuleDescriptor');
12-
var q = require('q');
12+
var Promise = require('bluebird');
1313
var fs = require('fs');
1414
var docblock = require('./docblock');
1515
var requirePattern = require('../requirePattern');
@@ -19,10 +19,10 @@ var debug = require('debug')('DependecyGraph');
1919
var util = require('util');
2020
var declareOpts = require('../../../lib/declareOpts');
2121

22-
var readFile = q.nfbind(fs.readFile);
23-
var readDir = q.nfbind(fs.readdir);
24-
var lstat = q.nfbind(fs.lstat);
25-
var realpath = q.nfbind(fs.realpath);
22+
var readFile = Promise.promisify(fs.readFile);
23+
var readDir = Promise.promisify(fs.readdir);
24+
var lstat = Promise.promisify(fs.lstat);
25+
var realpath = Promise.promisify(fs.realpath);
2626

2727
var validateOpts = declareOpts({
2828
roots: {
@@ -73,7 +73,7 @@ DependecyGraph.prototype.load = function() {
7373
return this._loading;
7474
}
7575

76-
this._loading = q.all([
76+
this._loading = Promise.all([
7777
this._search(),
7878
this._buildAssetMap(),
7979
]);
@@ -263,7 +263,7 @@ DependecyGraph.prototype._search = function() {
263263
var dir = this._queue.shift();
264264

265265
if (dir == null) {
266-
return q.Promise.resolve(this._graph);
266+
return Promise.resolve(this._graph);
267267
}
268268

269269
// Steps:
@@ -292,10 +292,10 @@ DependecyGraph.prototype._search = function() {
292292

293293
var processing = self._findAndProcessPackage(files, dir)
294294
.then(function() {
295-
return q.all(modulePaths.map(self._processModule.bind(self)));
295+
return Promise.all(modulePaths.map(self._processModule.bind(self)));
296296
});
297297

298-
return q.all([
298+
return Promise.all([
299299
processing,
300300
self._search()
301301
]);
@@ -324,7 +324,7 @@ DependecyGraph.prototype._findAndProcessPackage = function(files, root) {
324324
if (packagePath != null) {
325325
return this._processPackage(packagePath);
326326
} else {
327-
return q();
327+
return Promise.resolve();
328328
}
329329
};
330330

@@ -338,15 +338,15 @@ DependecyGraph.prototype._processPackage = function(packagePath) {
338338
packageJson = JSON.parse(content);
339339
} catch (e) {
340340
debug('WARNING: malformed package.json: ', packagePath);
341-
return q();
341+
return Promise.resolve();
342342
}
343343

344344
if (packageJson.name == null) {
345345
debug(
346346
'WARNING: package.json `%s` is missing a name field',
347347
packagePath
348348
);
349-
return q();
349+
return Promise.resolve();
350350
}
351351

352352
packageJson._root = packageRoot;
@@ -556,7 +556,7 @@ DependecyGraph.prototype._getAbsolutePath = function(filePath) {
556556

557557
DependecyGraph.prototype._buildAssetMap = function() {
558558
if (this._assetRoots == null || this._assetRoots.length === 0) {
559-
return q();
559+
return Promise.resolve();
560560
}
561561

562562
this._assetMap = Object.create(null);
@@ -640,13 +640,13 @@ function withExtJs(file) {
640640

641641
function handleBrokenLink(e) {
642642
debug('WARNING: error stating, possibly broken symlink', e.message);
643-
return q();
643+
return Promise.resolve();
644644
}
645645

646646
function readAndStatDir(dir) {
647647
return readDir(dir)
648648
.then(function(files){
649-
return q.all(files.map(function(filePath) {
649+
return Promise.all(files.map(function(filePath) {
650650
return realpath(path.join(dir, filePath)).catch(handleBrokenLink);
651651
}));
652652
}).then(function(files) {
@@ -660,7 +660,7 @@ function readAndStatDir(dir) {
660660

661661
return [
662662
files,
663-
q.all(stats),
663+
Promise.all(stats),
664664
];
665665
});
666666
}
@@ -676,7 +676,7 @@ function buildAssetMap(roots, processAsset) {
676676
var root = queue.shift();
677677

678678
if (root == null) {
679-
return q();
679+
return Promise.resolve();
680680
}
681681

682682
return readAndStatDir(root).spread(function(files, stats) {

packager/react-packager/src/DependencyResolver/haste/__tests__/HasteDependencyResolver-test.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ jest.dontMock('../')
1313
.dontMock('../requirePattern')
1414
.setMock('../../ModuleDescriptor', function(data) {return data;});
1515

16-
var q = require('q');
16+
var Promise = require('bluebird');
1717

1818
describe('HasteDependencyResolver', function() {
1919
var HasteDependencyResolver;
@@ -41,7 +41,7 @@ describe('HasteDependencyResolver', function() {
4141
return deps;
4242
});
4343
depGraph.load.mockImpl(function() {
44-
return q();
44+
return Promise.resolve();
4545
});
4646

4747
return depResolver.getDependencies('/root/index.js', { dev: false })
@@ -101,7 +101,7 @@ describe('HasteDependencyResolver', function() {
101101
return deps;
102102
});
103103
depGraph.load.mockImpl(function() {
104-
return q();
104+
return Promise.resolve();
105105
});
106106

107107
return depResolver.getDependencies('/root/index.js', { dev: true })
@@ -162,7 +162,7 @@ describe('HasteDependencyResolver', function() {
162162
return deps;
163163
});
164164
depGraph.load.mockImpl(function() {
165-
return q();
165+
return Promise.resolve();
166166
});
167167

168168
return depResolver.getDependencies('/root/index.js', { dev: false })

packager/react-packager/src/DependencyResolver/node/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*/
99
'use strict';
1010

11-
var Promise = require('q').Promise;
11+
var Promise = require('bluebird');
1212
var ModuleDescriptor = require('../ModuleDescriptor');
1313

1414
var mdeps = require('module-deps');

packager/react-packager/src/FileWatcher/index.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@
1010

1111
var EventEmitter = require('events').EventEmitter;
1212
var sane = require('sane');
13-
var q = require('q');
13+
var Promise = require('bluebird');
1414
var util = require('util');
1515
var exec = require('child_process').exec;
1616

17-
var Promise = q.Promise;
18-
1917
var detectingWatcherClass = new Promise(function(resolve) {
2018
exec('which watchman', function(err, out) {
2119
if (err || out.length === 0) {
@@ -41,7 +39,7 @@ function FileWatcher(rootConfigs) {
4139

4240
fileWatcher = this;
4341

44-
this._loading = q.all(
42+
this._loading = Promise.all(
4543
rootConfigs.map(createWatcher)
4644
).then(function(watchers) {
4745
watchers.forEach(function(watcher) {
@@ -59,7 +57,7 @@ util.inherits(FileWatcher, EventEmitter);
5957
FileWatcher.prototype.end = function() {
6058
return this._loading.then(function(watchers) {
6159
watchers.forEach(function(watcher) {
62-
return q.ninvoke(watcher, 'close');
60+
return Promise.promisify(watcher.close, watcher)();
6361
});
6462
});
6563
};
@@ -88,7 +86,7 @@ function createWatcher(rootConfig) {
8886
FileWatcher.createDummyWatcher = function() {
8987
var ev = new EventEmitter();
9088
ev.end = function() {
91-
return q();
89+
return Promise.resolve();
9290
};
9391
return ev;
9492
};

packager/react-packager/src/JSTransformer/Cache.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,10 @@ var declareOpts = require('../lib/declareOpts');
1414
var fs = require('fs');
1515
var isAbsolutePath = require('absolute-path');
1616
var path = require('path');
17-
var q = require('q');
17+
var Promise = require('bluebird');
1818
var tmpdir = require('os').tmpDir();
1919
var version = require('../../../../package.json').version;
2020

21-
var Promise = q.Promise;
22-
2321
var validateOpts = declareOpts({
2422
resetCache: {
2523
type: 'boolean',
@@ -74,7 +72,7 @@ Cache.prototype._set = function(filepath, loaderPromise) {
7472
this._data[filepath] = loaderPromise.then(function(data) {
7573
return [
7674
data,
77-
q.nfbind(fs.stat)(filepath)
75+
Promise.promisify(fs.stat)(filepath)
7876
];
7977
}).spread(function(data, stat) {
8078
this._persistEventually();
@@ -105,13 +103,13 @@ Cache.prototype._persistCache = function() {
105103
var data = this._data;
106104
var cacheFilepath = this._cacheFilePath;
107105

108-
this._persisting = q.all(_.values(data))
106+
this._persisting = Promise.all(_.values(data))
109107
.then(function(values) {
110108
var json = Object.create(null);
111109
Object.keys(data).forEach(function(key, i) {
112110
json[key] = values[i];
113111
});
114-
return q.nfbind(fs.writeFile)(cacheFilepath, JSON.stringify(json));
112+
return Promise.promisify(fs.writeFile)(cacheFilepath, JSON.stringify(json));
115113
})
116114
.then(function() {
117115
this._persisting = null;

packager/react-packager/src/JSTransformer/__tests__/Cache-test.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jest
1515
.dontMock('crypto')
1616
.dontMock('../Cache');
1717

18-
var q = require('q');
18+
var Promise = require('bluebird');
1919

2020
describe('JSTransformer Cache', function() {
2121
var Cache;
@@ -32,7 +32,7 @@ describe('JSTransformer Cache', function() {
3232
it('calls loader callback for uncached file', function() {
3333
var cache = new Cache({projectRoots: ['/rootDir']});
3434
var loaderCb = jest.genMockFn().mockImpl(function() {
35-
return q();
35+
return Promise.resolve();
3636
});
3737
cache.get('/rootDir/someFile', loaderCb);
3838
expect(loaderCb).toBeCalledWith('/rootDir/someFile');
@@ -48,7 +48,7 @@ describe('JSTransformer Cache', function() {
4848
});
4949
var cache = new Cache({projectRoots: ['/rootDir']});
5050
var loaderCb = jest.genMockFn().mockImpl(function() {
51-
return q('lol');
51+
return Promise.resolve('lol');
5252
});
5353
return cache.get('/rootDir/someFile', loaderCb).then(function(value) {
5454
expect(value).toBe('lol');
@@ -65,7 +65,7 @@ describe('JSTransformer Cache', function() {
6565
});
6666
var cache = new Cache({projectRoots: ['/rootDir']});
6767
var loaderCb = jest.genMockFn().mockImpl(function() {
68-
return q('lol');
68+
return Promise.resolve('lol');
6969
});
7070
return cache.get('/rootDir/someFile', loaderCb).then(function() {
7171
var shouldNotBeCalled = jest.genMockFn();
@@ -152,7 +152,7 @@ describe('JSTransformer Cache', function() {
152152

153153
var cache = new Cache({projectRoots: ['/rootDir']});
154154
var loaderCb = jest.genMockFn().mockImpl(function() {
155-
return q('new value');
155+
return Promise.resolve('new value');
156156
});
157157

158158
return cache.get('/rootDir/someFile', loaderCb).then(function(value) {
@@ -193,13 +193,13 @@ describe('JSTransformer Cache', function() {
193193

194194
var cache = new Cache({projectRoots: ['/rootDir']});
195195
cache.get('/rootDir/bar', function() {
196-
return q('bar value');
196+
return Promise.resolve('bar value');
197197
});
198198
cache.get('/rootDir/foo', function() {
199-
return q('foo value');
199+
return Promise.resolve('foo value');
200200
});
201201
cache.get('/rootDir/baz', function() {
202-
return q('baz value');
202+
return Promise.resolve('baz value');
203203
});
204204

205205
jest.runAllTicks();

packager/react-packager/src/JSTransformer/index.js

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@
99
'use strict';
1010

1111
var fs = require('fs');
12-
var q = require('q');
12+
var Promise = require('bluebird');
1313
var Cache = require('./Cache');
14-
var _ = require('underscore');
1514
var workerFarm = require('worker-farm');
1615
var declareOpts = require('../lib/declareOpts');
1716
var util = require('util');
1817

19-
var readFile = q.nfbind(fs.readFile);
18+
var readFile = Promise.promisify(fs.readFile);
2019

2120
module.exports = Transformer;
2221
Transformer.TransformError = TransformError;
@@ -63,12 +62,14 @@ function Transformer(options) {
6362
});
6463

6564
if (options.transformModulePath == null) {
66-
this._failedToStart = q.Promise.reject(new Error('No transfrom module'));
65+
this._failedToStart = Promise.reject(new Error('No transfrom module'));
6766
} else {
6867
this._workers = workerFarm(
6968
{autoStart: true, maxConcurrentCallsPerWorker: 1},
7069
options.transformModulePath
7170
);
71+
72+
this._transform = Promise.promisify(this._workers);
7273
}
7374
}
7475

@@ -86,13 +87,13 @@ Transformer.prototype.loadFileAndTransform = function(filePath) {
8687
return this._failedToStart;
8788
}
8889

89-
var workers = this._workers;
90+
var transform = this._transform;
9091
return this._cache.get(filePath, function() {
9192
return readFile(filePath)
9293
.then(function(buffer) {
9394
var sourceCode = buffer.toString();
9495

95-
return q.nfbind(workers)({
96+
return transform({
9697
sourceCode: sourceCode,
9798
filename: filePath,
9899
}).then(

0 commit comments

Comments
 (0)