packager: Bundler: refactor the maxWorkerCount

Summary: The function giving the worker count was duplicated, let's just pass it down from a central place, the Bundler. Also, I simplified the function to use a simple formula rather than arbitrary ranges (it's still arbitrary, just a tad bit less :D ).

Reviewed By: davidaurelio

Differential Revision: D4689366

fbshipit-source-id: fe5b349396f1a07858f4f80ccaa63c375122fac8
This commit is contained in:
Jean Lauliac 2017-03-13 05:17:54 -07:00 committed by Facebook Github Bot
parent 4e106d45c3
commit bdacb9595b
6 changed files with 50 additions and 50 deletions

View File

@ -15,6 +15,7 @@ jest
.setMock('uglify-js') .setMock('uglify-js')
.mock('image-size') .mock('image-size')
.mock('fs') .mock('fs')
.mock('os')
.mock('assert') .mock('assert')
.mock('progress') .mock('progress')
.mock('../../node-haste') .mock('../../node-haste')
@ -31,6 +32,7 @@ var Resolver = require('../../Resolver');
var defaults = require('../../../defaults'); var defaults = require('../../../defaults');
var sizeOf = require('image-size'); var sizeOf = require('image-size');
var fs = require('fs'); var fs = require('fs');
const os = require('os');
var commonOptions = { var commonOptions = {
allowBundleUpdates: false, allowBundleUpdates: false,
@ -76,6 +78,8 @@ describe('Bundler', function() {
var projectRoots; var projectRoots;
beforeEach(function() { beforeEach(function() {
os.cpus.mockReturnValue({length: 1});
getDependencies = jest.fn(); getDependencies = jest.fn();
getModuleSystemDependencies = jest.fn(); getModuleSystemDependencies = jest.fn();
projectRoots = ['/root']; projectRoots = ['/root'];
@ -348,5 +352,16 @@ describe('Bundler', function() {
'/root/img/new_image2@3x.png', '/root/img/new_image2@3x.png',
])); ]));
}); });
it('return correct number of workers', () => {
os.cpus.mockReturnValue({length: 1});
expect(Bundler.getMaxWorkerCount()).toBe(1);
os.cpus.mockReturnValue({length: 8});
expect(Bundler.getMaxWorkerCount()).toBe(6);
os.cpus.mockReturnValue({length: 24});
expect(Bundler.getMaxWorkerCount()).toBe(14);
process.env.REACT_NATIVE_MAX_WORKERS = 5;
expect(Bundler.getMaxWorkerCount()).toBe(5);
});
}); });
}); });

View File

@ -25,6 +25,8 @@ const imageSize = require('image-size');
const path = require('path'); const path = require('path');
const denodeify = require('denodeify'); const denodeify = require('denodeify');
const defaults = require('../../defaults'); const defaults = require('../../defaults');
const os = require('os');
const invariant = require('fbjs/lib/invariant');
const { const {
sep: pathSeparator, sep: pathSeparator,
@ -163,8 +165,10 @@ class Bundler {
cacheKey: transformCacheKey, cacheKey: transformCacheKey,
}); });
const maxWorkerCount = Bundler.getMaxWorkerCount();
/* $FlowFixMe: in practice it's always here. */ /* $FlowFixMe: in practice it's always here. */
this._transformer = new Transformer(opts.transformModulePath); this._transformer = new Transformer(opts.transformModulePath, maxWorkerCount);
const getTransformCacheKey = (src, filename, options) => { const getTransformCacheKey = (src, filename, options) => {
return transformCacheKey + getCacheKey(src, filename, options); return transformCacheKey + getCacheKey(src, filename, options);
@ -178,6 +182,7 @@ class Bundler {
getTransformCacheKey, getTransformCacheKey,
globalTransformCache: opts.globalTransformCache, globalTransformCache: opts.globalTransformCache,
hasteImpl: opts.hasteImpl, hasteImpl: opts.hasteImpl,
maxWorkerCount,
minifyCode: this._transformer.minify, minifyCode: this._transformer.minify,
moduleFormat: opts.moduleFormat, moduleFormat: opts.moduleFormat,
platforms: new Set(opts.platforms), platforms: new Set(opts.platforms),
@ -780,6 +785,26 @@ class Bundler {
getResolver(): Promise<Resolver> { getResolver(): Promise<Resolver> {
return this._resolverPromise; return this._resolverPromise;
} }
/**
* Unless overriden, we use a diminishing amount of workers per core, because
* using more and more of them does not scale much. Ex. 6 workers for 8
* cores, or 14 workers for 24 cores.
*/
static getMaxWorkerCount() {
const cores = os.cpus().length;
const envStr = process.env.REACT_NATIVE_MAX_WORKERS;
if (envStr == null) {
return Math.max(1, Math.ceil(cores * (0.5 + 0.5 * Math.exp(-cores * 0.07)) - 1));
}
const envCount = parseInt(process.env.REACT_NATIVE_MAX_WORKERS, 10);
invariant(
Number.isInteger(envCount),
'environment variable `REACT_NATIVE_MAX_WORKERS` must be a valid integer',
);
return Math.min(cores, envCount);
}
} }
function getPathRelativeToRoot(roots, absPath) { function getPathRelativeToRoot(roots, absPath) {

View File

@ -16,7 +16,6 @@ const Logger = require('../Logger');
const debug = require('debug')('RNP:JStransformer'); const debug = require('debug')('RNP:JStransformer');
const denodeify = require('denodeify'); const denodeify = require('denodeify');
const invariant = require('fbjs/lib/invariant'); const invariant = require('fbjs/lib/invariant');
const os = require('os');
const path = require('path'); const path = require('path');
const util = require('util'); const util = require('util');
const workerFarm = require('worker-farm'); const workerFarm = require('worker-farm');
@ -35,24 +34,7 @@ const TRANSFORM_TIMEOUT_INTERVAL = 301000;
// How may times can we tolerate failures from the worker. // How may times can we tolerate failures from the worker.
const MAX_RETRIES = 2; const MAX_RETRIES = 2;
const maxConcurrentWorkers = ((cores, override) => { function makeFarm(worker, methods, timeout, maxConcurrentWorkers) {
if (override) {
return Math.min(cores, override);
}
if (cores < 3) {
return cores;
}
if (cores < 8) {
return Math.floor(cores * 0.75);
}
if (cores < 24) {
return Math.floor(3 / 8 * cores + 3); // between cores *.75 and cores / 2
}
return cores / 2;
})(os.cpus().length, parseInt(process.env.REACT_NATIVE_MAX_WORKERS, 10));
function makeFarm(worker, methods, timeout) {
return workerFarm( return workerFarm(
{ {
autoStart: true, autoStart: true,
@ -83,7 +65,7 @@ class Transformer {
sourceMap: SourceMap, sourceMap: SourceMap,
) => Promise<{code: string, map: SourceMap}>; ) => Promise<{code: string, map: SourceMap}>;
constructor(transformModulePath: string) { constructor(transformModulePath: string, maxWorkerCount: number) {
invariant(path.isAbsolute(transformModulePath), 'transform module path should be absolute'); invariant(path.isAbsolute(transformModulePath), 'transform module path should be absolute');
this._transformModulePath = transformModulePath; this._transformModulePath = transformModulePath;
@ -91,6 +73,7 @@ class Transformer {
require.resolve('./worker'), require.resolve('./worker'),
['minify', 'transformAndExtractDependencies'], ['minify', 'transformAndExtractDependencies'],
TRANSFORM_TIMEOUT_INTERVAL, TRANSFORM_TIMEOUT_INTERVAL,
maxWorkerCount,
); );
this._transform = denodeify(this._workers.transformAndExtractDependencies); this._transform = denodeify(this._workers.transformAndExtractDependencies);
this.minify = denodeify(this._workers.minify); this.minify = denodeify(this._workers.minify);

View File

@ -37,6 +37,7 @@ type Options = {
getTransformCacheKey: GetTransformCacheKey, getTransformCacheKey: GetTransformCacheKey,
globalTransformCache: ?GlobalTransformCache, globalTransformCache: ?GlobalTransformCache,
hasteImpl?: HasteImpl, hasteImpl?: HasteImpl,
maxWorkerCount: number,
minifyCode: MinifyCode, minifyCode: MinifyCode,
platforms: Set<string>, platforms: Set<string>,
polyfillModuleNames?: Array<string>, polyfillModuleNames?: Array<string>,

View File

@ -112,7 +112,7 @@ describe('DependencyGraph', function() {
platforms: new Set(['ios', 'android']), platforms: new Set(['ios', 'android']),
useWatchman: false, useWatchman: false,
ignoreFilePath: () => false, ignoreFilePath: () => false,
maxWorkers: 1, maxWorkerCount: 1,
moduleOptions: {cacheTransformResults: true}, moduleOptions: {cacheTransformResults: true},
resetCache: true, resetCache: true,
transformCode: (module, sourceCode, transformOptions) => { transformCode: (module, sourceCode, transformOptions) => {

View File

@ -24,8 +24,8 @@ const fs = require('fs');
const getAssetDataFromName = require('./lib/getAssetDataFromName'); const getAssetDataFromName = require('./lib/getAssetDataFromName');
const getInverseDependencies = require('./lib/getInverseDependencies'); const getInverseDependencies = require('./lib/getInverseDependencies');
const getPlatformExtension = require('./lib/getPlatformExtension'); const getPlatformExtension = require('./lib/getPlatformExtension');
const invariant = require('fbjs/lib/invariant');
const isAbsolutePath = require('absolute-path'); const isAbsolutePath = require('absolute-path');
const os = require('os');
const path = require('path'); const path = require('path');
const replacePatterns = require('./lib/replacePatterns'); const replacePatterns = require('./lib/replacePatterns');
const util = require('util'); const util = require('util');
@ -58,7 +58,7 @@ type Options = {
getTransformCacheKey: GetTransformCacheKey, getTransformCacheKey: GetTransformCacheKey,
globalTransformCache: ?GlobalTransformCache, globalTransformCache: ?GlobalTransformCache,
ignoreFilePath: (filePath: string) => boolean, ignoreFilePath: (filePath: string) => boolean,
maxWorkers: ?number, maxWorkerCount: number,
moduleOptions: ModuleOptions, moduleOptions: ModuleOptions,
platforms: Set<string>, platforms: Set<string>,
preferNativePlatform: boolean, preferNativePlatform: boolean,
@ -87,6 +87,7 @@ class DependencyGraph extends EventEmitter {
initialModuleMap: ModuleMap, initialModuleMap: ModuleMap,
}) { }) {
super(); super();
invariant(config.opts.maxWorkerCount >= 1, 'worker count must be greater or equal to 1');
this._opts = config.opts; this._opts = config.opts;
this._haste = config.haste; this._haste = config.haste;
this._hasteFS = config.initialHasteFS; this._hasteFS = config.initialHasteFS;
@ -97,12 +98,11 @@ class DependencyGraph extends EventEmitter {
} }
static _createHaste(opts: Options): JestHasteMap { static _createHaste(opts: Options): JestHasteMap {
const mw = opts.maxWorkers;
return new JestHasteMap({ return new JestHasteMap({
extensions: opts.extensions.concat(opts.assetExts), extensions: opts.extensions.concat(opts.assetExts),
forceNodeFilesystemAPI: opts.forceNodeFilesystemAPI, forceNodeFilesystemAPI: opts.forceNodeFilesystemAPI,
ignorePattern: {test: opts.ignoreFilePath}, ignorePattern: {test: opts.ignoreFilePath},
maxWorkers: typeof mw === 'number' && mw >= 1 ? mw : getMaxWorkers(), maxWorkers: opts.maxWorkerCount,
mocksPattern: '', mocksPattern: '',
name: 'react-native-packager', name: 'react-native-packager',
platforms: Array.from(opts.platforms), platforms: Array.from(opts.platforms),
@ -306,28 +306,4 @@ function NotFoundError() {
} }
util.inherits(NotFoundError, Error); util.inherits(NotFoundError, Error);
function getMaxWorkers() {
const cores = os.cpus().length;
if (cores <= 1) {
// oh well...
return 1;
}
if (cores <= 4) {
// don't starve the CPU while still reading reasonably rapidly
return cores - 1;
}
if (cores <= 8) {
// empirical testing showed massive diminishing returns when going over
// 4 or 5 workers on 8-core machines
return Math.floor(cores * 0.75) - 1;
}
// pretty much guesswork
if (cores < 24) {
return Math.floor(3 / 8 * cores + 3);
}
return cores / 2;
}
module.exports = DependencyGraph; module.exports = DependencyGraph;