-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Description
❤️ 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 invocationsfs.existsSync
: 100K invocationsfs.readFile
: 55K invocationsfs.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.
- 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.
- 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?