Skip to content

Add page render benchmark#6415

Merged
bluwy merged 9 commits intomainfrom
bench-render
Mar 6, 2023
Merged

Add page render benchmark#6415
bluwy merged 9 commits intomainfrom
bench-render

Conversation

@bluwy
Copy link
Copy Markdown
Member

@bluwy bluwy commented Mar 3, 2023

Changes

Add new pnpm run benchmark render CLI command, and !bench render comment command.

Question: Is creating a private @astrojs/timer integration in /packages/integrations/ fine? Does it affect docs generation?

Example output locally:

Page Avg (ms) Stdev (ms) Max (ms)
/astro 0.58 0.92 9.52
/md 2.75 0.31 4.21
/mdx 63.81 14.06 113.71

Testing

Tested manually locally, and through https://github.com/bluwy/astro/pull/1#issuecomment-1453708161

Docs

I've added a new private @astrojs/timer integration that has a README. Ideally this should be kept private from users too.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 3, 2023

⚠️ No Changeset found

Latest commit: 5f433b1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Mar 3, 2023
@github-actions github-actions bot added the 🚨 action Modifies GitHub Actions label Mar 3, 2023
@bluwy bluwy marked this pull request as ready for review March 3, 2023 15:46
@bluwy bluwy requested a review from a team as a code owner March 3, 2023 15:46
with:
issue-number: ${{ github.event.issue.number }}
message: |
body: |
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a typo that prevented from CI commenting

Comment on lines +13 to +19
handler: async (req: IncomingMessage, res: ServerResponse) => {
const start = performance.now();
await app.render(req);
const end = performance.now();
res.write(end - start + '');
res.end();
},
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main code in the @astrojs/timer integration is here. The rest are largely copied from @astrojs/node but stripped out the unneeded parts.

@bluwy bluwy mentioned this pull request Mar 6, 2023
Copy link
Copy Markdown
Contributor

@matthewp matthewp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great!

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. One thing I struggled with: it took me a while to understand how the render benchmark works. Perhaps, there should be a description somewhere (even the README) to explain how we run the benchmarks.

@bluwy
Copy link
Copy Markdown
Member Author

bluwy commented Mar 6, 2023

One thing I struggled with: it took me a while to understand how the render benchmark works. Perhaps, there should be a description somewhere (even the README) to explain how we run the benchmarks.

Yes, I think the docs needs to be elaborated more on this end. The current available ones are quite brief. I can send some followups to improve on that end in the new few days. (Or if you're interested too!)

@bluwy bluwy merged commit c5bac09 into main Mar 6, 2023
@bluwy bluwy deleted the bench-render branch March 6, 2023 14:55
@bluwy
Copy link
Copy Markdown
Member Author

bluwy commented Mar 6, 2023

Oh crap I forgot there's a requested review for docs on this. Let me check with them real quick 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚨 action Modifies GitHub Actions pkg: integration Related to any renderer integration (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants