Skip to content
This repository has been archived by the owner on Jan 28, 2025. It is now read-only.

perf(lambda-at-edge): fix rollup bundling for dynamic imports perf #1029

Merged
merged 4 commits into from
Apr 29, 2021

Conversation

dphang
Copy link
Collaborator

@dphang dphang commented Apr 29, 2021

Note: testing rollup config to properly bundle dynamic imports like s3client which is not always needed. The cost is an additional wrapper on top of main code but this seems expected optimization from Rollup.js (preloading and parsing optimization).

Previously, inlining dynamic imports into one file was still causing that code to be at least partially preloaded outside of the handler, e.g initializing some S3/AWS SDK related objects even if the S3Client itself was only initialized when needed. So it caused higher cold starts due to this for all paths. With this change, we split into multiple chunks. Total there should be a few chunks:

  • Entry chunk (wrapper of ~20 lines on top of other code)
  • Core chunk (all of the routing code bundled into one file)
  • Dynamic chunks (e.g S3 client, GetObjectCommand, PutObjectCommand) which is heavy code only needed on fallback paths

Total size of all chunks remains around the same but slightly higher at 657 kB vs. 651 kB before. However, Lambda does not need to read (and preload some parts of it) ~166 kB of code for S3 unless it's needed. There is one extra file I/O due to the wrapper requiring the core code but from tests, it appears to be negligible as ~25 ms is saved in init time from my tests (195 -> 170 ms with the 512 MB Lambda function)
In the future, we can do further optimizations like getting rid of complexity of having the origin response handler, and even using Rollup again at build/deploy time to further get rid of code paths that are not used (e.g if user has no rewrites, we can get rid of that code)

@dphang dphang marked this pull request as draft April 29, 2021 07:47
@dphang dphang mentioned this pull request Apr 29, 2021
@codecov
Copy link

codecov bot commented Apr 29, 2021

Codecov Report

Merging #1029 (ebbb63a) into master (c86bd09) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ebbb63a differs from pull request most recent head 244853c. Consider uploading reports for the commit 244853c to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1029   +/-   ##
=======================================
  Coverage   82.31%   82.31%           
=======================================
  Files          69       69           
  Lines        2578     2578           
  Branches      615      615           
=======================================
  Hits         2122     2122           
  Misses        390      390           
  Partials       66       66           
Impacted Files Coverage Δ
packages/libs/lambda-at-edge/src/build.ts 95.33% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c86bd09...244853c. Read the comment docs.

@jvarho
Copy link
Collaborator

jvarho commented Apr 29, 2021

Yes, this looks better than my effort 👍

I can confirm this reproduces the improvement I saw in cold starts. Here are five-run averages for before and after this branch. I'm using 128 MB memory setting and a redirect to get the worst case.

Before:

Max Memory Used: 71 MB
Init Duration: 201.8 +/- 8.3 ms
Duration: 30.2 +/- 5.3 ms

After:

Max Memory Used: 67 MB
Init Duration: 161.2 +/- 6.9 ms
Duration: 31.3 +/- 6.7 ms

About 20% lower init duration and a bit less memory use.

I have not tested the fallback time, because it's dominated by S3 latency and rendering.

@dphang dphang marked this pull request as ready for review April 29, 2021 18:33
@dphang dphang changed the title fix(lambda-at-edge): fix rollup bundling for dynamic imports perf(lambda-at-edge): fix rollup bundling for dynamic imports Apr 29, 2021
@dphang dphang changed the title perf(lambda-at-edge): fix rollup bundling for dynamic imports perf(lambda-at-edge): fix rollup bundling for dynamic imports perf Apr 29, 2021
@dphang dphang merged commit f8f694c into master Apr 29, 2021
@delete-merged-branch delete-merged-branch bot deleted the dphang/fix-rollup-bundle branch April 29, 2021 18:57
@kirkness kirkness mentioned this pull request May 4, 2021
13 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants