Repository: serverless-heaven/serverless-webpack
Pull request #858 (2021-06-05 01:56:00)

Reduce memory usage by releasing webpack stats objects after compile
## What did you implement:

The current implementation keeps the full webpack stats object for all functions. This object is huge and causes very large memory usage when using package: individually and a large number of functions.

The webpack stats object is used for 2 things, outputPath and external modules. Instead of keeping the full stats object around, we calculate the outputPath and external modules after compilation and discard the stats object. Then packageModules and packageExternalModules can use these values directly instead of the stats object.

## How did you implement it:

Set outputPath and externalModules in the compile phase and discard webpack stats object.

## How can we verify it:

Tested this change in a large serverless app.

Before: Memory usage slowly increases up to 7.2gb, builds in ~1500s
After: Memory usage hovers around 1.5gb, builds in ~1000s

## Todos:

- [x] Write tests
- [ ] Write documentation
- [x] Fix linting errors
- [ ] Make sure code coverage hasn't dropped
- [ ] 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


janicduplessis: Accomplish something similar to #570, but simpler.

Should fix #299
j0k3r: This looks interesting, I'll try out on some project and let you know!
In the mean time, could you rebase against the master? Thanks 🙏

Patch:
diff --git a/lib/compile.js b/lib/compile.js
index faa1b9a..1818ddd 100644
--- a/lib/compile.js
+++ b/lib/compile.js
@@ -4,6 +4,7 @@ const _ = require('lodash');
 const BbPromise = require('bluebird');
 const webpack = require('webpack');
 const tty = require('tty');
+const isBuiltinModule = require('is-builtin-module');
 
 const defaultStatsConfig = {
   colors: tty.isatty(process.stdout.fd),
@@ -26,6 +27,64 @@ function getStatsLogger(statsConfig, consoleLog) {
   };
 }
 
+function getExternalModuleName(module) {
+  const path = /^external "(.*)"$/.exec(module.identifier())[1];
+  const pathComponents = path.split('/');
+  const main = pathComponents[0];
+
+  // this is a package within a namespace
+  if (main.charAt(0) == '@') {
+    return `${main}/${pathComponents[1]}`;
+  }
+
+  return main;
+}
+
+function isExternalModule(module) {
+  return _.startsWith(module.identifier(), 'external ') && !isBuiltinModule(getExternalModuleName(module));
+}
+
+/**
+ * Gets the module issuer. The ModuleGraph api does not exists in webpack@4
+ * so falls back to using module.issuer.
+ */
+function getIssuerCompat(moduleGraph, module) {
+  if (moduleGraph) {
+    return moduleGraph.getIssuer(module);
+  }
+
+  return module.issuer;
+}
+
+/**
+ * Find the original module that required the transient dependency. Returns
+ * undefined if the module is a first level dependency.
+ * @param {Object} moduleGraph - Webpack module graph
+ * @param {Object} issuer - Module issuer
+ */
+function findExternalOrigin(moduleGraph, issuer) {
+  if (!_.isNil(issuer) && _.startsWith(issuer.rawRequest, './')) {
+    return findExternalOrigin(moduleGraph, getIssuerCompat(moduleGraph, issuer));
+  }
+  return issuer;
+}
+
+function getExternalModules({ compilation }) {
+  const externals = new Set();
+  for (const module of compilation.modules) {
+    if (isExternalModule(module)) {
+      externals.add({
+        origin: _.get(
+          findExternalOrigin(compilation.moduleGraph, getIssuerCompat(compilation.moduleGraph, module)),
+          'rawRequest'
+        ),
+        external: getExternalModuleName(module)
+      });
+    }
+  }
+  return Array.from(externals);
+}
+
 function webpackCompile(config, logStats) {
   return BbPromise.fromCallback(cb => webpack(config).run(cb)).then(stats => {
     // ensure stats in any array in the case of concurrent build.
@@ -38,7 +97,10 @@ function webpackCompile(config, logStats) {
       }
     });
 
-    return stats;
+    return _.map(stats, compileStats => ({
+      outputPath: compileStats.compilation.compiler.outputPath,
+      externalModules: getExternalModules(compileStats)
+    }));
   });
 }
 
diff --git a/lib/packExternalModules.js b/lib/packExternalModules.js
index f45f6a6..85477bf 100644
--- a/lib/packExternalModules.js
+++ b/lib/packExternalModules.js
@@ -4,7 +4,6 @@ const BbPromise = require('bluebird');
 const _ = require('lodash');
 const path = require('path');
 const fse = require('fs-extra');
-const isBuiltinModule = require('is-builtin-module');
 
 const Packagers = require('./packagers');
 
@@ -178,64 +177,6 @@ function getProdModules(externalModules, packagePath, nodeModulesRelativeDir, de
   return prodModules;
 }
 
-function getExternalModuleName(module) {
-  const path = /^external "(.*)"$/.exec(module.identifier())[1];
-  const pathComponents = path.split('/');
-  const main = pathComponents[0];
-
-  // this is a package within a namespace
-  if (main.charAt(0) == '@') {
-    return `${main}/${pathComponents[1]}`;
-  }
-
-  return main;
-}
-
-function isExternalModule(module) {
-  return _.startsWith(module.identifier(), 'external ') && !isBuiltinModule(getExternalModuleName(module));
-}
-
-/**
- * Gets the module issuer. The ModuleGraph api does not exists in webpack@4
- * so falls back to using module.issuer.
- */
-function getIssuerCompat(moduleGraph, module) {
-  if (moduleGraph) {
-    return moduleGraph.getIssuer(module);
-  }
-
-  return module.issuer;
-}
-
-/**
- * Find the original module that required the transient dependency. Returns
- * undefined if the module is a first level dependency.
- * @param {Object} moduleGraph - Webpack module graph
- * @param {Object} issuer - Module issuer
- */
-function findExternalOrigin(moduleGraph, issuer) {
-  if (!_.isNil(issuer) && _.startsWith(issuer.rawRequest, './')) {
-    return findExternalOrigin(moduleGraph, getIssuerCompat(moduleGraph, issuer));
-  }
-  return issuer;
-}
-
-function getExternalModules({ compilation }) {
-  const externals = new Set();
-  for (const module of compilation.modules) {
-    if (isExternalModule(module)) {
-      externals.add({
-        origin: _.get(
-          findExternalOrigin(compilation.moduleGraph, getIssuerCompat(compilation.moduleGraph, module)),
-          'rawRequest'
-        ),
-        external: getExternalModuleName(module)
-      });
-    }
-  }
-  return Array.from(externals);
-}
-
 module.exports = {
   /**
    * We need a performant algorithm to install the packages for each single
@@ -305,7 +246,7 @@ module.exports = {
         const compositeModules = _.uniq(
           _.flatMap(stats.stats, compileStats => {
             const externalModules = _.concat(
-              getExternalModules.call(this, compileStats),
+              compileStats.externalModules,
               _.map(packageForceIncludes, whitelistedPackage => ({
                 external: whitelistedPackage
               }))
@@ -381,7 +322,7 @@ module.exports = {
               .return(stats.stats);
           })
           .mapSeries(compileStats => {
-            const modulePath = compileStats.compilation.compiler.outputPath;
+            const modulePath = compileStats.outputPath;
 
             // Create package.json
             const modulePackageJson = path.join(modulePath, 'package.json');
@@ -399,7 +340,7 @@ module.exports = {
             const prodModules = getProdModules.call(
               this,
               _.concat(
-                getExternalModules.call(this, compileStats),
+                compileStats.externalModules,
                 _.map(packageForceIncludes, whitelistedPackage => ({
                   external: whitelistedPackage
                 }))
diff --git a/lib/packageModules.js b/lib/packageModules.js
index a28577f..70d0397 100644
--- a/lib/packageModules.js
+++ b/lib/packageModules.js
@@ -131,7 +131,7 @@ module.exports = {
     return BbPromise.mapSeries(stats.stats, (compileStats, index) => {
       const entryFunction = _.get(this.entryFunctions, index, {});
       const filename = getArtifactName.call(this, entryFunction);
-      const modulePath = compileStats.compilation.compiler.outputPath;
+      const modulePath = compileStats.outputPath;
 
       const startZip = _.now();
       return zip


Test patch:
diff --git a/tests/compile.test.js b/tests/compile.test.js
index 7e1ecd8..b9134cf 100644
--- a/tests/compile.test.js
+++ b/tests/compile.test.js
@@ -97,7 +97,8 @@ describe('compile', () => {
             errors: [],
             compiler: {
               outputPath: 'statsMock-outputPath'
-            }
+            },
+            modules: []
           },
           toString: sandbox.stub().returns('testStats'),
           hasErrors: _.constant(false)
@@ -124,7 +125,8 @@ describe('compile', () => {
             errors: [],
             compiler: {
               outputPath: 'statsMock-outputPath'
-            }
+            },
+            modules: []
           },
           toString: sandbox.stub().returns('testStats'),
           hasErrors: _.constant(false)
@@ -152,7 +154,8 @@ describe('compile', () => {
         errors: [],
         compiler: {
           outputPath: 'statsMock-outputPath'
-        }
+        },
+        modules: []
       },
       toString: sandbox.stub().returns('testStats'),
       hasErrors: _.constant(false)
@@ -175,4 +178,93 @@ describe('compile', () => {
         return null;
       });
   });
+
+  it('should set stats outputPath', () => {
+    const testWebpackConfig = 'testconfig';
+    const multiStats = {
+      stats: [
+        {
+          compilation: {
+            errors: [],
+            compiler: {
+              outputPath: 'compileStats-outputPath'
+            },
+            modules: []
+          },
+          toString: sandbox.stub().returns('testStats'),
+          hasErrors: _.constant(false)
+        }
+      ]
+    };
+    module.webpackConfig = testWebpackConfig;
+    module.configuration = { concurrency: 1 };
+    webpackMock.compilerMock.run.reset();
+    webpackMock.compilerMock.run.yields(null, multiStats);
+    return expect(module.compile()).to.be.fulfilled.then(() => {
+      expect(module.compileStats.stats[0].outputPath).to.equal('compileStats-outputPath');
+      return null;
+    });
+  });
+
+  it('should set stats externals', () => {
+    const testWebpackConfig = 'testconfig';
+    const multiStats = {
+      stats: [
+        {
+          compilation: {
+            errors: [],
+            compiler: {
+              outputPath: 'compileStats-outputPath'
+            },
+            modules: [
+              {
+                identifier: _.constant('"crypto"')
+              },
+              {
+                identifier: _.constant('"uuid/v4"')
+              },
+              {
+                identifier: _.constant('"mockery"')
+              },
+              {
+                identifier: _.constant('"@scoped/vendor/module1"')
+              },
+              {
+                identifier: _.constant('external "@scoped/vendor/module2"')
+              },
+              {
+                identifier: _.constant('external "uuid/v4"')
+              },
+              {
+                identifier: _.constant('external "localmodule"')
+              },
+              {
+                identifier: _.constant('external "bluebird"')
+              },
+              {
+                identifier: _.constant('external "aws-sdk"')
+              }
+            ]
+          },
+          toString: sandbox.stub().returns('testStats'),
+          hasErrors: _.constant(false)
+        }
+      ]
+    };
+    module.webpackConfig = testWebpackConfig;
+    module.configuration = { concurrency: 1 };
+    webpackMock.compilerMock.run.reset();
+    webpackMock.compilerMock.run.yields(null, multiStats);
+    return expect(module.compile()).to.be.fulfilled.then(() => {
+      console.log(JSON.stringify(module.compileStats.stats[0].externalModules));
+      expect(module.compileStats.stats[0].externalModules).to.eql([
+        { external: '@scoped/vendor', origin: undefined },
+        { external: 'uuid', origin: undefined },
+        { external: 'localmodule', origin: undefined },
+        { external: 'bluebird', origin: undefined },
+        { external: 'aws-sdk', origin: undefined }
+      ]);
+      return null;
+    });
+  });
 });
diff --git a/tests/packExternalModules.test.js b/tests/packExternalModules.test.js
index 70c2397..57af3c5 100644
--- a/tests/packExternalModules.test.js
+++ b/tests/packExternalModules.test.js
@@ -20,21 +20,16 @@ chai.use(require('sinon-chai'));
 
 const expect = chai.expect;
 
-class WebpackModuleGraphMock {
-  getIssuer(module) {
-    return module.issuer;
-  }
-}
-
-class WebpackCompilationMock {
-  constructor(modules) {
-    this.moduleGraph = new WebpackModuleGraphMock();
-    this.modules = modules;
-    this.compiler = {
-      outputPath: '/my/Service/Path/.webpack/service'
-    };
-  }
-}
+const createStatsMock = modules => ({
+  stats: [
+    {
+      outputPath: '/my/Service/Path/.webpack/service',
+      externalModules: _.map(modules, m => ({
+        external: m
+      }))
+    }
+  ]
+});
 
 const packagerMockFactory = {
   create(sandbox) {
@@ -135,157 +130,23 @@ describe('packExternalModules', () => {
 
   describe('packExternalModules()', () => {
     // Test data
-    const stats = {
-      stats: [
-        {
-          compilation: new WebpackCompilationMock([
-            {
-              identifier: _.constant('"crypto"')
-            },
-            {
-              identifier: _.constant('"uuid/v4"')
-            },
-            {
-              identifier: _.constant('"mockery"')
-            },
-            {
-              identifier: _.constant('"@scoped/vendor/module1"')
-            },
-            {
-              identifier: _.constant('external "@scoped/vendor/module2"')
-            },
-            {
-              identifier: _.constant('external "uuid/v4"')
-            },
-            {
-              identifier: _.constant('external "bluebird"')
-            }
-          ])
-        }
-      ]
-    };
+    const stats = createStatsMock([ '@scoped/vendor', 'uuid', 'bluebird' ]);
     const noExtStats = {
       stats: [
         {
-          compilation: new WebpackCompilationMock([
-            {
-              identifier: _.constant('"crypto"')
-            },
-            {
-              identifier: _.constant('"uuid/v4"')
-            },
-            {
-              identifier: _.constant('"mockery"')
-            },
-            {
-              identifier: _.constant('"@scoped/vendor/module1"')
-            }
-          ])
-        }
-      ]
-    };
-    const statsWithFileRef = {
-      stats: [
-        {
-          compilation: new WebpackCompilationMock([
-            {
-              identifier: _.constant('"crypto"')
-            },
-            {
-              identifier: _.constant('"uuid/v4"')
-            },
-            {
-              identifier: _.constant('"mockery"')
-            },
-            {
-              identifier: _.constant('"@scoped/vendor/module1"')
-            },
-            {
-              identifier: _.constant('external "@scoped/vendor/module2"')
-            },
-            {
-              identifier: _.constant('external "uuid/v4"')
-            },
-            {
-              identifier: _.constant('external "localmodule"')
-            },
-            {
-              identifier: _.constant('external "bluebird"')
-            }
-          ])
-        }
-      ]
-    };
-    const statsWithDevDependency = {
-      stats: [
-        {
-          compilation: new WebpackCompilationMock([
-            {
-              identifier: _.constant('"crypto"')
-            },
-            {
-              identifier: _.constant('"uuid/v4"')
-            },
-            {
-              identifier: _.constant('external "eslint"')
-            },
-            {
-              identifier: _.constant('"mockery"')
-            },
-            {
-              identifier: _.constant('"@scoped/vendor/module1"')
-            },
-            {
-              identifier: _.constant('external "@scoped/vendor/module2"')
-            },
-            {
-              identifier: _.constant('external "uuid/v4"')
-            },
-            {
-              identifier: _.constant('external "localmodule"')
-            },
-            {
-              identifier: _.constant('external "bluebird"')
-            }
-          ])
-        }
-      ]
-    };
-    const statsWithIgnoredDevDependency = {
-      stats: [
-        {
-          compilation: new WebpackCompilationMock([
-            {
-              identifier: _.constant('"crypto"')
-            },
-            {
-              identifier: _.constant('"uuid/v4"')
-            },
-            {
-              identifier: _.constant('"mockery"')
-            },
-            {
-              identifier: _.constant('"@scoped/vendor/module1"')
-            },
-            {
-              identifier: _.constant('external "@scoped/vendor/module2"')
-            },
-            {
-              identifier: _.constant('external "uuid/v4"')
-            },
-            {
-              identifier: _.constant('external "localmodule"')
-            },
-            {
-              identifier: _.constant('external "bluebird"')
-            },
-            {
-              identifier: _.constant('external "aws-sdk"')
-            }
-          ])
+          externalModules: []
         }
       ]
     };
+    const statsWithFileRef = createStatsMock([ '@scoped/vendor', 'uuid', 'localmodule', 'bluebird' ]);
+    const statsWithDevDependency = createStatsMock([ 'eslint', '@scoped/vendor', 'uuid', 'localmodule', 'bluebird' ]);
+    const statsWithIgnoredDevDependency = createStatsMock([
+      '@scoped/vendor',
+      'uuid',
+      'localmodule',
+      'bluebird',
+      'aws-sdk'
+    ]);
 
     it('should do nothing if webpackIncludeModules is not set', () => {
       module.configuration = new Configuration();
@@ -1146,32 +1007,7 @@ describe('packExternalModules', () => {
           };
 
           const dependencyGraph = require('./data/npm-ls-peerdeps.json');
-          const peerDepStats = {
-            stats: [
-              {
-                compilation: new WebpackCompilationMock([
-                  {
-                    identifier: _.constant('"crypto"')
-                  },
-                  {
-                    identifier: _.constant('"uuid/v4"')
-                  },
-                  {
-                    identifier: _.constant('"mockery"')
-                  },
-                  {
-                    identifier: _.constant('"@scoped/vendor/module1"')
-                  },
-                  {
-                    identifier: _.constant('external "bluebird"')
-                  },
-                  {
-                    identifier: _.constant('external "request-promise"')
-                  }
-                ])
-              }
-            ]
-          };
+          const peerDepStats = createStatsMock([ 'bluebird', 'request-promise' ]);
 
           module.webpackOutputPath = 'outputPath';
           fsExtraMock.pathExists.yields(null, false);
@@ -1228,32 +1064,7 @@ describe('packExternalModules', () => {
         };
 
         const dependencyGraph = require('./data/npm-ls-peerdeps.json');
-        const peerDepStats = {
-          stats: [
-            {
-              compilation: new WebpackCompilationMock([
-                {
-                  identifier: _.constant('"crypto"')
-                },
-                {
-                  identifier: _.constant('"uuid/v4"')
-                },
-                {
-                  identifier: _.constant('"mockery"')
-                },
-                {
-                  identifier: _.constant('"@scoped/vendor/module1"')
-                },
-                {
-                  identifier: _.constant('external "bluebird"')
-                },
-                {
-                  identifier: _.constant('external "request-promise"')
-                }
-              ])
-            }
-          ]
-        };
+        const peerDepStats = createStatsMock([ 'bluebird', 'request-promise' ]);
 
         describe('without nodeModulesRelativeDir', () => {
           before(() => {
@@ -1402,17 +1213,7 @@ describe('packExternalModules', () => {
           }
         };
 
-        const transitiveDepStats = {
-          stats: [
-            {
-              compilation: new WebpackCompilationMock([
-                {
-                  identifier: _.constant('external "classnames"')
-                }
-              ])
-            }
-          ]
-        };
+        const transitiveDepStats = createStatsMock(['classnames']);
 
         module.webpackOutputPath = 'outputPath';
         fsExtraMock.pathExists.yields(null, false);
diff --git a/tests/packageModules.test.js b/tests/packageModules.test.js
index d733bbf..c7040aa 100644
--- a/tests/packageModules.test.js
+++ b/tests/packageModules.test.js
@@ -126,11 +126,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
@@ -184,11 +180,7 @@ describe('packageModules', () => {
           const stats = {
             stats: [
               {
-                compilation: {
-                  compiler: {
-                    outputPath: '/my/Service/Path/.webpack/service'
-                  }
-                }
+                outputPath: '/my/Service/Path/.webpack/service'
               }
             ]
           };
@@ -225,11 +217,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
@@ -273,11 +261,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
@@ -320,11 +304,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
@@ -368,11 +348,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
@@ -410,18 +386,10 @@ describe('packageModules', () => {
       const stats = {
         stats: [
           {
-            compilation: {
-              compiler: {
-                outputPath: '/my/Service/Path/.webpack/func1'
-              }
-            }
+            outputPath: '/my/Service/Path/.webpack/func1'
           },
           {
-            compilation: {
-              compiler: {
-                outputPath: '/my/Service/Path/.webpack/func2'
-              }
-            }
+            outputPath: '/my/Service/Path/.webpack/func2'
           }
         ]
       };
@@ -539,11 +507,7 @@ describe('packageModules', () => {
         const stats = {
           stats: [
             {
-              compilation: {
-                compiler: {
-                  outputPath: '/my/Service/Path/.webpack/service'
-                }
-              }
+              outputPath: '/my/Service/Path/.webpack/service'
             }
           ]
         };
diff --git a/tests/webpack.mock.js b/tests/webpack.mock.js
index 23e2651..9c0d819 100644
--- a/tests/webpack.mock.js
+++ b/tests/webpack.mock.js
@@ -7,7 +7,8 @@ const StatsMock = () => ({
     errors: [],
     compiler: {
       outputPath: 'statsMock-outputPath'
-    }
+    },
+    modules: []
   },
   toString: sinon.stub().returns('testStats'),
   hasErrors() {