Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run uglify in parallel, using a workerpool #63

Merged
merged 2 commits into from
Mar 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 42 additions & 63 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ var mkdirp = require('mkdirp');
var srcURL = require('source-map-url');
var MatcherCollection = require('matcher-collection');
var debug = require('debug')('broccoli-uglify-sourcemap');
var queue = require('async-promise-queue');
var workerpool = require('workerpool');

var processFile = require('./lib/process-file');

module.exports = UglifyWriter;

Expand All @@ -19,6 +23,8 @@ UglifyWriter.prototype.constructor = UglifyWriter;

const silent = process.argv.indexOf('--silent') !== -1;

const worker = queue.async.asyncify((doWork) => doWork());

function UglifyWriter (inputNodes, options) {
if (!(this instanceof UglifyWriter)) {
return new UglifyWriter(inputNodes, options);
Expand All @@ -34,6 +40,14 @@ function UglifyWriter (inputNodes, options) {
},
});

// consumers of this plugin can opt-in to async and concurrent behavior
// TODO docs in the README
this.async = (this.options.async === true);
this.concurrency = this.options.concurrency || Number(process.env.JOBS) || Math.max(require('os').cpus().length - 1, 1);

// create a worker pool using an external worker script
this.pool = workerpool.pool(path.join(__dirname, 'lib', 'worker.js'), { maxWorkers: this.concurrency });

this.inputNodes = inputNodes;

var exclude = this.options.exclude;
Expand All @@ -53,6 +67,9 @@ var MatchNothing = {
UglifyWriter.prototype.build = function () {
var writer = this;

// when options.async === true, allow processFile() operations to complete asynchronously
var pendingWork = [];

this.inputPaths.forEach(function(inputPath) {
walkSync(inputPath).forEach(function(relativePath) {
if (relativePath.slice(-1) === '/') {
Expand All @@ -64,7 +81,15 @@ UglifyWriter.prototype.build = function () {
mkdirp.sync(path.dirname(outFile));

if (relativePath.slice(-3) === '.js' && !writer.excludes.match(relativePath)) {
writer.processFile(inFile, outFile, relativePath, writer.outputPath);
// wrap this in a function so it doesn't actually run yet, and can be throttled
var uglifyOperation = function() {
return writer.processFile(inFile, outFile, relativePath, writer.outputPath);
};
if (writer.async) {
pendingWork.push(uglifyOperation);
return;
}
return uglifyOperation();
} else if (relativePath.slice(-4) === '.map') {
if (writer.excludes.match(relativePath.slice(0, -4) + '.js')) {
// ensure .map files for excluded JS paths are also copied forward
Expand All @@ -77,70 +102,24 @@ UglifyWriter.prototype.build = function () {
});
});

return this.outputPath;
return queue(worker, pendingWork, writer.concurrency)
.then((/* results */) => {
// files are finished processing, shut down the workers
writer.pool.terminate();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may want to do this even if the promise fails. So in the finally clause (if this is a new promise, or a rsvp promise) otherwise we can use the catch clause.

Copy link
Collaborator Author

@mikrostew mikrostew Mar 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing this now and it looks like it doesn't support finally, so I'll use catch

return writer.outputPath;
})
.catch((e) => {
// make sure to shut down the workers on error
writer.pool.terminate();
throw e;
});
};

UglifyWriter.prototype.processFile = function(inFile, outFile, relativePath, outDir) {
var src = fs.readFileSync(inFile, 'utf-8');
var mapName = path.basename(outFile).replace(/\.js$/,'') + '.map';

var mapDir;
if (this.options.sourceMapDir) {
mapDir = path.join(outDir, this.options.sourceMapDir);
} else {
mapDir = path.dirname(path.join(outDir, relativePath));
}

let options = defaults({}, this.options.uglify);
if (options.sourceMap) {
let filename = path.basename(inFile);
let url = this.options.sourceMapDir ? '/' + path.join(this.options.sourceMapDir, mapName) : mapName;

let sourceMap = { filename, url };

if (srcURL.existsIn(src)) {
let url = srcURL.getFrom(src);
let sourceMapPath = path.join(path.dirname(inFile), url);
if (fs.existsSync(sourceMapPath)) {
sourceMap.content = JSON.parse(fs.readFileSync(sourceMapPath));
} else if (!silent) {
console.warn(`[WARN] (broccoli-uglify-sourcemap) "${url}" referenced in "${relativePath}" could not be found`);
}
}

options = defaults(options, { sourceMap });
}

var start = new Date();
debug('[starting]: %s %dKB', relativePath, (src.length / 1000));
var result = UglifyJS.minify(src, options);
var end = new Date();
var total = end - start;
if (total > 20000 && !silent) {
console.warn('[WARN] (broccoli-uglify-sourcemap) Minifying: `' + relativePath + '` took: ' + total + 'ms (more than 20,000ms)');
}

if (result.error) {
result.error.filename = relativePath;
throw result.error;
}

debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);

if (options.sourceMap) {
var newSourceMap = JSON.parse(result.map);

newSourceMap.sources = newSourceMap.sources.map(function(path){
// If out output file has the same name as one of our original
// sources, they will shadow eachother in Dev Tools. So instead we
// alter the reference to the upstream file.
if (path === relativePath) {
path = path.replace(/\.js$/, '-orig.js');
}
return path;
});
mkdirp.sync(mapDir);
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
// don't run this in the workerpool if concurrency is disabled (can set JOBS <= 1)
if (this.async && this.concurrency > 1) {
// each of these arguments is a string, which can be sent to the worker process as-is
return this.pool.exec('processFileParallel', [inFile, outFile, relativePath, outDir, silent, this.options]);
}
fs.writeFileSync(outFile, result.code);
return processFile(inFile, outFile, relativePath, outDir, silent, this.options);
};
77 changes: 77 additions & 0 deletions lib/process-file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
'use strict';

var debug = require('debug')('broccoli-uglify-sourcemap');
var defaults = require('lodash.defaultsdeep');
var fs = require('fs');
var mkdirp = require('mkdirp');
var path = require('path');
var srcURL = require('source-map-url');

var Promise = require('rsvp').Promise;
var UglifyJS = require('uglify-es');


module.exports = function processFile(inFile, outFile, relativePath, outDir, silent, _options) {
var src = fs.readFileSync(inFile, 'utf-8');
var mapName = path.basename(outFile).replace(/\.js$/,'') + '.map';

var mapDir;
if (_options.sourceMapDir) {
mapDir = path.join(outDir, _options.sourceMapDir);
} else {
mapDir = path.dirname(path.join(outDir, relativePath));
}

let options = defaults({}, _options.uglify);
if (options.sourceMap) {
let filename = path.basename(inFile);
let url = _options.sourceMapDir ? '/' + path.join(_options.sourceMapDir, mapName) : mapName;

let sourceMap = { filename, url };

if (srcURL.existsIn(src)) {
let url = srcURL.getFrom(src);
let sourceMapPath = path.join(path.dirname(inFile), url);
if (fs.existsSync(sourceMapPath)) {
sourceMap.content = JSON.parse(fs.readFileSync(sourceMapPath));
} else if (!silent) {
console.warn(`[WARN] (broccoli-uglify-sourcemap) "${url}" referenced in "${relativePath}" could not be found`);
}
}

options = defaults(options, { sourceMap });
}

var start = new Date();
debug('[starting]: %s %dKB', relativePath, (src.length / 1000));
var result = UglifyJS.minify(src, options);
var end = new Date();
var total = end - start;
if (total > 20000 && !silent) {
console.warn('[WARN] (broccoli-uglify-sourcemap) Minifying: `' + relativePath + '` took: ' + total + 'ms (more than 20,000ms)');
}

if (result.error) {
result.error.filename = relativePath;
throw result.error;
}

debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);

if (options.sourceMap) {
var newSourceMap = JSON.parse(result.map);

newSourceMap.sources = newSourceMap.sources.map(function(path){
// If out output file has the same name as one of our original
// sources, they will shadow eachother in Dev Tools. So instead we
// alter the reference to the upstream file.
if (path === relativePath) {
path = path.replace(/\.js$/, '-orig.js');
}
return path;
});
mkdirp.sync(mapDir);
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
}
fs.writeFileSync(outFile, result.code);
};
15 changes: 15 additions & 0 deletions lib/worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';

var workerpool = require('workerpool');

var processFile = require('./process-file');

// TODO - with an option to disable this parallelism
function processFileParallel(inFile, outFile, relativePath, outDir, silent, _options) {
return processFile(inFile, outFile, relativePath, outDir, silent, _options);
}

// create worker and register public functions
workerpool.worker({
processFileParallel
});
8 changes: 6 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
"license": "MIT",
"author": "Edward Faulkner <ef@alum.mit.edu>",
"files": [
"index.js"
"index.js",
"lib"
],
"main": "index.js",
"repository": {
Expand All @@ -22,6 +23,7 @@
"test:watch": "jest --watchAll"
},
"dependencies": {
"async-promise-queue": "^1.0.4",
"broccoli-plugin": "^1.2.1",
"debug": "^3.1.0",
"lodash.defaultsdeep": "^4.6.0",
Expand All @@ -30,7 +32,8 @@
"source-map-url": "^0.4.0",
"symlink-or-copy": "^1.0.1",
"uglify-es": "^3.1.3",
"walk-sync": "^0.3.2"
"walk-sync": "^0.3.2",
"workerpool": "^2.3.0"
},
"devDependencies": {
"babel-jest": "^21.2.0",
Expand All @@ -53,6 +56,7 @@
"modulePathIgnorePatterns": [
"<rootDir>/tmp"
],
"testEnvironment": "node",
"testMatch": [
"<rootDir>/test/test.js"
],
Expand Down
44 changes: 44 additions & 0 deletions test/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,50 @@ Object {
}
`;

exports[`broccoli-uglify-sourcemap generates expected output async 1`] = `
Object {
"inside": Object {
"with-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=with-upstream-sourcemap.map",
"with-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"/inner/first.js\\",\\"/inner/second.js\\",\\"/other/fourth.js\\",\\"/other/third.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AAAA,SAAAA,gBAEA,MAAA,IAAAC,MADA,IAIA,SAAAC,OACA,MAAA,IAAAD,MAAA,QCNA,SAAAE,gBACA,MAAA,IAAAF,MAAA,kBCEA,SAAAG,SACA,MAAA,IAAAH,MAAA,UCJA,SAAAI,QACA,MAAA,IAAAJ,MAAA\\",\\"file\\":\\"with-upstream-sourcemap.js\\",\\"sourcesContent\\":[\\"function meaningOfLife() {\\\\n var thisIsALongLocal = 42;\\\\n throw new Error(thisIsALongLocal);\\\\n}\\\\n\\\\nfunction boom() {\\\\n throw new Error('boom');\\\\n}\\\\n\\",\\"function somethingElse() {\\\\n throw new Error(\\\\\\"somethign else\\\\\\");\\\\n}\\\\n\\",\\"\\\\n// Hello world\\\\n\\\\nfunction fourth(){\\\\n throw new Error('fourth');\\\\n}\\\\n\\",\\"function third(){\\\\n throw new Error(\\\\\\"oh no\\\\\\");\\\\n}\\\\n\\"]}",
},
"no-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=no-upstream-sourcemap.map",
"no-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AACA,SAASA,gBAEP,MAAM,IAAIC,MADa,IAIzB,SAASC,OACP,MAAM,IAAID,MAAM,QAGlB,SAASE,gBACP,MAAM,IAAIF,MAAM,kBAMlB,SAASG,SACP,MAAM,IAAIH,MAAM,UAGlB,SAASI,QACP,MAAM,IAAIJ,MAAM\\",\\"file\\":\\"no-upstream-sourcemap.js\\"}",
"something.css": "#id {
background: white;
}",
"with-broken-sourcemap.js": "function meaningOfLife(){throw new Error(42)}
//# sourceMappingURL=with-broken-sourcemap.map",
"with-broken-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\"],\\"mappings\\":\\"AAAA,SAASA,gBAEP,MAAM,IAAIC,MADa\\",\\"file\\":\\"with-broken-sourcemap.js\\"}",
}
`;

exports[`broccoli-uglify-sourcemap on error rejects with BuildError 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap on error rejects with BuildError async 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap on error shuts down the workerpool 1`] = `Object {}`;

exports[`broccoli-uglify-sourcemap shuts down the workerpool 1`] = `
Object {
"inside": Object {
"with-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=with-upstream-sourcemap.map",
"with-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"/inner/first.js\\",\\"/inner/second.js\\",\\"/other/fourth.js\\",\\"/other/third.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AAAA,SAAAA,gBAEA,MAAA,IAAAC,MADA,IAIA,SAAAC,OACA,MAAA,IAAAD,MAAA,QCNA,SAAAE,gBACA,MAAA,IAAAF,MAAA,kBCEA,SAAAG,SACA,MAAA,IAAAH,MAAA,UCJA,SAAAI,QACA,MAAA,IAAAJ,MAAA\\",\\"file\\":\\"with-upstream-sourcemap.js\\",\\"sourcesContent\\":[\\"function meaningOfLife() {\\\\n var thisIsALongLocal = 42;\\\\n throw new Error(thisIsALongLocal);\\\\n}\\\\n\\\\nfunction boom() {\\\\n throw new Error('boom');\\\\n}\\\\n\\",\\"function somethingElse() {\\\\n throw new Error(\\\\\\"somethign else\\\\\\");\\\\n}\\\\n\\",\\"\\\\n// Hello world\\\\n\\\\nfunction fourth(){\\\\n throw new Error('fourth');\\\\n}\\\\n\\",\\"function third(){\\\\n throw new Error(\\\\\\"oh no\\\\\\");\\\\n}\\\\n\\"]}",
},
"no-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
//# sourceMappingURL=no-upstream-sourcemap.map",
"no-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AACA,SAASA,gBAEP,MAAM,IAAIC,MADa,IAIzB,SAASC,OACP,MAAM,IAAID,MAAM,QAGlB,SAASE,gBACP,MAAM,IAAIF,MAAM,kBAMlB,SAASG,SACP,MAAM,IAAIH,MAAM,UAGlB,SAASI,QACP,MAAM,IAAIJ,MAAM\\",\\"file\\":\\"no-upstream-sourcemap.js\\"}",
"something.css": "#id {
background: white;
}",
"with-broken-sourcemap.js": "function meaningOfLife(){throw new Error(42)}
//# sourceMappingURL=with-broken-sourcemap.map",
"with-broken-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\"],\\"mappings\\":\\"AAAA,SAASA,gBAEP,MAAM,IAAIC,MADa\\",\\"file\\":\\"with-broken-sourcemap.js\\"}",
}
`;

exports[`broccoli-uglify-sourcemap supports alternate sourcemap location 1`] = `
Object {
"inside": Object {
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures-error/no-upstream-sourcemap.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/* This file has an error. */
var i =

Loading