From 2ca10a9c26cb5bbf047f093b8dfdfe2dae6d4987 Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Sun, 31 Mar 2024 17:35:55 +0100 Subject: [PATCH 1/3] feat: rework child rendering to use class Quite a lot of garbage collection happens when rendering due to the fact we create anonymous functions when buffering writes (the `catch`, the destination/write function, etc). By extracting this logic into a class, we can instead rely on prototype methods which exist once in memory. --- .../astro/src/runtime/server/render/any.ts | 5 +- .../astro/src/runtime/server/render/util.ts | 74 +++++++++++-------- 2 files changed, 46 insertions(+), 33 deletions(-) diff --git a/packages/astro/src/runtime/server/render/any.ts b/packages/astro/src/runtime/server/render/any.ts index 20de2d7320ce..e7e9f0b56e9e 100644 --- a/packages/astro/src/runtime/server/render/any.ts +++ b/packages/astro/src/runtime/server/render/any.ts @@ -1,11 +1,14 @@ import { escapeHTML, isHTMLString, markHTMLString } from '../escape.js'; +import { isPromise } from '../util.js'; import { isAstroComponentInstance, isRenderTemplateResult } from './astro/index.js'; import { type RenderDestination, isRenderInstance } from './common.js'; import { SlotString } from './slot.js'; import { renderToBufferDestination } from './util.js'; export async function renderChild(destination: RenderDestination, child: any) { - child = await child; + if (isPromise(child)) { + child = await child; + } if (child instanceof SlotString) { destination.write(child); } else if (isHTMLString(child)) { diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index 61caff6cc6b0..4afa7c75ff6e 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -149,6 +149,46 @@ export function renderElement( return `<${name}${internalSpreadAttributes(props, shouldEscape)}>${children}`; } +const noop = () => {}; +class BufferedRenderer implements RenderDestination { + private __chunks: RenderDestinationChunk[] = []; + private __renderPromise: Promise | void; + private __destination?: RenderDestination; + + public constructor(bufferRenderFunction: RenderFunction) { + this.__renderPromise = bufferRenderFunction(this); + // Catch here in case it throws before `renderToFinalDestination` is called, + // to prevent an unhandled rejection. + Promise.resolve(this.__renderPromise).catch(noop); + } + + public write(chunk: RenderDestinationChunk): void { + if (this.__destination) { + this.__destination.write(chunk); + } else { + this.__chunks.push(chunk); + } + } + + public async renderToFinalDestination(destination: RenderDestination) { + // Write the buffered chunks to the real destination + for (const chunk of this.__chunks) { + destination.write(chunk); + } + + // NOTE: We don't empty `bufferChunks` after it's written as benchmarks show + // that it causes poorer performance, likely due to forced memory re-allocation, + // instead of letting the garbage collector handle it automatically. + // (Unsure how this affects on limited memory machines) + + // Re-assign the real destination so `instance.render` will continue and write to the new destination + this.__destination = destination; + + // Wait for render to finish entirely + await this.__renderPromise; + } +} + /** * Executes the `bufferRenderFunction` to prerender it into a buffer destination, and return a promise * with an object containing the `renderToFinalDestination` function to flush the buffer to the final @@ -171,38 +211,8 @@ export function renderElement( export function renderToBufferDestination(bufferRenderFunction: RenderFunction): { renderToFinalDestination: RenderFunction; } { - // Keep chunks in memory - const bufferChunks: RenderDestinationChunk[] = []; - const bufferDestination: RenderDestination = { - write: (chunk) => bufferChunks.push(chunk), - }; - - // Don't await for the render to finish to not block streaming - const renderPromise = bufferRenderFunction(bufferDestination); - // Catch here in case it throws before `renderToFinalDestination` is called, - // to prevent an unhandled rejection. - Promise.resolve(renderPromise).catch(() => {}); - - // Return a closure that writes the buffered chunk - return { - async renderToFinalDestination(destination) { - // Write the buffered chunks to the real destination - for (const chunk of bufferChunks) { - destination.write(chunk); - } - - // NOTE: We don't empty `bufferChunks` after it's written as benchmarks show - // that it causes poorer performance, likely due to forced memory re-allocation, - // instead of letting the garbage collector handle it automatically. - // (Unsure how this affects on limited memory machines) - - // Re-assign the real destination so `instance.render` will continue and write to the new destination - bufferDestination.write = (chunk) => destination.write(chunk); - - // Wait for render to finish entirely - await renderPromise; - }, - }; + const renderer = new BufferedRenderer(bufferRenderFunction); + return renderer; } export const isNode = From 2eb2f1a4176c5322470b90c6fd9c81694b376199 Mon Sep 17 00:00:00 2001 From: 43081j <43081j@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:40:12 +0100 Subject: [PATCH 2/3] chore: rename buffered renderer properties to remove prefix Removes the `__` prefix of the private properties to match the conventions used elsewhere in the repo. --- .../astro/src/runtime/server/render/util.ts | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index 4afa7c75ff6e..afece3f7b841 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -150,29 +150,34 @@ export function renderElement( } const noop = () => {}; + +/** + * Renders into a buffer until `renderToFinalDestination` is called (which + * flushes the buffer) + */ class BufferedRenderer implements RenderDestination { - private __chunks: RenderDestinationChunk[] = []; - private __renderPromise: Promise | void; - private __destination?: RenderDestination; + private chunks: RenderDestinationChunk[] = []; + private renderPromise: Promise | void; + private destination?: RenderDestination; public constructor(bufferRenderFunction: RenderFunction) { - this.__renderPromise = bufferRenderFunction(this); + this.renderPromise = bufferRenderFunction(this); // Catch here in case it throws before `renderToFinalDestination` is called, // to prevent an unhandled rejection. - Promise.resolve(this.__renderPromise).catch(noop); + Promise.resolve(this.renderPromise).catch(noop); } public write(chunk: RenderDestinationChunk): void { - if (this.__destination) { - this.__destination.write(chunk); + if (this.destination) { + this.destination.write(chunk); } else { - this.__chunks.push(chunk); + this.chunks.push(chunk); } } public async renderToFinalDestination(destination: RenderDestination) { // Write the buffered chunks to the real destination - for (const chunk of this.__chunks) { + for (const chunk of this.chunks) { destination.write(chunk); } @@ -182,10 +187,10 @@ class BufferedRenderer implements RenderDestination { // (Unsure how this affects on limited memory machines) // Re-assign the real destination so `instance.render` will continue and write to the new destination - this.__destination = destination; + this.destination = destination; // Wait for render to finish entirely - await this.__renderPromise; + await this.renderPromise; } } From 17d5a6e9609f2bceaff346f4822ca9e81fd8eb32 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Mon, 1 Apr 2024 21:25:00 +0800 Subject: [PATCH 3/3] Update packages/astro/src/runtime/server/render/util.ts --- packages/astro/src/runtime/server/render/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts index afece3f7b841..5e1fff9fc0ea 100644 --- a/packages/astro/src/runtime/server/render/util.ts +++ b/packages/astro/src/runtime/server/render/util.ts @@ -181,7 +181,7 @@ class BufferedRenderer implements RenderDestination { destination.write(chunk); } - // NOTE: We don't empty `bufferChunks` after it's written as benchmarks show + // NOTE: We don't empty `this.chunks` after it's written as benchmarks show // that it causes poorer performance, likely due to forced memory re-allocation, // instead of letting the garbage collector handle it automatically. // (Unsure how this affects on limited memory machines)