-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve performance of scanning source files #15270
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RobinMalfait
changed the title
Improve scanning files in parallel
Improve performance of scanning source files
Dec 2, 2024
All the files are already covered when running the initial build. This is for subsequent builds (in dev/watch mode). This also prevents to do some work twice and just do it once.
The `part_sort` isn't a super big difference, however it goes from ~3ms to ~1ms to do the sorting.
We already know that we can't have whitespace in our candidates which means that we can operate on many many small chunks instead. This change means that we run a lot of extractors on very small pieces of code all in parallel, instead of running the extractor per file. In my testing, this results in 2x speedup (~800ms -> ~400ms on a big project with over a 1000 files).
To reduce overhead of the Extractor itself, we can chunk the work by lines instead of every whitespace-separated chunk. This seems to improve the overall cost even more! Co-authored-by: Jordan Pittman <jordan@cryptica.me>
RobinMalfait
force-pushed
the
feat/scanner-performance
branch
from
December 2, 2024 16:44
d273a39
to
8fe3977
Compare
RobinMalfait
force-pushed
the
feat/scanner-performance
branch
from
December 2, 2024 17:01
35f40ca
to
abd6c8f
Compare
adamwathan
reviewed
Dec 2, 2024
thecrypticace
approved these changes
Dec 2, 2024
adamwathan
approved these changes
Dec 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves scanning files by scanning chunks of the files in parallel. Each chunk is separated by new lines since we can't use whitespace in classes anyway.
This also means that we can use the power of your CPU to scan files faster. The extractor itself also has less state to worry about on these smaller chunks.
On a dedicated benchmark machine: Mac Mini, M1, 16 GB RAM
On a more powerful machine, MacBook Pro M1 Max, 64 GB RAM, the results look even more promising:
These benchmarks are running on our Tailwind UI project where we have >1000 files, and >750 000 lines of code in those files.
I am sure there is more we can do here, because reading all of these 1000 files only takes ~10ms, whereas parsing all these files takes ~180ms. But I'm still happy with these results as an incremental improvement.
For good measure, I also wanted to make sure that we didn't regress on smaller projects. Running this on Catalyst, we only have to deal with ~100 files and ~18 000 lines of code. In this case reading all the files takes ~890µs and parsing takes about ~4ms.
Not a huge difference, still better and definitely no regressions which sounds like a win to me.
Edit: after talking to @thecrypticace, instead of splitting on any whitespace we just split on newlines. This makes the chunks a bit larger, but it reduces the overhead of the extractor itself. This now results in a 2.45x speedup in Tailwind UI compared to 1.94x speedup.