Serialized compile to address #299

[asprouse]

What did you implement:

Add the option to serialize webpack compilation when packaging individually. This addresses concerns in #299 and improves compilation speed significantly. In our project with 25 function this decreased the build time from ~400s to ~265s when run with the following command:

time node --max-old-space-size=4096 node_modules/.bin/sls package --verbose

While using thread-loader + cache-loader might be able to optimize accross the multiple builds it takes significant effort to get to a performant build. Running the webpack compilation in series appears to have better performance out of the box, so much so that I suggest it be the default behavior of this plugin. This PR maintains the existing multi-compile as the default and uses custom.webpack.serializedCompile to enable the serial build.

How did you implement it:

Modified compile.js to use BbPromise.mapSeries instead of webpack's multi-compiler functionality. This cut down

How can we verify it:

Set custom.webpack.serializedCompile to true in serverless.yml and run the following command:

time node --max-old-space-size=4096 node_modules/.bin/sls package --verbose

Now set the custom.webpack.serializedCompile to false or omit the option entirely to revert to the default behavior and run the same command. Verify that both compile successfully and observe a significant speed improvement when a project with many functions is serialized.

Todos:

• Write tests
• Write documentation
• Fix linting errors
• Make sure code coverage hasn't dropped
• Provide verification config / commands / resources
• Enable "Allow edits from maintainers" for this PR
• Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

[daniel-cottone]

@asprouse This is awesome, thank you! Can you fix the merge conflicts on this?
@HyperBrain Can we get your eyes on this PR? This would fix a huge issue for us.

[asprouse]

Rebased! @daniel-cottone @HyperBrain

[cipriancaba]

@daniel-cottone @HyperBrain Is anything else required for this to go in? Would love to give it a try and see if it solves our build issues..

[asprouse]

@HyperBrain @mgarciadelojo is there anything I can do to move this along?

[coyoteecd]

@HyperBrain @mgarciadelojo can you guys review and merge this? We've been hitting #299 and it's a pity that a solution exists in this PR and sits here for 1 year...

[mgarciadelojo]

I just approved because I was in the same situation as you. It seems this PR needs an approval from @serverless-heaven in order to be merged?

[mogusbi-motech]

@serverless-heaven @ceilfors @franciscocpg @hassankhan @HyperBrain @MartinRisk @nathanchapman @rdsedmundo could one of you please take a look at this PR

[vicary]

This should helps my situation when worked along with TypeStrong/fork-ts-checker-webpack-plugin#425, I have ~100 lambda bundling at the same time because of serverless-appsync-plugin.

[vicary]

@asprouse Do you think .map(..., { concurrency }) gives more control and usability then .mapSeries(...)?

[asprouse]

@asprouse Do you think .map(..., { concurrency }) gives more control and usability then .mapSeries(...)?

@vicary I think it would add some more flexibility, however running in parallel might be best done in separate processes. You should look at #570 it may be the solution you are looking for. At this point this PR has been open for over a year and approved for ~8 months I think it should be merged as is and then we can address enhancements with other PRs like #570.

[coyoteecd]

I did a dry-run of this PR in my setup of 13 lambdas and total time of a serverless package run went from ~2 minutes to ~3:50 minutes. However it does solve the memory issues without the issues noted here.

I tried playing with map(..., concurrency) and got a decent compromise with batches of 4 => total time decreased to nearly 2 mins while the memory usage was still lower than having everything build in parallel.

Based on the above, I suggest that the serializedCompile option be changed to maxParallelCompiles (or similar). Not present uses old behavior (no maximum); when present, it uses map(..., concurrency) to throttle the simultaneous compiles to the configured number. To make it useful in CI environments, the setting should be overridable via CLI options e.g. serverless package --max-parallel-compiles 2 would limit everything to 2, instead of whatever value is in serverless.yml.

[coyoteecd]

Found that @vicary already implemented the concurrency setting in his fork. I forked it to add a compile-concurrency override so I can call serverless package with different concurrency levels based on whether it runs in CI or not.

This solved the problems we've been having with this plugin.

@miguel-a-calles-mba @asprouse @vicary I think we should bring together this as "the solution" for #299, for the time being, since it doesn't introduce any breaking changes (I tried #570 and it didn't work). Opinions?

[vicary]

I am open to anything that makes progress here.

@coyoteecd You may try ${env:..., default_value} like this to have your CLI option for the moment.

[asprouse]

@coyoteecd @vicary My priority here is getting this PR merged in some form or another. It has already been approved by @mgarciadelojo and @miguel-a-calles-mba. We are just waiting for someone to hit merge. If the reviewers are on board with the change it's an easy tweak to make. I don't want to jeopardize it being merged for the 5.4.0 milestone. @mgarciadelojo, @miguel-a-calles-mba, @rdsedmundo what are your thoughts?

[coyoteecd]

@asprouse reason I think the fixes should be combined is that having a "concurrency" setting kinda obsoletes the serializedCompile one (just set concurrency to 1 and you get the same effect). If this PR is merged as-is, you'll have to support and document both.

[miguel-a-calles-mba]

I re-ran the unit tests and code coverage decreased.

[lhr0909]

@miguel-a-calles-mba Hi, we are running into resource issues and would love to use this flag in our serverless stack. Any ETA on the release of v5.4.0 ? Is it safe to directly take this git branch as an npm dependency in production? Thanks.

[miguel-a-calles-mba]

@lhr0909, we are working to release 5.3.4 and will start 5.4.0 shortly thereafter.

[miguel-a-calles-mba]

@lhr0909, release/5.4.0 branch has been successfully building. Give it a try.

[xahhy]

@lhr0909 any plan to release this feature along with 5.4.0 ?