fix: reduce number of fs.realpath calls

[aleclarson]

Description

#6030 reported a performance regression related to fs.realpath calls, so I've implemented a cache for symlink resolution. We need some people to test this on bigger projects where the regression is most noticeable. 👀

Additional context

Use this plugin to inspect the effectiveness of this approach:

export function debugSymlinkResolver(): vite.Plugin {
  return {
    name: 'debugSymlinkResolver',
    configResolved(config) {
      const { symlinkResolver } = config
      this.generateBundle = () => {
        console.log('cacheSize: %O', symlinkResolver.cacheSize)
        console.log('cacheHits: %O', symlinkResolver.cacheHits)
        console.log('fsCalls:   %O', symlinkResolver.fsCalls)
      }
    },
  }
}

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[aleclarson]

Just rebased onto main

[bluwy]

Are there notes on how caching realpath calls improves perf? Looks like node also implements it's own cache, so I'm curious how another layer of cache helps with this.

[aleclarson]

Looks like node also implements it's own cache, so I'm curious how another layer of cache helps with this.

We are using realpathSync.native here, which doesn't have a caching mechanism. The native realpath is still faster than cached realpathSync by "a few orders of magnitude" according to this Node.js commit.

Are there notes on how caching realpath calls improves perf?

I'll add some, but the basic idea is to avoid I/O calls as much as possible.

[aleclarson]

@Lani stated in this comment:

Preserving symlinks doubled the speed!

He's referring to the preserveSymlinks option (which avoids realpath calls entirely) and its effect on vite dev startup time. So it seems that avoiding I/O can have real tangible benefits to the user.

[bluwy]

Thanks for the explaination. If fs.realpath does gives us caching ootb, and fs.realpath.native does not, I wonder if we should use fs.realpath directly for its cache. Even with it's slightly slower perf, we can avoid another layer of cache, and avoid the inconsistency issues with .native (nodejs/node#7899).

I feel like I'm missing a history in nodejs of why .native isn't supported in fs/promises too. (Are they not recommending it?) Trying to understand the value of another caching layer as it would be something to maintain in the long run.

[aleclarson]

Even with it's slightly slower perf, we can avoid another layer of cache, and avoid the inconsistency issues with .native (nodejs/node#7899).

Maybe a config option to disable realpathSync.native use is warranted? The issue you linked is from 2016, and it's been working perfectly for me on macOS.

Trying to understand the value of another caching layer as it would be something to maintain in the long run.

I think this will be relatively painless to maintain, because it isn't doing anything that crazy or unsafe, and we've got some good tests for it already (as well as verbose logs for easier debugging). Though, I'd be okay with publishing it as a separate package that Vite depends on.

[bluwy]

I feel like I'm missing a history in nodejs of why .native isn't supported in fs/promises too. (Are they not recommending it?) Trying to understand the value of another caching layer as it would be something to maintain in the long run.

I've found out that fs/promises doesn't have .native because fsp.realpath uses .native by default, which is an interesting decision from node.

[patak-dev]

With the changes in these past weeks for 4.3, it isn't clear that a cache at this level is needed. I think it is an interesting idea though, and maybe we should revisit this PR in the future. Let's close this for now, and we can re-evaluate for Vite 5.