perf: in-memory public files check

[patak-dev]

Description

Around 4% of the time when running https://github.com/yyx990803/vite-dev-server-perf is spent on the serve public middleware.

This middleware will check every request before getting into the transform pipeline. Internally, it uses sirv, which does a fs.existsSync check for both filePath and filePath + '/index'. It seems that we can't configure it to avoid the /index suffix check, which I don't think we should be serving from the public dir. Even if this is fixed, there will still be a toll for every request because of the public dir feature.

This PR reads the public dir recursively on server start and keeps an in-memory set of all public files so we can avoid going through sirv.

The drawback is that a server restart is needed if new files are added to the public directory. If we want to move forward with this PR, we could decide to watch the public dir and add new files.

We are currently patching sirv to add a shouldServe option, so we can do a case check in case-insensitive platforms. If we merge this PR, we can also remove this patch and the check because the in-memory map will already filter these out.

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[patak-dev]

@ArnaudBarre is telling me he implemented the same (+the watcher) in rds: https://github.com/ArnaudBarre/rds/blob/main/src/server/public.ts, we should check rds more often :)

[ArnaudBarre]

For ideas only, most of my implementations where never tested on Windows 👀
But that's from there I also bring the idea of not using one big file watcher, having used multiple dedicated watcher (public, config, src) worked well and allowed to keep things independent (even if the entry point is a complicated merge of all, it really misses the plugin architecture of Vite)

[patak-dev]

The watcher is now used to delete/add publicFiles to the in-memory cache, and the cache is also used in checkPublicFile when available (in dev, but later on we could add it to build time too if we'd like).

The publicFiles don't have the publicDir prefixed in the set, so we avoid path.join calls to do the checks.

[patak-dev]

We can't remove the shouldServe patch because it is used during preview too (at least in this PR, but it is removed now from the serve public middleware)

[patak-dev]

/ecosystem-ci run

[vite-ecosystem-ci]

📝 Ran ecosystem CI on 4bb5f14: Open

suite	result	latest scheduled
analogjs	✅ success	✅ success
astro	✅ success	✅ success
histoire	❌ failure	❌ failure
ladle	✅ success	✅ success
laravel	❌ failure	❌ failure
marko	✅ success	✅ success
nuxt	❌ failure	❌ failure
nx	✅ success	✅ success
previewjs	✅ success	✅ success
qwik	✅ success	✅ success
rakkas	✅ success	✅ success
sveltekit	❌ failure	❌ failure
unocss	✅ success	✅ success
vike	✅ success	✅ success
vite-plugin-pwa	✅ success	✅ success
vite-plugin-react	✅ success	✅ success
vite-plugin-react-pages	✅ success	✅ success
vite-plugin-react-swc	✅ success	✅ success
vite-plugin-svelte	✅ success	✅ success
vite-plugin-vue	✅ success	✅ success
vite-setup-catalogue	✅ success	✅ success
vitepress	✅ success	✅ success
vitest	✅ success	✅ success

[sapphi-red]

Cool!

[sapphi-red]

Here's the results of https://github.com/yyx990803/vite-dev-server-perf on my Windows machine. It's around 3%.

before

1000 TS modules (50x20) loaded in: 1167ms (runs: [1387,1181,1167,1166,1160])
1000 TS modules (20x50) loaded in: 1086ms (runs: [1161,1086,1078,1082,1099])
2000 TS modules (100x20) loaded in: 2530ms (runs: [2632,2530,2523,2504,2543])
5000 TS modules (250x20) loaded in: 6675ms (runs: [7346,6790,6630,6598,6675])
10000 TS modules (400x25) loaded in: 13616ms (runs: [15038,13693,13616,13447,13249])

after

1000 TS modules (50x20) loaded in: 1150ms (runs: [1521,1162,1131,1132,1150])
1000 TS modules (20x50) loaded in: 1025ms (runs: [1069,1025,1058,997,1013])
2000 TS modules (100x20) loaded in: 2424ms (runs: [2562,2426,2424,2403,2400])
5000 TS modules (250x20) loaded in: 6524ms (runs: [6957,6547,6524,6462,6517])
10000 TS modules (400x25) loaded in: 13286ms (runs: [14343,13192,13206,13290,13286])

[patak-dev]

Here's the results of yyx990803/vite-dev-server-perf on my Windows machine. It's around 3%.

Thanks for benchmarking! That isn't bad 🙌🏼

[patak-dev]

Should we wait for 5.1 to merge this one? I think it is fine to release it in a patch.

[sapphi-red]

It depends on when we start 5.1 beta for me. If we are starting in a week, then I'd say let's wait for that.

[bluwy]

I think we can cut this as a patch since it shouldn't work too differently that before to be noticable. Nice perf fix!

[patak-dev]

It depends on when we start 5.1 beta for me. If we are starting in a week, then I'd say let's wait for that.

I don't think we have enough minor-only PRs yet to justify starting work on 5.1, so let's go with this one in a patch then.

[cpojer]

The drawback is that a server restart is needed if new files are added to the public directory. If we want to move forward with this PR, we could decide to watch the public dir and add new files.

This seems like a user-hostile change that feels like a regression given that it comes as a patch release. It does not match expectations that people have about Vite, considering that it doesn't work like that for source code. If you are sticking with this approach, how about integrating the behavior of vite-plugin-restart for the public/ folder?

[patak-dev]

@cpojer I implemented watching of public files before this PR was merged, see #15195 (comment)

Are you seeing a regression? The public dir should work in the same way as before.

We're probably going to remove this approach once we merge #15279, as we can use that same cache for the checks (that is more performant when there are symlinks)

[cpojer]

Oh fantastic, sorry I missed that change was part of this PR already 🤦‍♂️