Multithread processing source files #243
Conversation
|
@allevato I just noticed that this project isn't setup to require CI checks to pass before merging. Is this something you'd consider implementing for the swift-format repository? |
|
We've submitted changes to checkout @shahmishal , can you update us on the current status of swift-format on Swift CI? Is there anything else we need to do on our end or does your team need to enable something for this to work? |
|
This looks amazing, thank you @keith. I think the new |
|
Done! |
| diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to open \(path)")) | ||
| continue | ||
| if parallel { | ||
| let allFilePaths = Array(FileIterator(paths: paths)) |
There was a problem hiding this comment.
Rambling: I still have some concerns about the upfront flattening of the FileIterator being slow if the formatter is run in parallel on something like a networked file system or in recursive mode on the root of a large monorepo (who would do such a thing), but I should probably test my concerns against real data before going to the trouble of adding more work.
If we need to change this in the future, I was concerned about having to do our own thread pooling, but maybe there's an easy alternative here: instead of flattening the whole iterator at once, just pull off a both sufficiently large and sufficiently small number of files at a time (say, 200, or a nice round number like 256) and loop over the batches while processing each batch with concurrentPerform. That would probably still get us good parallelism in the batches while avoiding eagerly flattening the whole array.
You don't need to make this change now (I might be worried about the performance for nothing), but I just wanted to write it out here so I don't forget it later if we do need it.
There was a problem hiding this comment.
+1 good thoughts, I would definitely be interested to hear if you see issues with this given the large number of files.
Previously every source file was formatted / linted one by one. On our codebase this took a full project format from ~17 minutes to ~5 minutes.
keith
force-pushed
the
ks/multithread-processing-source-files
branch
from
1b81a28 to
e2b29d5
Compare
|
Thanks for your work on this, and your patience while we sorted out various other related issues. Let's merge it finally 🎉 |
|
Thanks! |
…swiftlang#243) * Add the possibility to test homebrew's swift-doc in the e2e tests. * Add an GitHub action to make sure we don't break the homebrew version. * Apply suggestions from code review * Rename swiftDocCommand helper Pass swiftDocCommand directly to Process.run Co-authored-by: Mattt <mattt@me.com>
Previously every source file was formatted / linted one by one. On our
codebase this took a full project format from ~17 minutes to ~5 minutes.
This is an updated version of #117
Depends on: