Experimental-serverless-trace target increases Lambda@Edge SSR page cold boot times

ari-luokkala commented 3 years ago

Issue Summary

The use of experimental-serverless-trace target with SSR page doubles lambda cold boot times. This causes application performance downgrade on lambda cold starts serving that page.

Actual behaviour

Boot times are roughly doubled with experimental-serverless-trace compared to serverless target.

Expected behaviour

Boot times are roughly same with experimental-serverless-trace compared to serverless target.

Steps to reproduce

Set useServerlessTraceTarget: true in serveless.yml for application with SSR.

Versions

OS/Environment: AWS Lambda@Edge
@sls-next/serverless-component version: 3.5.2
Next.js version: 11.1.2

Additional context

N/A

Checklist

[x ] You have reviewed the README and FAQs, which answers several common questions.
[ x] You have reviewed our DEBUGGING wiki and have tried your best to include complete information and reproduction steps (including your configuration) as is possible.
[ x] You have first tried using the most recent latest or alpha @sls-next/serverless-component release version, which may have already fixed your issue or implemented the feature you are trying to use. Note that the old serverless-next.js component and the serverless-next.js plugin are deprecated and no longer maintained.

Screenshots/Code/Configuration/Logs

lambda-edege-cold-start-with-trace-target

lambda-edege-cold-start-without-serverless-target

dphang commented 3 years ago

Please follow the bug template in the future and include the versions of Next.js and this component's versions you are using.

Yes, in older versions of Next.js (I think before 11) it has high cold start time because Next.js just provides lightweight pages and we simply used their node-file-trace package to trace the node_modules, they were not bundled/tree-shaken. In newer versions, Next.js now uses new shared chunks of JS even for serverless trace target (which actually seems to be mostly the same as serverless target) so this should not be happening now. I believe Vercel will also be removing experimental-serverless-trace in the future too?

See: https://github.com/vercel/next.js/discussions/16276

jonahallibone commented 3 years ago

@dphang what is the default target used by this package when you deploy? I was under the impression (I think now wrongly) that it was the old serverless target, with an option for experiential-serverless-trace.

dphang commented 3 years ago

We are using the serverless target by default, which as of new Next.js versions (I think 11+) is using shared SSR chunks, and it seems to be the same as experimental-serverless-trace now, so not sure if it's doing much?

As mentioned, my problem with the previous behavior of serverless-trace target was that we needed to use @vercel/nft to trace the dependencies once (e.g node modules), although it was not tree-shaking things so for example packages like the entire @material-ui may get included when you may only need one component, and slowed perf a lot due to cold starts.

Not sure what the plan is from Vercel but it seems like the serverless-trace target is going away?

jonahallibone commented 3 years ago

@dphang honestly the documentation I've seen from Vercel on this front has been incredibly confusing. Sometimes it seems like serverless is going away and serverless-trace is staying. In that comment Tim cites this repo as a reason for keeping serverless, although at this point it seems like it's just not the right default option anymore? I've probably read every thread like 10 times and still don't quite understand what their approach is. As you've pointed out before, it seems like serverless-trace is being removed as well though, from comments by Tim. It does seem like Next 12+ does include some new tracing, not sure if you've noticed any differences

ari-luokkala commented 3 years ago

@dphang I refreshed the description to contain the correct bug template now.

jonahallibone commented 3 years ago

@dphang not to blow up this thread, but I found a new useful things:

target itself is going away, not just the serverless or experimental-serverless-trace here

Bringing @vercel/nft into Next.js supersedes the target approach, making target deprecated in Next.js 12.
There is now documentation on how to use @vercel/nft and seems like it's what they recommend using for serverless environments. here

dphang commented 3 years ago

@ari-luokkala thanks. Any particular dependencies you are using and can you share a simple repo with? Wondering if experimental-serverless-trace is not efficiently bundling now...

@jonahallibone thanks for the update, I hadn't read into that latest update yet. I will look into that more. Actually that could be beneficial if it's now more efficiently tree shaking now - last time there was poor performance because it @vercel/nft was not properly tree shaking the MaterialUI library

jonahallibone commented 3 years ago

@dphang they also added two exportable functions where you can ignore/add dependencies which might not be properly bundled

ari-luokkala commented 3 years ago

@dphang I can provide some specific files and configs, but I need to know what exact files would suffice.

dphang commented 3 years ago

@ari-luokkala Yes, I think it would be good to know which node dependencies (if any) you are using that could cause the higher cold starts...otherwise I can try to repro later myself but I hadn't used this target myself for a while so would take some time. It would be good if you can try to compare the generated code first and get a sense of what the differences are?

I remember before MaterialUI had a problem with old serverless-trace, not sure if it's a similar thing.

ari-luokkala commented 3 years ago

@dphang I noticed that with trace target also node_modules are put to the deployment zip file, other than that files look the same, but did not check e.g. require blocks.

ari-luokkala commented 3 years ago

@dphang It seems to be that the problem appears only when we need to do a NodeJS require('path/to/nextjs/page') to render pages server-side. With SSG this problem is gone. And things not being straightforward, it seems to be that when using trace target there is additional ~4 sec delay with SSR with that above mentioned NodeJS require.

dphang commented 3 years ago

@ari-luokkala sorry I hadn't gotten a chance to look again, yes if node_modules I recall it's not bundled efficiently...I also had the same problem with material UI as mentioned, it seemed @vercel/nft does not tree-shake node_modules efficiently compared to webpack? You should check if you are using any large libraries where you might be inadvertently including code that's not actually used but adds to require time

federicobarera commented 2 years ago

How are you guys tracing the performance of the lambda@edge function (X-Ray doesn't seem applicable to @edge). We are facing performance issues that see ssr pages load time doubling when deployed @edge (compared to ec2/ecs deployments, same vcores and mem allocations).

We do have experimental-server-trace enabled due to the size limitations. However the performance degradation is consistent, hence I am trying to drill down were the bottleneck lies (I cannot pinpoint it to cold starts yet).

Suggestions?

ari-luokkala commented 2 years ago

We have used CloudFront monitoring and from there Lambda@Edge metrics. Additionally for debugging we some times fork this repo and deploy custom version with added custom debug logging. We have also set "logLambdaExecutionTimes: true" in serverless.yml, but I haven not noticed any impact with that. In the lambda logs, I think there is also this "initial time" or similar showing the cold boot times.

khg0712 commented 2 years ago

Any updates here?

serverless-nextjs / serverless-next.js