Process input files in parallel #26
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
Signed-off-by: ricekot <[email protected]>
DavidBakerEffendi
requested changes
Nov 8, 2024
| @@ -218,6 +211,8 @@ private static void _SummaryForDLLFile(FileInfo filePath, string? exclusionRegex | |||
| return; | |||
| } | |||
|
|
|||
| _logger?.LogInformation("Parsing file {fileName}", fullPath); | |||
There was a problem hiding this comment.
What is the reason for moving this? Would this not end up logging all types of files?
There was a problem hiding this comment.
That's the existing behaviour too I believe, see line 192 which logs the file name in the loop.
https://github.com/joernio/DotNetAstGen/pull/26/files#diff-600f8abdf835e81e081c02e55f3bed18ecb1fdd2c80333c77184fefc67c6ad92L192
There was a problem hiding this comment.
Ah I see the looping has changed, my bad. Looks all good to me then
DavidBakerEffendi
approved these changes
Nov 8, 2024
| @@ -218,6 +211,8 @@ private static void _SummaryForDLLFile(FileInfo filePath, string? exclusionRegex | |||
| return; | |||
| } | |||
|
|
|||
| _logger?.LogInformation("Parsing file {fileName}", fullPath); | |||
There was a problem hiding this comment.
Ah I see the looping has changed, my bad. Looks all good to me then
DavidBakerEffendi
added a commit
to joernio/joern
that referenced
this pull request
Nov 10, 2024
Uses new version of `DotNetAstGen` that uses a parallelized version. Credits to @ricekot joernio/DotNetAstGen#26
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.
Process input source files in parallel to reduce the total AST generation time. The time taken to generate an AST is cut down by more than half with parallel processing.
About the changes
A PLINQ query is used to parallelize the
foreachloops that iterate over files in the input directory one by one.Since the
AsParallelmethod is called on anIEnumerable<FileInfo>, PLINQ should opt for chunk partitioning (dynamic batch sizes) by default, as opposed to a range partitioning (fixed batch sizes) approach as suggested in #11.Refs:
Benchmark
A fresh clone of the bitwarden/server repo was created locally (at commit
4adcecb).Then, a scan was run against the cloned repo twice, once from the
mainbranch of this repo and once with the code changes in this PR.time dotnet run --project DotNetAstGen/ -i /tmp/bitwarden-server -o /tmp/bitwarden-server-ast/The results of the scans are added below:
Without the changes from this PR (sequential processing):
With the changes from this PR (parallel processing)
Thus, AST generation with parallel processing was 48.69 seconds or about 55% faster.
I did multiple runs and the result was similar each time.
For reference these runs were done on a machine running macOS 15.0.1 with Apple M3 Pro and 36GiB RAM.