Revert to external middleware #656
Conversation
🦋 Changeset detectedLatest commit: 238b72f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
| if (reqOrResp instanceof Response) { | ||
| return reqOrResp; | ||
| } | ||
|
|
||
| // @ts-expect-error: resolved by wrangler build | ||
| const { handler } = await import("./server-functions/default/handler.mjs"); |
There was a problem hiding this comment.
Thought: The code is always parsed wether or not we return on line 41. Meaning that the perf difference could be explained by the global init. To have the init done only once we could have a top level serverHandler and have
serverHandler ??= (await import("./server-functions/default/handler.mjs")).handler;We probably want something similar for the middlewareHandler - we the current top level handler, we would have the same issues as with the server handler: process.env not being initialized at that point and not possible to do IO at top level.
There was a problem hiding this comment.
We probably want something similar for the middlewareHandler
That's not an issue for the middleware, we already import it using an awaited import https://github.com/opennextjs/opennextjs-aws/blob/c2da3a8df638b8facdb896e22538a143b3425006/packages/open-next/src/core/routing/middleware.ts#L38
The code is always parsed wether or not we return on line 41. Meaning that the perf difference could be explained by the global init.
I think the problem is not with the parsing itself, but with the v8 compilation. The whole file is parsed, but not all of it is compiled.
I did a simple experiment of returning a silly Response just after the await import and it added barely 100ms of cpu time, compared to the minimum 400 that i would get when returning something from the middleware layer (a redirect or any response from the middleware)
I did an experiment with v8 compile cache in aws and by precompiling the files, you can easily shave 200/300ms, but it did not fully remove the cold start. Things that are imported/required only inside a function doesn't get compiled until the function is actually called.
And i think that when the middleware is bundled, it is actually inside a big function that also contains every other NextServer related things, which probably explains the difference
|
I'll fix it and rebase on main then. |
d6ef564 to
6113481
Compare
+1, We could use |
This is just a proposal for now, but this is the best way that i can think of to really reduce cold start for ISR/SSG (in combination with
enableCacheInterception)By using that we can go from 600-800ms of cpu time for ISR/SSG during cold start to around 30-40ms. Tested with the app-router e2e app with
enableCacheInterceptionand cache purge as well.This has the drawback of increasing the size of the app.
We should also think if we want to move back the main handler import at the top as well. This could reduce cpu time for SSR route themselves, but could cause issue with thing that are loaded right away like instrumentation (especially if there is I/O)