serverless-webpack
serverless-webpack copied to clipboard
serverless-heaven
New: Use woker-farm to handle webpack multi compile
What did you implement:
Memory usage of this plugin is really high when using package individually. This is because we store the webpack stats/results from each function compile until all functions are finished compiling. These stats object contain all of the compiled source code for each module and chunk generated. We don't use many properties from webpack stats and this change adds a new function processWebpackStats() that only extracts the properties we need (cliOutput, errors, outputPath, and externalModules). This greatly reduces the memory footprint during compile and lets the garbage collector clean up a lot more memory.
Lowering the memory improves performance a little bit but there's still a chance of a memory leak if you are using many webpack plugins that do not cleanup properly. To fix this we must ensure memory is cleaned up after every function compile. To do this I added the npm package worker-farm that runs each function compile in a separate thread/process. This way we can end the process and clean up the memory after each compilation is done. This also lets us compile multiple functions at once and has a huge performance improvement. Almost 50% faster in some repositories I have tested this on. This is the same pattern that parallel-webpack uses to improve performance of webpack multi compile.
Closes #299
How did you implement it:
Refactored the code to create 2 different compilers. Single compiler that uses webpack() in main thread, same as current implementation. Multi compiler uses worker-farm to run each webpack compilation in a seperate thread. Since each worker is its own process we can only pass in serializable objects into the thread. This means we can pass webpack config file path or a JSON object that does not have functions or other non serializable objects. We use the V8 serialize function to test if a webpack config can be serialized using the HTML structured clone algorithm. This might be a breaking change but I don't think its possible to include a non serializable object into a serverless config. If it is possible it will throw an error letting the user know they should use webpack config file path instead.
How can we verify it:
- Enable package individually
- Run a build or deploy
- Ensure every function is compiled correclty
Todos:
- [X] Write tests
- [ ] 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
Its failing since we build on NodeJS 6. V8 serialize function was added in NodeJS 8. I think we should bump up min version to NodeJS10 since 6 has been deprecated for a while. Let me know if that is ok.
@miguel-a-calles-mba I agree with @soda0289, we should bump minimum version of node to 10. We should only have build & support for:
- Node 10
- Node 12
AWS disabled ability to update functions running:
- Node.js 6.10 since May 30 2019
- Node.js 8.10 since February 3, 2020
@HyperBrain, may you please update the Travis CI and AppVeyor configurations to test against Node 10 and 12, please? Thanks!
@miguel-a-calles-mba you can do it on your own by editing .travis.yml and .appveyor.yml.
@soda0289 May you please merge master into your fork? I'm not able to do it for you. The latest version of master now runs unit tests against Node 8 and 10.
Nevermind @soda0289. I figured out how to update your PR to update CI node version.
@soda0289 May you fix the unit tests? I addressed the linting issues, but the test are still failing. Thanks.
@miguel-a-calles-mba Sorry for delayed response. I have fixed up the linting errors and test cases.
Another question I have is if we should add options to allow the user to customize how many threads they want to use or if they want to use multi thread compile at all. Currently it is used every time package individually is set but this might be undesired for some users.
Also should I reabse onto branch 'release/5.4.0' or should I change base branch to be master?
@soda0289 Maybe there should be a default value of one thread?
We are targeting release/5.4.0.
Sane thread default is probably the number of CPU cores (as an example from another ecosystem, I believe that's the default for python's multiprocessing module - using what's returned by multiprocessing.cpu_count())
Having it be configurable would obviously be nice.
@soda0289 @miguel-a-calles-mba Any chance to get this fix in? The plugin is becoming nearly unusable for us
@coyoteecd, we are working to get 5.3.3 released and will start on 5.4.0.
You can install this PR in the meantime.
npm install serverless-heaven/serverless-webpack#pull/570/head
@soda0289 @miguel-a-calles-mba I did a dry-run of this pull request in our environment and hit an issue.
Previously, the plugin exposed entries and options through slsw.lib.entries and slsw.lib.options. See code here and functional description here. I am using a webpack.config that runs a particular plugin based on a command -line option passed to serverless. Now, slsw.lib.options is undefined, so I can't access those options anymore.
Is there a way to keep the existing functionality, or is there an alternative to get the command line options in the worker thread config?
Edit: tried to fix this issue in a fork and gave up. While I can pass the lib.options and make that available in child workers, can't do the same with the entire serverless instance, because of circular references. At this point I gave up and switched to https://github.com/serverless-heaven/serverless-webpack/pull/517, which works well and is backwards compatible.
I tried this branch today, and it didn't work out for me. Probably because my webpack config isn't serializable? (It's something like https://gist.github.com/dominics/f9f24430789d1b4c4f0297a31b5b3ff7, so there's a function in there - not a common case.) But there were no errors; the bundled code just didn't have any externals (and the verbose logging just says "no external modules needed")
I'd prefer a nice loud error I guess.
This PR had also helped us overcome the OoM errors.
For us, it worked where #299 which is 5.4, did not.
We'd love to see this one make it into 5.4
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.
@j0k3r What's going on with this? I have a bug that will be fixed by #570
Edit: I meant a bug fixed by #661
@mikewolfd I'm not sure, but looks like it was addressed by https://github.com/serverless-heaven/serverless-webpack/pull/517, see v5.4.0
I'll echo above, from what I can tell this can now be closed. The original issues are now solved for me by using the following serverless config:
package:
individually: true
custom:
webpack:
serializedCompile: true