Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[@aws-cdk/aws-lambda-nodejs] Bad performance on deployment of NodejsFunction construct, compared to plain Function construct #9639

Closed
Dzhuneyt opened this issue Aug 12, 2020 · 4 comments · Fixed by #9632
Assignees
Labels
@aws-cdk/aws-lambda-nodejs duplicate This issue is a duplicate. guidance Question that needs advice or information. in-progress This issue is being actively worked on.

Comments

@Dzhuneyt
Copy link
Contributor

Dzhuneyt commented Aug 12, 2020

Whenever I try to deploy a stack that's based mainly on Lambdas, as part of a relatively sized project (20-30 Lambdas), I am faced with a terrible development experience when using a fairly common combination of:

  • AWS CDK
  • The NodejsFunction construct by the @aws-cdk/aws-lambda-nodejs package
  • TypeScript based Lambdas
  • A Mac laptop

To demonstrate the magnitude of the issue, I created a simple CDK app with a simple CDK stack with 10 identical (almost empty of logic) Lambdas.
In the first scenario, I deploy the stack using .js files and the standard new lambda.Function() construct.
In the second scenario I deploy the stack using .ts files and the new NodejsFunction() construct.

The time difference between both deployments (always starting from a non-existing stack in CloudFormation) is the following:

  • lambda.Function construct deployment time for a stack of 10 Lambdas: 71 seconds
  • NodejsFunction construct deployment time for a stack of 10 Lambdas: 330 seconds

To help mitigate it to some degree, I contributed a pull request a while ago (#8544) which adds a ":delegated" flag to Docker volumes that build the Lambda inside the NodejsFunction construct. However, apparently, this optimization is not enough for real life development and deployment.

I might be interested in contributing to the solution of this problem, but currently - I'm a bit out of ideas on how to approach it. Suggestions are welcome.


Minimum reproducable code pushed to this temporary repo: https://github.com/Dzhuneyt/aws-cdk-nodejs-function-performance-test

@Dzhuneyt Dzhuneyt added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2020
@jogold
Copy link
Contributor

jogold commented Aug 12, 2020

Duplicate of #9120

See #9632 for a WIP to solve this (and #9564) by bundling locally instead of in Docker

Could you try your app with the code in #9632? You will need to have Parcel v2 installed in your project: yarn add --dev parcel@next or npm install --save-dev parcel@next

@Dzhuneyt
Copy link
Contributor Author

Dzhuneyt commented Aug 12, 2020

@jogold I tried, but wasn't able to change the @aws-cdk/aws-lambda-nodejs to refer to your pull request due to various cross-package version dependencies, etc. Do you know of an easy way to attach a specific commit of a specific package of @aws-cdk to an existing CDK project?

@SomayaB SomayaB added in-progress This issue is being actively worked on. duplicate This issue is a duplicate. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 12, 2020
@jogold
Copy link
Contributor

jogold commented Aug 13, 2020

@Dzhuneyt starting from v1.58.0, replace the files in node_modules/@aws-cdk/aws-lambda-nodejs/lib/bundling.js and node_modules/@aws-cdk/aws-lambda-nodejs/lib/util.js with the files provided here https://gist.github.com/jogold/63e5f315aefb8a0bb88477caaa10d826

@eladb
Copy link
Contributor

eladb commented Aug 17, 2020

Duplicate #9632

@eladb eladb closed this as completed Aug 17, 2020
mergify bot pushed a commit that referenced this issue Aug 17, 2020
Bundle locally if Parcel v2 is installed.

Closes #9120
Closes #9639
Closes #9153 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
misterjoshua pushed a commit to misterjoshua/aws-cdk that referenced this issue Aug 19, 2020
Bundle locally if Parcel v2 is installed.

Closes aws#9120
Closes aws#9639
Closes aws#9153 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-nodejs duplicate This issue is a duplicate. guidance Question that needs advice or information. in-progress This issue is being actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants