S3 import in default-handler likely causing higher cold start time

[dphang]

Describe the bug
Not really a bug, more a slight optimization we can do. So I saw the S3 import was added for getStaticPaths support:

serverless-next.js/packages/libs/lambda-at-edge/src/default-handler.ts

Line 21 in a854139

import S3 from "aws-sdk/clients/s3";

Based on some quick perf tests, this require seems to be adding up to 80-100 ms to init times on cold start (1024 MB function, Node 12.x). Total init time is ~250 ms for my app.

I think s3 is not needed all the time, just if getStaticPaths is being used, so maybe this should be required just before where it's actually needed, i.e: https://github.com/serverless-nextjs/serverless-next.js/blame/a854139f4344530de1a42268828231a4d38c7c91/packages/libs/lambda-at-edge/src/default-handler.ts#L319 instead?

To Reproduce
Steps to reproduce the behavior:

1. Add performance logging to the default-handler before and after the require of S3. Then build and deploy with the new handler.
2. Load any page and ensure it hits the Lambda.
3. Look into Lambda logs, you should see that S3 require takes ~100 ms (1024 MB function).

Expected behavior
Maybe lazily require this module for better overall performance.

Additional context
Also, maybe consider using the aws-sdk-js-v3 (https://github.com/aws/aws-sdk-js-v3), as it is more modularized and lightweight? Although it is in gamma release now, so maybe we can wait for a more stable release.

I also did do some initial tests by bundling this new version: @aws-sdk/client-s3-node (via Rollup.js) and it seems to be required somewhat faster (by around 20-30 ms) than the built-in aws-sdk one, which I thought would have been fairly optimized already...

Related article about aws-sdk require times: https://theburningmonk.com/2019/03/just-how-expensive-is-the-full-aws-sdk/

[thchia]

Sounds good - also this is the first I've heard of the @aws-sdk scope... it looks official on npm but is there any release documentation on it (just out of interest)?

[danielcondemarin]

I like the idea of lazy requiring stuff that is only needed in certain code paths, sort of what Next.js does here with jsonwebtoken.

[dphang]

@thchia yep, the project link is here, it's written by AWS: https://github.com/aws/aws-sdk-js-v3. A little bit confusing since in npm, they don't publish it as v3 (s3-client-node itself is 0.1.0-preview), but I believe all the clients under @aws-sdk/client-* are for this new version.

[dphang]

Basically, it's more modularized so you can write code like this:

const { DynamoDBClient, ListTablesCommand } = require("@aws-sdk/client-dynamodb");

(async () => {
  const client = new DynamoDBClient({ region: "us-west-2" });
  const command = new ListTablesCommand({});
  try {
    const results = await client.send(command);
    console.log(results.TableNames.join("\n"));
  } catch (err) {
    console.error(err);
  }
})();

and that will only require the code needed for ListTablesCommand, I think current approach will get a bunch of S3 APIs that we don't use, because of how aws-sdk is built, even importing just S3 client is pretty heavy. That + Rollup.js bundler should help the code be loaded pretty quickly.

PS: just noticed in my trial, I was using the new @aws-sdk/client-s3-node but still non-modularized (importing S3, which has all the APIs in it). Though it's still faster to load than aws-sdk/s3, probably because of some other core optimizations AWS team did. I imagine something like this will be even faster, maybe even close to 20 ms?

import { S3Client, GetObjectCommand, PutObjectCommand } from "@aws-sdk/client-s3-node";

[dphang]

Finished doing some more tests. Added performance.now() logging while importing S3Client in the origin request handler, and requests a very simple page which just has a MaterialUI button (basically my index page on nextjs-repros GitHub repo). Actually found the new version is under @aws-sdk/s3-client:

  // Lazily import only S3Client to reduce init times until actually needed
  console.log("Before S3Client import: " + performance.now());
  const { S3Client } = await import("@aws-sdk/client-s3/S3Client");
  console.log("Before S3Client construction: " + performance.now());
  const s3 = new S3Client({ region: "us-west-1" });
  console.log("Before GetObjectCommand import: " + performance.now());
  const { GetObjectCommand } = await import(
    "@aws-sdk/client-s3/commands/GetObjectCommand"
  );
  console.log("Before PutObjectCommand import: " + performance.now());
  const { PutObjectCommand } = await import(
    "@aws-sdk/client-s3/commands/PutObjectCommand"
  );
  console.log("After s3: " + performance.now());

2020-09-02T18:45:42.648-07:00 | START RequestId: 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 Version: 194
-- | --
  | 2020-09-02T18:45:42.651-07:00 | 2020-09-03T01:45:42.650Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO Before S3Client import: 155.6819770000875
  | 2020-09-02T18:45:42.651-07:00 | 2020-09-03T01:45:42.651Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO Before S3Client construction: 156.27793400012888
  | 2020-09-02T18:45:42.652-07:00 | 2020-09-03T01:45:42.652Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO Before GetObjectCommand import: 157.97929100017063
  | 2020-09-02T18:45:42.652-07:00 | 2020-09-03T01:45:42.652Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO Before PutObjectCommand import: 158.0727440000046
  | 2020-09-02T18:45:42.652-07:00 | 2020-09-03T01:45:42.652Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO After s3: 158.1599400001578
  | 2020-09-02T18:45:42.867-07:00 | 2020-09-03T01:45:42.867Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO info - Loaded env from .env
  | 2020-09-02T18:45:42.946-07:00 | 2020-09-03T01:45:42.945Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO require JS execution time: 292.49969900003634 (ms)
  | 2020-09-02T18:45:42.985-07:00 | 2020-09-03T01:45:42.985Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO SSR execution time: 39.24606999987736 (ms)
  | 2020-09-02T18:45:42.985-07:00 | 2020-09-03T01:45:42.985Z 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089 INFO handler execution time: 335.230237999931 (ms)
  | 2020-09-02T18:45:42.986-07:00 | END RequestId: 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089
REPORT RequestId: 6578ff96-fc4f-4ac5-9ec9-8e6e324bb089	Duration: 338.27 ms	Billed Duration: 350 ms	Memory Size: 1024 MB	Max Memory Used: 90 MB	Init Duration: 178.50 ms

It's even faster than I imagined! It takes less than 3 ms to import all 3 classes on a cold start. So in most cases, we should be able to save the entire ~100 ms of importing S3, and when needed for getStaticPaths it will add a mere 3 ms latency.

With this, size of default-handler is about 500 kB. We could minify further using rollup-terser plugin, but it shouldn't affect too much.

@danielcondemarin any thoughts of using this aws-sdk-js-v3 (https://github.com/aws/aws-sdk-js-v3)? It is in gamma and although not final release, it is actively published and looks stable enough. If you are fine, I can work on the PR to update to to using this.

Now if only we can reduce that page require time of 290 ms for even a simple page...

[danielcondemarin]

@danielcondemarin any thoughts of using this aws-sdk-js-v3 (https://github.com/aws/aws-sdk-js-v3)? It is in gamma and although not final release, it is actively published and looks stable enough. If you are fine, I can work on the PR to update to to using this.

Go for it! I think we should pin the version of the aws-sdk-js-v3 whilst is on gamma though.

[dphang]

Thanks! Will try to work on it tonight.

[dphang]

I created the PR, handler size did increase from ~150 kB to 500 kB since I bundled the new @aws-sdk/client-s3 code into the handler, it should be faster but I am currently testing e2e. We could minify the handler using rollup terser plugin in the future if this is an issue (~180 kB minified).

@thchia if you have a test repo with getStaticPathsthat would be helpful too :)

[thchia]

@dphang I can push one up in a few hours.

[dphang]

No worries, thanks a bunch!

[thchia]

@dphang my test repo is here. Pretty basic, but it has some dynamic SSG routes and a page to check if preview mode is working properly.

[dphang]

@thchia thanks! I'll test it out with these changes.