Skip to content

Commit

Permalink
feat: allow specifying the number of parallel workers to use (#137)
Browse files Browse the repository at this point in the history
This should make it possible to better match the number of cores on CI instead
of always having 8 workers.
  • Loading branch information
alangpierce committed Oct 29, 2017
1 parent 1a26aa7 commit 89e1cd3
Show file tree
Hide file tree
Showing 13 changed files with 46 additions and 14 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ file values; see the result of `--help` for more information.
* `landBase`: if specified, overrides the auto-detected base commit when running
the `land` command. Generally this is specified on the command line using
`--land-base` rather than in a config file.
* `numWorkers`: if specified, the number of parallel workers to use for parallel
operations like `decaffeinate` and `eslint --fix`.
* `skipVerify`: set to `true` to skip the initial verification step when running
the `convert` command. This makes bulk-decaffeinate take less time, but if any
files fail to convert, it may leave the filesystem in a partially-converted
Expand Down
1 change: 1 addition & 0 deletions src/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import pluralize from './util/pluralize';
export default async function check(config) {
let filesToProcess = await getFilesToProcess(config, COFFEE_FILE_RECOGNIZER);
let decaffeinateResults = await runWithProgressBar(
config,
`Doing a dry run of decaffeinate on ${pluralize(filesToProcess.length, 'file')}...`,
filesToProcess, makeDecaffeinateVerifyFn(config),
{allowFailures: true});
Expand Down
2 changes: 2 additions & 0 deletions src/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export default function () {
`The git revision to use as the base commit when running the "land"
command. If none is specified, bulk-decaffeinate tries to use the
first auto-generated commit in recent history.`)
.option('--num-workers [number]',
`The number of workers to use for parallel operations.`)
.option('--skip-verify',
`If specified, skips the initial verification step when running the
"convert" command.`)
Expand Down
5 changes: 5 additions & 0 deletions src/config/resolveConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default async function resolveConfig(commander) {
mochaEnvFilePattern: config.mochaEnvFilePattern,
codePrefix: config.codePrefix,
landBase: config.landBase,
numWorkers: config.numWorkers || 8,
skipVerify: config.skipVerify,
decaffeinatePath: await resolveDecaffeinatePath(config),
jscodeshiftPath: await resolveJscodeshiftPath(config),
Expand Down Expand Up @@ -99,6 +100,7 @@ function getCLIParamsConfig(config, commander) {
dir,
useJsModules,
landBase,
numWorkers,
skipVerify,
decaffeinatePath,
jscodeshiftPath,
Expand Down Expand Up @@ -127,6 +129,9 @@ function getCLIParamsConfig(config, commander) {
if (landBase) {
config.landBase = landBase;
}
if (numWorkers) {
config.numWorkers = numWorkers;
}
if (skipVerify) {
config.skipVerify = true;
}
Expand Down
11 changes: 9 additions & 2 deletions src/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export default async function convert(config) {
if (!config.skipVerify) {
try {
await runWithProgressBar(
config,
'Verifying that decaffeinate can successfully convert these files...',
coffeeFiles, makeDecaffeinateVerifyFn(config));
} catch (e) {
Expand All @@ -46,13 +47,15 @@ Re-run with the "check" command for more details.`);
}

await runWithProgressBar(
config,
'Backing up files to .original.coffee...',
coffeeFiles,
async function(coffeePath) {
await copy(`${coffeePath}`, `${backupPathFor(coffeePath)}`);
});

await runWithProgressBar(
config,
`Renaming files from .coffee to .${config.outputFileExtension}...`,
movingCoffeeFiles,
async function(coffeePath) {
Expand All @@ -71,26 +74,30 @@ Re-run with the "check" command for more details.`);
}

await runWithProgressBar(
config,
'Moving files back...',
movingCoffeeFiles,
async function(coffeePath) {
await move(jsPathFor(coffeePath, config), coffeePath);
});

await runWithProgressBar(
config,
'Running decaffeinate on all files...',
coffeeFiles,
makeCLIFn(path => `${decaffeinatePath} ${decaffeinateArgs.join(' ')} ${path}`)
);

await runWithProgressBar(
config,
'Deleting old files...',
coffeeFiles,
async function(coffeePath) {
await unlink(coffeePath);
});

await runWithProgressBar(
config,
'Setting proper extension for all files...',
coffeeFiles,
async function(coffeePath) {
Expand All @@ -112,15 +119,15 @@ Re-run with the "check" command for more details.`);
await runJscodeshiftScripts(jsFiles, config);
}
if (config.mochaEnvFilePattern) {
await prependMochaEnv(jsFiles, config.mochaEnvFilePattern);
await prependMochaEnv(config, jsFiles, config.mochaEnvFilePattern);
}
let thirdCommitModifiedFiles = jsFiles.slice();
if (config.fixImportsConfig) {
thirdCommitModifiedFiles = await runFixImports(jsFiles, config);
}
await runEslintFix(jsFiles, config, {isUpdate: false});
if (config.codePrefix) {
await prependCodePrefix(jsFiles, config.codePrefix);
await prependCodePrefix(config, jsFiles, config.codePrefix);
}

let postProcessCommitMsg =
Expand Down
3 changes: 2 additions & 1 deletion src/modernize/prependCodePrefix.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import prependToFile from '../util/prependToFile';
import runWithProgressBar from '../runner/runWithProgressBar';

export default async function prependCodePrefix(jsFiles, codePrefix) {
export default async function prependCodePrefix(config, jsFiles, codePrefix) {
await runWithProgressBar(
config,
'Adding code prefix to converted files...', jsFiles, async function(path) {
await prependToFile(path, codePrefix);
return {error: null};
Expand Down
3 changes: 2 additions & 1 deletion src/modernize/prependMochaEnv.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import runWithProgressBar from '../runner/runWithProgressBar';
import prependToFile from '../util/prependToFile';

export default async function prependMochaEnv(jsFiles, mochaEnvFilePattern) {
export default async function prependMochaEnv(config, jsFiles, mochaEnvFilePattern) {
let regex = new RegExp(mochaEnvFilePattern);
let testFiles = jsFiles.filter(f => regex.test(f));
await runWithProgressBar(
config,
'Adding /* eslint-env mocha */ to test files...', testFiles, async function(path) {
await prependToFile(path, '/* eslint-env mocha */\n');
return {error: null};
Expand Down
7 changes: 5 additions & 2 deletions src/modernize/removeAutogeneratedHeader.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ import { readFile, writeFile } from 'fs-promise';
import runWithProgressBar from '../runner/runWithProgressBar';
import { HEADER_COMMENT_LINES } from './runEslintFix';

export default async function removeAutogeneratedHeader(jsFiles) {
export default async function removeAutogeneratedHeader(config, jsFiles) {
await runWithProgressBar(
'Removing any existing autogenerated headers...', jsFiles, removeHeadersFromFile);
config,
'Removing any existing autogenerated headers...',
jsFiles,
removeHeadersFromFile);
}

async function removeHeadersFromFile(path) {
Expand Down
1 change: 1 addition & 0 deletions src/modernize/runEslintFix.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import prependToFile from '../util/prependToFile';

export default async function runEslintFix(jsFiles, config, {isUpdate}) {
let eslintResults = await runWithProgressBar(
config,
'Running eslint --fix on all files...', jsFiles, makeEslintFixFn(config, {isUpdate}));
for (let result of eslintResults) {
for (let message of result.messages) {
Expand Down
6 changes: 4 additions & 2 deletions src/modernize/runFixImports.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ export default async function runFixImports(jsFiles, config) {
convertedFiles: jsFiles.map(p => resolve(p)),
absoluteImportPaths: absoluteImportPaths.map(p => resolve(p)),
};
let eligibleFixImportsFiles = await getEligibleFixImportsFiles(searchPath, jsFiles);
let eligibleFixImportsFiles = await getEligibleFixImportsFiles(
config, searchPath, jsFiles);
console.log('Fixing any imports across the whole codebase...');
if (eligibleFixImportsFiles.length > 0) {
// Note that the args can get really long, so we take reasonable steps to
Expand All @@ -36,11 +37,12 @@ export default async function runFixImports(jsFiles, config) {
return eligibleFixImportsFiles;
}

async function getEligibleFixImportsFiles(searchPath, jsFiles) {
async function getEligibleFixImportsFiles(config, searchPath, jsFiles) {
let jsBasenames = jsFiles.map(p => basename(p, '.js'));
let resolvedPaths = jsFiles.map(p => resolve(p));
let allJsFiles = await getFilesUnderPath(searchPath, p => p.endsWith('.js'));
await runWithProgressBar(
config,
'Searching for files that may need to have updated imports...',
allJsFiles,
async function(p) {
Expand Down
3 changes: 2 additions & 1 deletion src/modernizeJS.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ export default async function modernizeJS(config) {
return;
}

await removeAutogeneratedHeader(jsFiles);
await removeAutogeneratedHeader(config, jsFiles);
await runWithProgressBar(
config,
'Running decaffeinate --modernize-js on all files...',
jsFiles,
makeCLIFn(path => `${decaffeinatePath} --modernize-js ${decaffeinateArgs.join(' ')} ${path}`)
Expand Down
8 changes: 3 additions & 5 deletions src/runner/runWithProgressBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import runInParallel from './runInParallel';
import CLIError from '../util/CLIError';
import pluralize from '../util/pluralize';

const NUM_CONCURRENT_PROCESSES = 8;

/**
* Run the given command in parallel, showing a progress bar of results.
*
Expand All @@ -15,13 +13,13 @@ const NUM_CONCURRENT_PROCESSES = 8;
* any other fields.
*/
export default async function runWithProgressBar(
description, files, asyncFn, {runInSeries, allowFailures}={}) {
config, description, files, asyncFn, {runInSeries, allowFailures}={}) {
let numProcessed = 0;
let numFailures = 0;
let numTotal = files.length;
let startTime = moment();
console.log(description);
let numConcurrentProcesses = runInSeries ? 1 : NUM_CONCURRENT_PROCESSES;
let numConcurrentProcesses = runInSeries ? 1 : config.numWorkers;
console.log(`${description} (${pluralize(numConcurrentProcesses, 'worker')})`);
let results;
try {
results = await runInParallel(files, asyncFn, numConcurrentProcesses, ({result}) => {
Expand Down
8 changes: 8 additions & 0 deletions test/bulk-decaffeinate-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,4 +141,12 @@ describe('check', () => {
assertIncludes(stdout, 'All checks succeeded');
});
});

it('allows specifying the number of parallel workers', async function() {
await runWithTemplateDir('simple-success', async function() {
let {stdout, stderr} = await runCli('check --num-workers 3');
assert.equal(stderr, '');
assertIncludes(stdout, '(3 workers)');
});
});
});

0 comments on commit 89e1cd3

Please sign in to comment.