Repository: serverless-heaven/serverless-webpack
Pull request #681 (2021-01-13 04:23:02)

Add concurrency support for more than one thread
<!--
1. Do not remove any section of the template. If something is not applicable leave it empty but leave it in the PR
2. Please follow the template, otherwise we'll have to ask you to update it and it will take longer until your PR is merged
-->

## What did you implement:

Adding concurrency support for more than one thread.

Conditionally supersedes #680 if `release/5.4.0` is more preferable than `master`.

<!--
Briefly describe the feature if no issue exists for this PR. If possible only
submit PRs for existing issues. If the PR is trivial (like doc changes or simple
code fixes) it can be submitted without a related issue, but as soon as it adds
or changes functionality, a related issue should be present.
-->

## How did you implement it:

Please see https://github.com/serverless-heaven/serverless-webpack/pull/517#issuecomment-653547396, basically I did two changes:

1. Replacing `serizliedCompile: Boolean` with `concurrency: Number`
2. Translates `serializedCompile: true` to `concurrency: 1` for backward compatibility

<!--
If this is a nontrivial change please briefly describe your implementation so its easy for us to understand and review your code.
-->

## How can we verify it:

<!--
Add any applicable config, commands, screenshots or other resources
to make it easy for us to verify this works. The easier you make it for us
to review a PR, the faster we can review and merge it.

Examples:
* Step by step description, how to verify
* Screenshots - Showing the difference between your output and the master
* Other - Anything else that comes to mind to help us evaluate
-->

Use the config below when packaging:

```
package:
  individually: true

custom:
  webpack:
    concurrency: 5
```

## Todos:

- [x] Write tests
- [x] Write documentation
- [x] Fix linting errors
- [x] Make sure code coverage hasn't dropped
- [x] Provide verification config / commands / resources
- [x] Enable "Allow edits from maintainers" for this PR
- [x] Update the messages below

***Is this ready for review?:*** Yes
***Is it a breaking change?:*** No


coyoteecd: @miguel-a-calles-mba @j0k3r can you have a look and merge this? I am using vicary's fork for half a year and would love to switch back to the official package.
j0k3r: @coyoteecd we are in the process of cleaning up stuff to release 5.4.0 and this will be part of it
j0k3r: Sorry I made a mistake while merging `release/5.4.0` into master and removed it. The removal automatically closed that PR. It should now be rebased against the master and fix conflicts.
vicary: Not sure if I'm doing it wrong, but I found myself learning the backstory every conflict or two in the process of rebasing. This takes a day or two, could only do this on weekends.

Patch:
diff --git a/.gitignore b/.gitignore
index e701029..88ece12 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 017b2f5..8d08df3 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 d2fea80..d14cdd2 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/compile.js b/lib/compile.js
index 683242f..cdb9b6d 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 393eefe..8209ec8 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 0e6e995..256566b 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",


Test patch:
diff --git a/lib/Configuration.test.js b/lib/Configuration.test.js
index 4324b4b..2ef1acd 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/tests/compile.test.js b/tests/compile.test.js
index 58f6820..7e1ecd8 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 d9c0329..32fc6ae 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 a99da79..ee4210b 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,