Skip to content

Improving config resolution performance #15653

@pmdartus

Description

@pmdartus

❤️ First of all, I would to say a big THANK YOU to all the maintainers of this great project! I use Prettier on a daily basis, and I have to say that greatly improved my DX. ❤️

After seeing the interest on Twitter (sorry on X...) for working on a faster alternative to Prettier written in Rust. I was wondering how much could we potentially improve the current Prettier implementation without having to do a full rewrite in Rust.

Observation

For this purpose, I decided to use the mui/material-ui project to benchmark Prettier. This project contains ~42K files and roughly 8500 files files that are currently formatted by Prettier. On my machine, it takes 84s to format this project with the latest version of the main branch (median over 5 invocations, caching disabled, OSX, Node v20.6.1).

After further investigation, it appears that a sizable amount of time is spent in getOptionsForFile. To validate this observation, I commented the getOptionsForFile call and inlined the project configuration. Formatting the entire material-ui project now takes 46s, which is a 45% improvement.

The fact that it takes 45% of the processing time to resolve file formatting options is highly suspicious. There are certainly some performance improvement opportunities here.

Digging deeper

As far as I understand, the getOptionsForFile essentially resolves and loads the prettier and editor config files in a given directory. Looking at this code it appears to me that Prettier could get a great performance boost from caching the configuration data between getOptionsForFile invocations.

To put this in perspective, I logged all the file system invocations when formatting the material-ui project, the option resolution logic issues:

  • fs.promises.access: 827K invocations
  • fs.existsSync: 100K invocations
  • fs.readFile: 55K invocations
  • fs.promises.readFile: 13K invocations

The option resolution logic is delegated to 2 libraries: lilconfig and editorconfig. Both libraries do essentially the same thing, traversing up the file system and looking for specific configuration files. Currently, the configuration lookup isn’t cached between invocations resulting in repeated file system calls.

The main issue here is that it is currently difficult for Prettier to cache the config resolution result between invocations:

  • Prettier recently moved cosmiconfig from lilconfig. Unlike the previous config resolution library, lilconfig doesn’t have cache support and it appears to be a non-goal for this project.
  • Prettier currently pins the editorconfig dependency to version 0.15.3 but caching support was only introduced in version 1.0.0. As far as I understand, bumping the version isn’t an option because Prettier can’t bundle WASM modules.

Misc observations:

  • findProjectRoot function also recursively traverse the file system upward looking for .git or .hg directories. Caching the result of its invocation would certainly yield good performance improvement. It would also be ideal to replace the synchronous file system lookup with an async one to avoid blocking the main thread, but I don’t expect major performance improvements as all the files are currently processed serially.
  • I think Fix argument passed to lilconfig.search() #15363 should be revived. As mentioned in the PR, invoking .search on the file itself results in unnecessary file system lookups. It should be possible to optimize this without introducing a breaking change by checking first if the filename references a directory or not.

Moving forward

I would be happy to make adequate changes, but I would love to get the maintainers’ feedback first before I invest time into this.

  1. I don’t understand the reason for migrating from cosmiconfig to lilconfig (PR). Is it an option to move back to cosmiconfig to leverage its caching capabilities? If not, I would then open an issue on lilconfig to determine if the maintainer is interested in adding caching support.
  2. I don’t see a straightforward way to add support for WASM to the build in order to bump the editorconfig dependency. We can either ask if the maintainer is willing to make the parser swappable to work around this. An alternative approach would be re-implement .editorconfig parsing logic in Prettier. The processing logic looks small enough not to add too much tech debt. Any thoughts on this?

Metadata

Metadata

Assignees

No one assigned

    Labels

    type:perfIssue with performance of Prettier

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions