Improve performance of CLI when passing in files/folders - #1893
Conversation
|
I finally got around to refactoring CommandLineFormatter this weekend which caused conflicts with the changes you made. Some of the logic got moved into FormattingEngine. Could you resolve the conflicts and then I can take a look? As for tests, I don't know that it is possible to test the performance, but if there aren't already some tests that pass in multiple file paths that would be good to add. |
ea9195c to
1f70f0d
Compare
|
I have rebased the change onto latest in main. The change is a lot more spread out after the cli refactor. The biggest complicating factor is still the FormattingEngine now has a method called CommandLineFormatter now checks if file paths exist and if msbuild versions are correct before starting formatting. It also only creates one OptionsProvider, FormattingCache, and FormattingEngine per cli run instead of one per file/folder. I also added some tests for having multiple editorconfigs, testing the ignore-path option with multiple root directories passed in. FormattingCache now uses PrinterOptions hash instead of OptionsProvider since PrinterOptions defines all the evaluated configurations (indent, line endings, xml spacing, ...) and doesn't include extra information such as ignore file or directory mappings that don't effect how files are formatted. |
Only create one instance of OptionsProvider and FormattingCache per CLI run instead of creating one per file that is passed in.
Description
This PR improves the performance of running csharpier on the command line when passing in a list of files. When using a pre-commit hook it is common to pass in the list of staged files to csharpier to have the files formatted before committing. Currently this process is very slow when a large number of files are passed in. For example when testing this on the csharpier repo if every file was passed in on the command line it would take 45seconds to format. This slowdown is mainly caused by each file or folder creating its own instance of OptionsProvider which has to recalculate ignore file and config file per file.
To improve the performance this PR moves the OptionsProvider and FormattingCache outside of the file/directory loop then adds each file to a
List<Task>that gets run usingTask.WhenAll(). The one exception to this is when using--ignore-pathsince it needs a new OptionsProvider for every single directory that is passed in since when parsing the ignore file from--ignore-pathcli option it is based off the location of the directory not the ignore file itself. This is unlike prettier's--ignore-pathor git .gitignore whose ignore file are always based off the ignore file location and not the location of passed in file or directory. I can open another PR to change--ignore-pathto always base file paths from the location of the ignore file instead to simply this logic and improve performance even when using--ignore-path.Some benchmarks when testing this using cli to format all files in this repo by passing them as cli arguments.
If you think of a good way to test this I can add some tests. I was thinking of including a benchmark that used all cs files in the repo itself or creating a bunch of test cs files and using them.
Related Issue
Checklist
varthis.