fix(astro): Improve ssr performance (astro#11454)

[MatthewLymer]

Changes

Closes #11454

• Improves SSR performance for rendering synchronous components by avoiding the use of Promise.

Testing

• Added unit tests to ~/packages/astro/test/units/render/rendering.test.js
• Integrated into own project to validate performance and ensure correctness

Docs

No docs were added as this is specifically performance related refactoring.

[changeset-bot]

🦋 Changeset detected

Latest commit: 03f555b

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

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

[github-actions]

This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.

[codspeed-hq]

CodSpeed Performance Report

Merging #13195 will improve performances by 49.84%

_{Comparing MatthewLymer:astro-11454-improve-ssr-performance (03f555b) with main (6bac644)}

Summary

⚡ 3 improvements
✅ 3 untouched benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
⚡	Rendering: streaming [false], .astro file	1,052.5 ms	917.5 ms	+14.72%
⚡	Rendering: streaming [false], .md file	18.6 ms	12.8 ms	+45.44%
⚡	Rendering: streaming [true], .astro file	1,403.1 ms	936.4 ms	+49.84%

[MatthewLymer]

Can someone provide their $0.02 on https://github.com/withastro/astro/pull/13195/files#diff-d312d8fce663b920b083d795af0cf0ca4f20374f3467998906ec00ef2b85f6eaR321, I'm pretty sure I need to do an explicit try/catch around here for the non-promise scenario, but I'm curious as to how this would manifest.

[sarah11918]

Just noting that this can totally sound a bit like the blog post here! H.Y.P.E.!

[MatthewLymer]

Can someone provide their $0.02 on https://github.com/withastro/astro/pull/13195/files#diff-d312d8fce663b920b083d795af0cf0ca4f20374f3467998906ec00ef2b85f6eaR321, I'm pretty sure I need to do an explicit try/catch around here for the non-promise scenario, but I'm curious as to how this would manifest.

I've updated the code in question, it resolved the behaviour described in #13195 (comment)

fwiw I'm happy with the PR now.

[ematipico]

Thank you, @MatthewLymer. The code looks good, and we have no particular concerns. We will release this PR tomorrow and will be on the lookout for possible regressions.

If you have time, energy and interest, feel free to stick around and see if you can help us triage and fix possible regressions. 😄

[datner]

I'm curious though, why do preact and qwik out-perform astro? Noted that solidjs do as well

Merge day

[MatthewLymer]

@ematipico I can definitely help as I'd like to ensure it's the best possible outcome. The PR was authored on behalf of my company, VerticalScope Inc. Fora, so it's contingent on scheduling on that side, however, I don't see any impediments here.

@datner I haven't profiled either the preact or qwik islands, but my assumption right now is the eager loading of all components and rendering to a temporary buffer. Astro's current implementation would probably be faster with the use of more asynchronous components, as rendering to a buffer to synchronous components is largely just overhead.

[ascorbic]

@MatthewLymer we're going to highlight this in our release tomorrow and I've given you a shout-out in the blog. Would you like a credit for your company too? I'm happy to add one. See the example here where I credited work done for the Washington Post: https://astro.build/blog/astro-520/

[MatthewLymer]

@ascorbic That's awesome, yes please, if you could credit my company, it would be appreciated.

[ascorbic]

Great! Do you and/or the company have X and/or Bluesky accounts so we can tag you in posts?

[MatthewLymer]

@ascorbic we don't have a social media presence, however, instead of mentioning "VerticalScope Inc." above, can you instead mention "Fora" at https://fora.com instead, which is the business unit I'm working in.

[scott-the-brewer]

This is amazing! Thanks for pushing this through. Any thoughts on when this will get released?

[ematipico]

@scott-the-brewer

It's written here #13195 (comment)

[scott-the-brewer]

Thanks @ematipico

Interestingly, our website is experiencing about 2.5x worse SSR performance, by just updating to 5.3.0.

5.1.7

5.3.0

Will do some more digging next week.

[MatthewLymer]

@scott-the-brewer do you have a high level overview of what the page does, how many sync/async components, how much IO, etc?

[scott-the-brewer]

• We don't use any framework, besides Astro
• We have an abstraction for getting data - during SSR, this gets data from Astro.locals (no IO), during development it makes real API calls (IO)
• This means, during SSR rendering, there is no IO
• At a rough guess, 90% async and 10% sync components

The rendering times in the screenshots in my previous message, was during SSR

We use our own SSR adapter, essentially:

export function createExports(manifest: SSRManifest) {
  const app = new App(manifest)

  return {
    async handler(request: Request, locals?: object) {
      const routeData = app.match(request)

      if (routeData) {
        return await app.render(request, {
          routeData,
          locals,
        })
      }
    },
  }
}

To isolate things more, I've tried on 5.2.6, and can confirm it's only 5.3.0 when the performance got worse in our project.

[MatthewLymer]

@scott-the-brewer thanks for the information, I've done some investigation and unfortunately haven't been able to replicate a performance regression.

For my testsing I've been using the minimal example, with some synchronous and asynchronous components, see https://github.com/withastro/astro/blob/67e08afbfc5837b83371e0054601f26b330dac42/examples/minimal/src/pages/index.astro

For the test, I've been using the load test with 1 concurrent and 10 concurrent users, with rendering 1,500 async components.

BEFORE

iterations.....................: 70     2.014521/s

AFTER

iterations.....................: 110    3.49734/s

Granted it's a fairly trivial test, but I'm not sure how to replicate the results you're seeing.

If you can provide a minimal reproducible example I can investigate that, or perhaps provide some CPU profiling information.

[MatthewLymer]

@scott-the-brewer Can you also provide what tooling you're using to measure the performance?

[scott-the-brewer]

Yup, will try create a reproducible example this weekend.

For reference I'm using middleware to measure this:

import type { AstroIntegration } from 'astro'

export function performance(): AstroIntegration {
  return {
    name: 'astro-performance',
    hooks: {
      'astro:config:setup': ({ addMiddleware }) => {
        addMiddleware({
          entrypoint: '@ps/astro-integrations/performance-middleware',
          order: 'post',
        })
      },
    },
  }
}

import process from 'node:process'
import { Logger } from '@ps/common/helpers'
import { defineMiddleware } from 'astro:middleware'

const logger = new Logger({ label: 'astro-performance' })

export const onRequest = defineMiddleware(async (context, next) => {
  const start = process.hrtime.bigint()

  const response = await next()

  const end = process.hrtime.bigint()
  const duration = Number(end - start) / 1e6

  if (response.headers && response.headers.get('content-type') === 'text/html') {
    logger.info(`Page ${context.url} rendered in ${duration}ms`)
    response.headers.set('x-render-time', `${duration}ms`)

    return new Response(response.body, {
      status: response.status,
      headers: response.headers,
    })
  }

  return response
})

[MatthewLymer]

@scott-the-brewer Before you investigate further, I believe your instrumentation is giving you misleading results.

In Astro, middleware returns when (or around?) the response headers have been sent and the response has started streaming, so your instrumentation is giving you an approximation of that time, not a duration of how long the request took to be completely rendered.

I go over that in my comment #13195 (comment)

Instead, if you could measure the performance outside of middleware (by measuring the time observed by the client), this should give you vastly different numbers, pre and post v5.3.

Can you provide this information?

[scott-the-brewer]

Thanks @MatthewLymer will report back soon.