Skip to content

Commit

Permalink
Add --maxWorkers flag and allow transformers to run in-band.
Browse files Browse the repository at this point in the history
Summary:
This diff cleans up some cruft and adds some features:

* It removes the usage of an env variable to control workers.
* It removes the lazy and handwavy calculation on how many workers to use for jest-haste-map. Jest itself uses the maximum amount of workers available and it has never been reported as an issue – especially since it is a one-time startup cost of about 3 seconds on a cold cache only.
* It adds a `--max-workers` flag to replace the env variable. This one is able to control both the number of workers for `jest-haste-map` as well as the transformers.
* It makes the transformers run in the parent process if 1 or fewer workers are are specified. This should help with debugging.

Once you approve this diff, I will publish a new version of metro to npm and update the version used in RN and remove the use of the env variable altogether: https://our.intern.facebook.com/intern/biggrep/?corpus=xplat&filename=&case=false&view=default&extre=&s=REACT_NATIVE_MAX_WORKERS&engine=apr_strmatch&context=false&filter[uninteresting]=false&filter[intern]=false&filter[test]=false&grep_regex=

Note: the process of adding a CLI option is really broken. Commander also has a weird API. We should consider building a better public API for Metro and then consider how to build a new CLI on top of it and simplify our internal integration. I really don't like how Metro is integrated across pieces of the RN cli in ways that is hard to manage. But that is a larger task for another time :)

Reviewed By: jeanlauliac

Differential Revision: D5217726

fbshipit-source-id: 74efddbb87755a9e744c816fbc62efa21f6a79bf
  • Loading branch information
cpojer authored and facebook-github-bot committed Jun 13, 2017
1 parent f704441 commit 29d9c35
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 18 deletions.
1 change: 0 additions & 1 deletion ContainerShip/Dockerfile.android
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ FROM containership/android-base:latest
# set default environment variables # set default environment variables
ENV GRADLE_OPTS="-Dorg.gradle.jvmargs=\"-Xmx512m -XX:+HeapDumpOnOutOfMemoryError\"" ENV GRADLE_OPTS="-Dorg.gradle.jvmargs=\"-Xmx512m -XX:+HeapDumpOnOutOfMemoryError\""
ENV JAVA_TOOL_OPTIONS="-Dfile.encoding=UTF8" ENV JAVA_TOOL_OPTIONS="-Dfile.encoding=UTF8"
ENV REACT_NATIVE_MAX_WORKERS=1


# add ReactAndroid directory # add ReactAndroid directory
ADD .buckconfig /app/.buckconfig ADD .buckconfig /app/.buckconfig
Expand Down
5 changes: 2 additions & 3 deletions ContainerShip/scripts/run-ci-e2e-tests.sh
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ AVD_UUID=$(cat /dev/urandom | tr -dc 'a-zA-Z0-9' | fold -w 8 | head -n 1)
ANDROID_NPM_DEPS="appium@1.5.1 mocha@2.4.5 wd@0.3.11 colors@1.0.3 pretty-data2@0.40.1" ANDROID_NPM_DEPS="appium@1.5.1 mocha@2.4.5 wd@0.3.11 colors@1.0.3 pretty-data2@0.40.1"
CLI_PACKAGE=$ROOT/react-native-cli/react-native-cli-*.tgz CLI_PACKAGE=$ROOT/react-native-cli/react-native-cli-*.tgz
PACKAGE=$ROOT/react-native-*.tgz PACKAGE=$ROOT/react-native-*.tgz
REACT_NATIVE_MAX_WORKERS=1


# solve issue with max user watches limit # solve issue with max user watches limit
echo 65536 | tee -a /proc/sys/fs/inotify/max_user_watches echo 65536 | tee -a /proc/sys/fs/inotify/max_user_watches
Expand Down Expand Up @@ -231,13 +230,13 @@ function e2e_suite() {
# js tests # js tests
if [ $RUN_JS -ne 0 ]; then if [ $RUN_JS -ne 0 ]; then
# Check the packager produces a bundle (doesn't throw an error) # Check the packager produces a bundle (doesn't throw an error)
REACT_NATIVE_MAX_WORKERS=1 react-native bundle --platform android --dev true --entry-file index.android.js --bundle-output android-bundle.js react-native bundle --max-workers 1 --platform android --dev true --entry-file index.android.js --bundle-output android-bundle.js
if [ $? -ne 0 ]; then if [ $? -ne 0 ]; then
echo "Could not build android bundle" echo "Could not build android bundle"
return 1 return 1
fi fi


REACT_NATIVE_MAX_WORKERS=1 react-native bundle --platform ios --dev true --entry-file index.ios.js --bundle-output ios-bundle.js react-native bundle --max-workers 1 --platform ios --dev true --entry-file index.ios.js --bundle-output ios-bundle.js
if [ $? -ne 0 ]; then if [ $? -ne 0 ]; then
echo "Could not build iOS bundle" echo "Could not build iOS bundle"
return 1 return 1
Expand Down
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ test:


# integration tests # integration tests
# build JS bundle for instrumentation tests # build JS bundle for instrumentation tests
- REACT_NATIVE_MAX_WORKERS=1 node local-cli/cli.js bundle --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js - node local-cli/cli.js bundle --max-workers 1 --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js
# build test APK # build test APK
- buck install ReactAndroid/src/androidTest/buck-runner:instrumentation-tests --config build.threads=1 - buck install ReactAndroid/src/androidTest/buck-runner:instrumentation-tests --config build.threads=1
# run installed apk with tests # run installed apk with tests
Expand Down
4 changes: 3 additions & 1 deletion local-cli/bundle/buildBundle.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@


const log = require('../util/log').out('bundle'); const log = require('../util/log').out('bundle');
const Server = require('metro-bundler/build/Server'); const Server = require('metro-bundler/build/Server');
const Terminal = require('metro-bundler/build/lib/TerminalClass'); const Terminal = require('metro-bundler/build/lib/Terminal');
const TerminalReporter = require('metro-bundler/build/lib/TerminalReporter'); const TerminalReporter = require('metro-bundler/build/lib/TerminalReporter');
const TransformCaching = require('metro-bundler/build/lib/TransformCaching'); const TransformCaching = require('metro-bundler/build/lib/TransformCaching');


Expand All @@ -38,6 +38,7 @@ function buildBundle(
args: OutputOptions & { args: OutputOptions & {
assetsDest: mixed, assetsDest: mixed,
entryFile: string, entryFile: string,
maxWorkers: number,
resetCache: boolean, resetCache: boolean,
transformer: string, transformer: string,
}, },
Expand Down Expand Up @@ -89,6 +90,7 @@ function buildBundle(
getTransformOptions: config.getTransformOptions, getTransformOptions: config.getTransformOptions,
globalTransformCache: null, globalTransformCache: null,
hasteImpl: config.hasteImpl, hasteImpl: config.hasteImpl,
maxWorkers: args.maxWorkers,
platforms: defaultPlatforms.concat(platforms), platforms: defaultPlatforms.concat(platforms),
postMinifyProcess: config.postMinifyProcess, postMinifyProcess: config.postMinifyProcess,
postProcessModules: config.postProcessModules, postProcessModules: config.postProcessModules,
Expand Down
6 changes: 6 additions & 0 deletions local-cli/bundle/bundleCommandLineArgs.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ module.exports = [
command: '--bundle-encoding [string]', command: '--bundle-encoding [string]',
description: 'Encoding the bundle should be written in (https://nodejs.org/api/buffer.html#buffer_buffer).', description: 'Encoding the bundle should be written in (https://nodejs.org/api/buffer.html#buffer_buffer).',
default: 'utf8', default: 'utf8',
}, {
command: '--max-workers [number]',
description: 'Specifies the maximum number of workers the worker-pool ' +
'will spawn for transforming files. This defaults to the number of the ' +
'cores available on your machine.',
parse: (workers: string) => Number(workers),
}, { }, {
command: '--sourcemap-output [string]', command: '--sourcemap-output [string]',
description: 'File name where to store the sourcemap file for resulting bundle, ex. /tmp/groups.map', description: 'File name where to store the sourcemap file for resulting bundle, ex. /tmp/groups.map',
Expand Down
6 changes: 6 additions & 0 deletions local-cli/dependencies/dependencies.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ module.exports = {
}, { }, {
command: '--transformer [path]', command: '--transformer [path]',
description: 'Specify a custom transformer to be used' description: 'Specify a custom transformer to be used'
}, {
command: '--max-workers [number]',
description: 'Specifies the maximum number of workers the worker-pool ' +
'will spawn for transforming files. This defaults to the number of the ' +
'cores available on your machine.',
parse: (workers: string) => Number(workers),
}, { }, {
command: '--dev [boolean]', command: '--dev [boolean]',
description: 'If false, skip all dev-only code path', description: 'If false, skip all dev-only code path',
Expand Down
6 changes: 4 additions & 2 deletions local-cli/server/runServer.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
require('../../setupBabel')(); require('../../setupBabel')();
const InspectorProxy = require('./util/inspectorProxy.js'); const InspectorProxy = require('./util/inspectorProxy.js');
const ReactPackager = require('metro-bundler'); const ReactPackager = require('metro-bundler');
const Terminal = require('metro-bundler/build/lib/TerminalClass'); const Terminal = require('metro-bundler/build/lib/Terminal');


const attachHMRServer = require('./util/attachHMRServer'); const attachHMRServer = require('./util/attachHMRServer');
const connect = require('connect'); const connect = require('connect');
Expand Down Expand Up @@ -46,6 +46,7 @@ import type {Reporter} from 'metro-bundler/build/lib/reporting';
export type Args = {| export type Args = {|
+assetExts: $ReadOnlyArray<string>, +assetExts: $ReadOnlyArray<string>,
+host: string, +host: string,
+maxWorkers: number,
+nonPersistent: boolean, +nonPersistent: boolean,
+platforms: $ReadOnlyArray<string>, +platforms: $ReadOnlyArray<string>,
+port: number, +port: number,
Expand Down Expand Up @@ -160,10 +161,11 @@ function getPackagerServer(args, config) {
extraNodeModules: config.extraNodeModules, extraNodeModules: config.extraNodeModules,
getTransformOptions: config.getTransformOptions, getTransformOptions: config.getTransformOptions,
hasteImpl: config.hasteImpl, hasteImpl: config.hasteImpl,
maxWorkers: args.maxWorkers,
platforms: defaultPlatforms.concat(args.platforms), platforms: defaultPlatforms.concat(args.platforms),
polyfillModuleNames: config.getPolyfillModuleNames(), polyfillModuleNames: config.getPolyfillModuleNames(),
postProcessModules: config.postProcessModules,
postMinifyProcess: config.postMinifyProcess, postMinifyProcess: config.postMinifyProcess,
postProcessModules: config.postProcessModules,
projectRoots: args.projectRoots, projectRoots: args.projectRoots,
providesModuleNodeModules: providesModuleNodeModules, providesModuleNodeModules: providesModuleNodeModules,
reporter: new LogReporter(terminal), reporter: new LogReporter(terminal),
Expand Down
6 changes: 6 additions & 0 deletions local-cli/server/server.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ module.exports = {
} }
return null; return null;
}, },
}, {
command: '--max-workers [number]',
description: 'Specifies the maximum number of workers the worker-pool ' +
'will spawn for transforming files. This defaults to the number of the ' +
'cores available on your machine.',
parse: (workers: string) => Number(workers),
}, { }, {
command: '--skipflow', command: '--skipflow',
description: 'Disable flow checks' description: 'Disable flow checks'
Expand Down
3 changes: 1 addition & 2 deletions local-cli/util/Config.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
*/ */
'use strict'; 'use strict';


const findSymlinksPaths = require('./findSymlinksPaths');

const blacklist = require('metro-bundler/build/blacklist'); const blacklist = require('metro-bundler/build/blacklist');
const findSymlinksPaths = require('./findSymlinksPaths');
const fs = require('fs'); const fs = require('fs');
const invariant = require('fbjs/lib/invariant'); const invariant = require('fbjs/lib/invariant');
const path = require('path'); const path = require('path');
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@
"left-pad": "^1.1.3", "left-pad": "^1.1.3",
"lodash": "^4.16.6", "lodash": "^4.16.6",
"merge-stream": "^1.0.1", "merge-stream": "^1.0.1",
"metro-bundler": "^0.7.4", "metro-bundler": "^0.8.1",
"mime": "^1.3.4", "mime": "^1.3.4",
"mime-types": "2.1.11", "mime-types": "2.1.11",
"minimist": "^1.2.0", "minimist": "^1.2.0",
Expand Down
11 changes: 4 additions & 7 deletions scripts/run-ci-e2e-tests.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -125,12 +125,9 @@ try {
} }


echo(`Starting packager server, ${SERVER_PID}`); echo(`Starting packager server, ${SERVER_PID}`);
const packagerEnv = Object.create(process.env);
packagerEnv.REACT_NATIVE_MAX_WORKERS = 1;
// shelljs exec('', {async: true}) does not emit stdout events, so we rely on good old spawn // shelljs exec('', {async: true}) does not emit stdout events, so we rely on good old spawn
const packagerProcess = spawn('npm', ['start'], { const packagerProcess = spawn('npm', ['start', '--', '--max-workers 1'], {
// stdio: 'inherit', env: process.env
env: packagerEnv
}); });
SERVER_PID = packagerProcess.pid; SERVER_PID = packagerProcess.pid;
// wait a bit to allow packager to startup // wait a bit to allow packager to startup
Expand Down Expand Up @@ -193,12 +190,12 @@ try {


if (argv.js) { if (argv.js) {
// Check the packager produces a bundle (doesn't throw an error) // Check the packager produces a bundle (doesn't throw an error)
if (exec('REACT_NATIVE_MAX_WORKERS=1 react-native bundle --platform android --dev true --entry-file index.android.js --bundle-output android-bundle.js').code) { if (exec('react-native bundle --max-workers 1 --platform android --dev true --entry-file index.android.js --bundle-output android-bundle.js').code) {
echo('Could not build Android bundle'); echo('Could not build Android bundle');
exitCode = 1; exitCode = 1;
throw Error(exitCode); throw Error(exitCode);
} }
if (exec('REACT_NATIVE_MAX_WORKERS=1 react-native bundle --platform ios --dev true --entry-file index.ios.js --bundle-output ios-bundle.js').code) { if (exec('react-native --max-workers 1 bundle --platform ios --dev true --entry-file index.ios.js --bundle-output ios-bundle.js').code) {
echo('Could not build iOS bundle'); echo('Could not build iOS bundle');
exitCode = 1; exitCode = 1;
throw Error(exitCode); throw Error(exitCode);
Expand Down

0 comments on commit 29d9c35

Please sign in to comment.