diff --git a/.gitignore b/.gitignore index e701029f9..88ece127c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ node_modules +node_modules.nosync dist .webpack .serverless diff --git a/README.md b/README.md index 017b2f5db..8d08df3ad 100644 --- a/README.md +++ b/README.md @@ -541,12 +541,13 @@ if you are trying to override the entry in webpack.config.js with other unsuppor The individual packaging needs more time at the packaging phase, but you'll get that paid back twice at runtime. -#### Individual packaging serializedCompile +#### Individual packaging concurrency ```yaml # serverless.yml custom: webpack: - serializedCompile: true + concurrency: 5 # desired concurrency, defaults to the number of available cores + serializedCompile: true # backward compatible, this translates to concurrency: 1 ``` Will run each webpack build one at a time which helps reduce memory usage and in some cases impoves overall build performance. diff --git a/lib/Configuration.js b/lib/Configuration.js index d2fea80c7..d14cdd2eb 100644 --- a/lib/Configuration.js +++ b/lib/Configuration.js @@ -4,6 +4,7 @@ */ const _ = require('lodash'); +const os = require('os'); /** * Plugin defaults @@ -15,7 +16,7 @@ const DefaultConfig = { packagerOptions: {}, keepOutputDirectory: false, config: null, - serializedCompile: false + concurrency: os.cpus().length }; class Configuration { @@ -38,6 +39,21 @@ class Configuration { } } + // Concurrency may be passed via CLI, e.g. + // custom: + // webpack: + // concurrency: ${opt:compile-concurrency, 7} + // In this case it is typed as a string and we have to validate it + if (this._config.concurrency !== undefined) { + this._config.concurrency = Number(this._config.concurrency); + if (isNaN(this._config.concurrency) || this._config.concurrency < 1) { + throw new Error('concurrency option must be a positive number'); + } + } else if (this._config.serializedCompile === true) { + // Backwards compatibility with serializedCompile setting + this._config.concurrency = 1; + } + // Set defaults for all missing properties _.defaults(this._config, DefaultConfig); } @@ -53,7 +69,7 @@ class Configuration { get excludeFiles() { return this._config.excludeFiles; } - + get excludeRegex() { return this._config.excludeRegex; } @@ -78,8 +94,8 @@ class Configuration { return this._config.keepOutputDirectory; } - get serializedCompile() { - return this._config.serializedCompile; + get concurrency() { + return this._config.concurrency; } toJSON() { diff --git a/lib/Configuration.test.js b/lib/Configuration.test.js index 4324b4bde..2ef1acdc1 100644 --- a/lib/Configuration.test.js +++ b/lib/Configuration.test.js @@ -3,6 +3,7 @@ * Unit tests for Configuration. */ +const os = require('os'); const chai = require('chai'); const Configuration = require('./Configuration'); @@ -20,7 +21,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - serializedCompile: false + concurrency: os.cpus().length }; }); @@ -70,7 +71,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - serializedCompile: false + concurrency: os.cpus().length }); }); }); @@ -91,7 +92,7 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - serializedCompile: false + concurrency: os.cpus().length }); }); @@ -111,8 +112,52 @@ describe('Configuration', () => { packagerOptions: {}, keepOutputDirectory: false, config: null, - serializedCompile: false + concurrency: os.cpus().length }); }); + + it('should accept a numeric string as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: '3' + } + }; + const config = new Configuration(testCustom); + expect(config.concurrency).to.equal(3); + }); + + it('should not accept an invalid string as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: '3abc' + } + }; + expect(() => new Configuration(testCustom)).throws(); + }); + + it('should not accept a non-positive number as concurrency value', () => { + const testCustom = { + webpack: { + includeModules: { forceInclude: ['mod1'] }, + webpackConfig: 'myWebpackFile.js', + concurrency: 0 + } + }; + expect(() => new Configuration(testCustom)).throws(); + }); + + it('should be backward compatible with serializedCompile', () => { + const testCustom = { + webpack: { + serializedCompile: true + } + }; + const config = new Configuration(testCustom); + expect(config.concurrency).to.equal(1); + }); }); }); diff --git a/lib/compile.js b/lib/compile.js index 683242f7a..cdb9b6d61 100644 --- a/lib/compile.js +++ b/lib/compile.js @@ -22,27 +22,25 @@ function getStatsLogger(statsConfig, consoleLog) { } function webpackCompile(config, logStats) { - return BbPromise - .fromCallback(cb => webpack(config).run(cb)) - .then(stats => { - // ensure stats in any array in the case of multiCompile - stats = stats.stats ? stats.stats : [stats]; - - _.forEach(stats, compileStats => { - logStats(compileStats); - if (compileStats.hasErrors()) { - throw new Error('Webpack compilation error, see stats above'); - } - }); - - return stats; + return BbPromise.fromCallback(cb => webpack(config).run(cb)).then(stats => { + // ensure stats in any array in the case of concurrent build. + stats = stats.stats ? stats.stats : [stats]; + + _.forEach(stats, compileStats => { + logStats(compileStats); + if (compileStats.hasErrors()) { + throw new Error('Webpack compilation error, see stats above'); + } }); + + return stats; + }); } -function webpackCompileSerial(configs, logStats) { - return BbPromise - .mapSeries(configs, config => webpackCompile(config, logStats)) - .then(stats => _.flatten(stats)); +function webpackConcurrentCompile(configs, logStats, concurrency) { + return BbPromise.map(configs, config => webpackCompile(config, logStats), { concurrency }).then(stats => + _.flatten(stats) + ); } module.exports = { @@ -52,10 +50,14 @@ module.exports = { const configs = ensureArray(this.webpackConfig); const logStats = getStatsLogger(configs[0].stats, this.serverless.cli.consoleLog); - return (this.serializedCompile ? webpackCompileSerial : webpackCompile)(configs, logStats) - .then(stats => { - this.compileStats = { stats }; - return BbPromise.resolve(); - }); + if (!this.configuration) { + return BbPromise.reject('Missing plugin configuration'); + } + const concurrency = this.configuration.concurrency; + + return webpackConcurrentCompile(configs, logStats, concurrency).then(stats => { + this.compileStats = { stats }; + return BbPromise.resolve(); + }); } }; diff --git a/lib/validate.js b/lib/validate.js index 393eefebd..8209ec8e2 100644 --- a/lib/validate.js +++ b/lib/validate.js @@ -189,11 +189,9 @@ module.exports = { // In case of individual packaging we have to create a separate config for each function if (_.has(this.serverless, 'service.package') && this.serverless.service.package.individually) { - this.multiCompile = true; - this.serializedCompile = this.configuration.serializedCompile; this.options.verbose && this.serverless.cli.log( - `Using ${this.serializedCompile ? 'serialized' : 'multi'}-compile (individual packaging)` + `Individually packaging with concurrency at ${this.configuration.concurrency} entries a time.` ); if (this.webpackConfig.entry && !_.isEqual(this.webpackConfig.entry, entries)) { diff --git a/package-lock.json b/package-lock.json index 0e6e99583..256566b84 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9573,6 +9573,14 @@ "util-deprecate": "^1.0.1" } }, + "readdir-glob": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/readdir-glob/-/readdir-glob-1.0.0.tgz", + "integrity": "sha512-km0DIcwQVZ1ZUhXhMWpF74/Wm5aFEd5/jDiVWF1Hkw2myPQovG8vCQ8+FQO2KXE9npQQvCnAMZhhWuUee4WcCQ==", + "requires": { + "minimatch": "^3.0.4" + } + }, "readdirp": { "version": "3.4.0", "resolved": "https://registry.npmjs.org/readdirp/-/readdirp-3.4.0.tgz", @@ -11045,17 +11053,6 @@ "integrity": "sha512-1apePfXM1UOSqw0o9IiFAovVz9M5S1Dg+4TrDwfMewQ6p/rmMueb7tWZjQ1rx4Loy1ArBggoqGpfqqdI4rondg==", "dev": true }, - "string-width": { - "version": "3.1.0", - "resolved": "https://registry.npmjs.org/string-width/-/string-width-3.1.0.tgz", - "integrity": "sha512-vafcv6KjVZKSgz06oM/H6GDBrAtz8vdhQakGjFIvNrHA6y3HCF1CInLy+QLq8dTJPQ1b+KDUqDFctkdRW44e1w==", - "dev": true, - "requires": { - "emoji-regex": "^7.0.1", - "is-fullwidth-code-point": "^2.0.0", - "strip-ansi": "^5.1.0" - } - }, "strip-ansi": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-5.2.0.tgz", diff --git a/tests/compile.test.js b/tests/compile.test.js index 58f682029..7e1ecd8b8 100644 --- a/tests/compile.test.js +++ b/tests/compile.test.js @@ -44,10 +44,13 @@ describe('compile', () => { consoleLog: sandbox.stub() }; - module = _.assign({ - serverless, - options: {}, - }, baseModule); + module = _.assign( + { + serverless, + options: {} + }, + baseModule + ); }); afterEach(() => { @@ -62,72 +65,82 @@ describe('compile', () => { it('should compile with webpack from a context configuration', () => { const testWebpackConfig = 'testconfig'; module.webpackConfig = testWebpackConfig; - return expect(module.compile()).to.be.fulfilled - .then(() => { - expect(webpackMock).to.have.been.calledWith([testWebpackConfig]); + module.configuration = { concurrency: 1 }; + return expect(module.compile()).to.be.fulfilled.then(() => { + expect(webpackMock).to.have.been.calledWith(testWebpackConfig); expect(webpackMock.compilerMock.run).to.have.been.calledOnce; return null; }); }); + it('should fail if configuration is missing', () => { + const testWebpackConfig = 'testconfig'; + module.webpackConfig = testWebpackConfig; + module.configuration = undefined; + return expect(module.compile()).to.be.rejectedWith('Missing plugin configuration'); + }); + it('should fail if there are compilation errors', () => { module.webpackConfig = 'testconfig'; + module.configuration = { concurrency: 1 }; // We stub errors here. It will be reset again in afterEach() sandbox.stub(webpackMock.statsMock.compilation, 'errors').value(['error']); return expect(module.compile()).to.be.rejectedWith(/compilation error/); }); it('should work with multi compile', () => { - const testWebpackConfig = ['testconfig']; + const testWebpackConfig = 'testconfig'; const multiStats = { - stats: [{ - compilation: { - errors: [], - compiler: { - outputPath: 'statsMock-outputPath', + stats: [ + { + compilation: { + errors: [], + compiler: { + outputPath: 'statsMock-outputPath' + } }, - }, - toString: sandbox.stub().returns('testStats'), - hasErrors: _.constant(false) - }] + toString: sandbox.stub().returns('testStats'), + hasErrors: _.constant(false) + } + ] }; module.webpackConfig = testWebpackConfig; - module.multiCompile = true; + module.configuration = { concurrency: 1 }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, multiStats); - return expect(module.compile()).to.be.fulfilled - .then(() => { + return expect(module.compile()).to.be.fulfilled.then(() => { expect(webpackMock).to.have.been.calledWith(testWebpackConfig); expect(webpackMock.compilerMock.run).to.have.been.calledOnce; return null; }); }); - it('should work with serialized compile', () => { - const testWebpackConfig = ['testconfig']; + it('should work with concurrent compile', () => { + const testWebpackConfig = [ 'testconfig', 'testconfig2' ]; const multiStats = { - stats: [{ - compilation: { - errors: [], - compiler: { - outputPath: 'statsMock-outputPath', + stats: [ + { + compilation: { + errors: [], + compiler: { + outputPath: 'statsMock-outputPath' + } }, - }, - toString: sandbox.stub().returns('testStats'), - hasErrors: _.constant(false) - }] + toString: sandbox.stub().returns('testStats'), + hasErrors: _.constant(false) + } + ] }; module.webpackConfig = testWebpackConfig; - module.multiCompile = true; - module.serializedCompile = true; + module.configuration = { concurrency: 2 }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, multiStats); - return expect(module.compile()).to.be.fulfilled - .then(() => { - expect(webpackMock).to.have.been.calledWith(testWebpackConfig); - expect(webpackMock.compilerMock.run).to.have.been.calledOnce; - return null; - }); + return expect(module.compile()).to.be.fulfilled.then(() => { + expect(webpackMock).to.have.been.calledWith(testWebpackConfig[0]); + expect(webpackMock).to.have.been.calledWith(testWebpackConfig[1]); + expect(webpackMock.compilerMock.run).to.have.been.calledTwice; + return null; + }); }); it('should use correct stats option', () => { @@ -146,19 +159,20 @@ describe('compile', () => { }; module.webpackConfig = testWebpackConfig; + module.configuration = { concurrency: 1 }; webpackMock.compilerMock.run.reset(); webpackMock.compilerMock.run.yields(null, mockStats); - return (expect(module.compile()).to.be.fulfilled) - .then(() => { - expect(webpackMock).to.have.been.calledWith([testWebpackConfig]); - expect(mockStats.toString.firstCall.args).to.eql([testWebpackConfig.stats]); - module.webpackConfig = [testWebpackConfig]; - return (expect(module.compile()).to.be.fulfilled); - }) - .then(() => { - expect(webpackMock).to.have.been.calledWith([testWebpackConfig]); - expect(mockStats.toString.args).to.eql([ [testWebpackConfig.stats], [testWebpackConfig.stats] ]); - return null; - }); + return expect(module.compile()) + .to.be.fulfilled.then(() => { + expect(webpackMock).to.have.been.calledWith(testWebpackConfig); + expect(mockStats.toString.firstCall.args).to.eql([testWebpackConfig.stats]); + module.webpackConfig = [testWebpackConfig]; + return expect(module.compile()).to.be.fulfilled; + }) + .then(() => { + expect(webpackMock).to.have.been.calledWith(testWebpackConfig); + expect(mockStats.toString.args).to.eql([ [testWebpackConfig.stats], [testWebpackConfig.stats] ]); + return null; + }); }); }); diff --git a/tests/packageModules.test.js b/tests/packageModules.test.js index d9c0329f8..32fc6ae83 100644 --- a/tests/packageModules.test.js +++ b/tests/packageModules.test.js @@ -158,7 +158,6 @@ describe('packageModules', () => { fsMock._statMock.isDirectory.returns(false); module.compileStats = stats; - return expect(module.packageModules()).to.be.fulfilled.then(() => BbPromise.all([])); }); @@ -485,12 +484,8 @@ describe('packageModules', () => { expect(fsMock.copyFileSync).to.be.calledWith(expectedArtifactSource, expectedArtifactDestination), // Should set package artifact for each function to the single artifact - expect(func1) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedArtifactDestination), - expect(func2) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedArtifactDestination) + expect(func1).to.have.a.nested.property('package.artifact').that.equals(expectedArtifactDestination), + expect(func2).to.have.a.nested.property('package.artifact').that.equals(expectedArtifactDestination) ]) ); }); @@ -539,12 +534,8 @@ describe('packageModules', () => { getVersionStub.returns(version); return expect(module.copyExistingArtifacts()).to.be.fulfilled.then(() => BbPromise.all([ - expect(func1) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedArtifactPath), - expect(func2) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedArtifactPath) + expect(func1).to.have.a.nested.property('package.artifact').that.equals(expectedArtifactPath), + expect(func2).to.have.a.nested.property('package.artifact').that.equals(expectedArtifactPath) ]) ); }).then(() => @@ -552,12 +543,8 @@ describe('packageModules', () => { getVersionStub.returns(version); return expect(module.copyExistingArtifacts()).to.be.fulfilled.then(() => BbPromise.all([ - expect(func1) - .to.have.a.nested.property('artifact') - .that.equals(expectedArtifactPath), - expect(func2) - .to.have.a.nested.property('artifact') - .that.equals(expectedArtifactPath), + expect(func1).to.have.a.nested.property('artifact').that.equals(expectedArtifactPath), + expect(func2).to.have.a.nested.property('artifact').that.equals(expectedArtifactPath), expect(func1).to.have.a.nested.property('package.disable').that.is.true, expect(func2).to.have.a.nested.property('package.disable').that.is.true ]) @@ -610,9 +597,7 @@ describe('packageModules', () => { const expectedArtifactPath = path.join('.serverless', 'test-service.zip'); return expect(module.copyExistingArtifacts()).to.be.fulfilled.then(() => - expect(serverless.service) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedArtifactPath) + expect(serverless.service).to.have.a.nested.property('package.artifact').that.equals(expectedArtifactPath) ); }); }); @@ -647,12 +632,8 @@ describe('packageModules', () => { expect(fsMock.copyFileSync).to.be.calledWith(path.join('.webpack', 'func2.zip'), expectedFunc2Destination), // Should set package artifact locations - expect(func1) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedFunc1Destination), - expect(func2) - .to.have.a.nested.property('package.artifact') - .that.equals(expectedFunc2Destination) + expect(func1).to.have.a.nested.property('package.artifact').that.equals(expectedFunc1Destination), + expect(func2).to.have.a.nested.property('package.artifact').that.equals(expectedFunc2Destination) ]) ); }); diff --git a/tests/validate.test.js b/tests/validate.test.js index a99da798d..ee4210b99 100644 --- a/tests/validate.test.js +++ b/tests/validate.test.js @@ -686,19 +686,6 @@ describe('validate', () => { _.unset(module.serverless, 'service.package.individually'); }); - it('should enable multiCompile', () => { - _.set(module.serverless.service, 'custom.webpack.config', testConfig); - module.serverless.service.functions = testFunctionsConfig; - globSyncStub.callsFake(filename => [_.replace(filename, '*', 'js')]); - - expect(module.multiCompile).to.be.undefined; - return expect(module.validate()).to.be.fulfilled.then(() => { - expect(module.multiCompile).to.be.true; - - return null; - }); - }); - it('should fail if webpackConfig.entry is customised', () => { _.set( module.serverless.service,