chore: add benchmarks - #4157
Conversation
|
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
🚨 action
size-limit report 📦
|
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
|
I did not review all files yet, but I have to run for now so sharing what I found so far.
Depending on the time it ends up taking in the end, we should also see if we need to consider triggering benchmarks on labels like pre-releases to avoid running it on too many PRs unnecessarily, e.g. PRs adding or updating translations, adding an icon, CSS updates, etc. |
delucis
left a comment
delucis
left a comment
There was a problem hiding this comment.
Thanks for the idea to set this up and leading the effort @ematipico 🙌
I left some detailed comments.
Also one high-level thought: at least in our initial run, the three benchmarks for astro build gave basically identical results (24.9–25.0 seconds each). I wonder if that suggests we could just combine the three into one and get just as much benefit? It suggests there is a negligible difference between the components/sidebars/locales customizations so I guess we’re mostly measuring other parts of the build 3 times? What do you think?
| cache: pnpm | ||
| - run: pnpm i --filter @astrojs/starlight --filter @bench/components --filter @bench/large-sidebar --filter @bench/locales | ||
| - name: Run benchmarks | ||
| uses: CodSpeedHQ/action@562878fc153dee6c0f3719c99b1799334cfc3ccc # v5 |
There was a problem hiding this comment.
I couldn’t find this SHA in their repo? This would be the most recent release from before our 3-day embargo window:
| uses: CodSpeedHQ/action@562878fc153dee6c0f3719c99b1799334cfc3ccc # v5 | |
| uses: CodSpeedHQ/action@4296e51e7041e24dadb86d1d6e8b9320d223dbe8 # v5.0.3 |
| build?: Pick<AstroConfig['build'], 'format'>; | ||
| trailingSlash?: AstroConfig['trailingSlash']; | ||
| command?: 'dev' | 'build' | 'preview'; | ||
| snapshotSerializers?: boolean; |
There was a problem hiding this comment.
This is a bit of a nit, but it might be clearer how this works if we name this disableSnapshotSerializers and set it to true in the benchmarks currently passing false.
| title: 'Starlight benchmarks', | ||
| sidebar: [{ label: 'Reference', items: [{ autogenerate: { directory: 'reference' } }] }], | ||
| }, | ||
| { snapshotSerializers: false } |
There was a problem hiding this comment.
Might also be good to have a comment here explaining why these serializers cause issues and need to be disabled.
|
|
||
| export default defineConfig({ | ||
| plugins: [codspeedPlugin()], | ||
| benchmark: { |
There was a problem hiding this comment.
There’s a type error here currently: https://github.com/withastro/starlight/actions/runs/32977984781/job/98207323537?pr=4157#step:7:12
Object literal may only specify known properties, and 'benchmark' does not exist in type 'ViteUserConfigExport'.
| }, | ||
| test: { | ||
| fileParallelism: false, | ||
| hookTimeout: 600_000, |
There was a problem hiding this comment.
This is 10 minutes. I guess we don’t expect things to take that long so maybe a lower timeout could make sense?
| test: { | ||
| ...config.test, | ||
| fileParallelism: false, | ||
| hookTimeout: 600_000, |
There was a problem hiding this comment.
Same comment here, maybe should be a lower timeout?
4 participants
Description
This PR benchmarks to starlight, which can be run locally and via CodSpeed.
They are four benchmarks:
generateRouteDataThe latter is a "functional" benchmark, meaning it needs a different setup that the others need, and that's why there are two separate configs.