improve performance of responses

[anonrig]

For benchmarks that check TTFB, opennextjs was performing poorly. This was due to several reasons such as returning the body as a huge chunk with unnecessary copies. To summarize:

• Eliminated Buffer.concat() usage - to avoid copying the entire buffer into memory. From now on, we use ReadableStream.from() to stream chunks without copying.
• Eliminating Buffer.concat() on body length calculation - previously in order to get the body length, we were copying the entire buffer into memory (using Buffer.concat()) and disposing it after we calculated the length. This created huge GC pressure and degraded the performance.
• Chunk-by-chunk writing: Removed getBody() call that copied and concatenated all chunks into a single buffer before writing. From now on, we write each chunks individually - which reduces the TTFB.

I've also updated CI to use Node.js v20 since ReadableStream.from() requires it, but also v18 is EOL.

---

This improves the responses by roughly 20%.

---

PS: AI took my trust in pull-request descriptions with lists - but I wrote it regardless...

[changeset-bot]

🦋 Changeset detected

Latest commit: 2629191

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch
app-router	Patch

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

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/aws@991

commit: 2629191

[mhart]

This is still buffering all chunks and delivering them as one though right?

We should definitely look at streaming without doing that (you only need to buffer until headers have been delivered, can stream after that).

Eg, using Next's MockResponse: https://github.com/vercel/next.js/blob/v15.3.0-canary.13/packages/next/src/server/lib/mock-request.ts and then just wiring up resWriter to the output stream: https://github.com/vercel/next.js/blob/a1c27450e97ec3985c32f763355f8a564cd562bd/packages/next/src/server/lib/mock-request.ts#L238-L240

(or using https://github.com/mhart/fetch-to-node or similar)

[anonrig]

This is still buffering all chunks and delivering them as one though right?

Yes, technically we shouldn't store them in memory and stream them directly without copying/storing.

[jasnell]

well, splitting the write(...) and end() in the send() should at least (hopefully) stream it out a bit better but we need to test/verify further. These were just some low hanging fruit.

[mhart]

Everything in this repo is targeting Node 18 (including all esbuild configs and typescript configs), so I suspect this may require a major bump.

[jasnell]

Ugh, yep...

Welcome to Node.js v18.20.8.
Type ".help" for more information.
> ReadableStream.from
undefined
> 

I have to assume this is why the current code was used.

We can avoid the semver-major bump with a bit more work tho, it would just require an impl like..

async function* gen() {
  for (const chunk of _chunks) {
    yield chunk;
  }
}
const generator = gen();
new ReadableStream({
  async pull(controller) {
    const next = await generator.next();
    if (next.done) {
      controller.close();
    } else {
      controller.enqueue(next.value);
    }
  }
});

Or something along those lines. Bit more work. Bumping the major would be preferred, however, especially since 18 is so far behind.

[jasnell]

Question tho... if this is targeting 18.x... is CI not targeting that?

[vicb]

This is still buffering all chunks and delivering them as one though right?

Yes, technically we shouldn't store them in memory and stream them directly without copying/storing.

Correct, see #992

[conico974]

LGTM, Thanks a lot for that guys.

[vicb]

Thanks all for this work on this 🎉

[sommeeeer]

Awesome 👍