Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: cached fs utils with shared trees #15294

Draft
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

patak-dev
Copy link
Member

Description

Follow up to:

Implements an idea we discussed with @sheremet-va, sharing the DirentCache trees between FsUtils of different servers. In Vitest workspace mode this should avoid duplicating the trees of overlapping roots.

Pushing it as a draft for now. We should first merge #15279.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Copy link

stackblitz bot commented Dec 8, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@patak-dev patak-dev added the performance Performance related enhancement label Dec 8, 2023
@patak-dev patak-dev changed the base branch from main to perf/in-memory-fs-checks December 8, 2023 22:07
@patak-dev patak-dev mentioned this pull request Dec 9, 2023
4 tasks
Base automatically changed from perf/in-memory-fs-checks to main December 12, 2023 09:32
@ArnaudBarre ArnaudBarre self-requested a review January 24, 2024 08:50
@patak-dev patak-dev marked this pull request as ready for review January 26, 2024 20:06
@patak-dev
Copy link
Member Author

Given that we now use the workspace root, for most users what should happen is that servers started on the same monorepo will all share the same root, so connectRootCacheToActiveRoots will finish in this condition: https://github.com/vitejs/vite/pull/15294/files#diff-ff9e3d87ccc2b2e1b181bfbb4511b0f89263c19ef3c04cf212a8590ec2847812R200.

The PR also supports starting a server in a root that is a child of active config roots (in that case it is a matter of expanding one of the parent roots until it). This only happens if an active root was a parent out of the monorepo. If it isn't a child of any active config root, then we create a new direntCache (and expand it to connect it to any active config root that is a child of it).

@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci

This comment was marked as outdated.

@patak-dev patak-dev marked this pull request as draft January 27, 2024 05:42
@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci

This comment was marked as outdated.

@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci

This comment was marked as outdated.

@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci

This comment was marked as outdated.

@patak-dev
Copy link
Member Author

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

📝 Ran ecosystem CI on e11de80: Open

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

@patak-dev
Copy link
Member Author

astro, marko, and vitepress are failing with this PR. Even with its simplest form of only sharing the cache if the root is exactly the same for a config (that it would still be good given that we now use the workspace root for each config).
I don't yet understand why. What I could think is that now we have two watchers over the same cache and both are going to try to add or remove files, but I don't see how this could affect things here. Ideas welcome if someone is curious about what's going on here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants