Implement Project Parallelism in IlSpyCmd (Stacked PR #5)#3
Implement Project Parallelism in IlSpyCmd (Stacked PR #5)#3petercrabtree wants to merge 6 commits into
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
7bd6d02 to
580e4af
Compare
ea22eca to
67039dc
Compare
580e4af to
d46aad6
Compare
ac368ed to
960474c
Compare
|
Given the big file changes - crlf issues or other reformatting going on? |
| <PackageVersion Include="NuGet.Protocol" Version="6.13.2" /> | ||
| <PackageVersion Include="PowerShellStandard.Library" Version="5.1.1" /> | ||
| <PackageVersion Include="Shouldly" Version="4.3.0" /> | ||
| <PackageVersion Include="Spectre.Console" Version="0.49.1" /> |
There was a problem hiding this comment.
Note: that then needs an entry in https://github.com/icsharpcode/ILSpy/blob/master/doc/third-party-notices.txt
There was a problem hiding this comment.
Added, thanks for catching that.
c12d851 to
bf0b7cb
Compare
bf0b7cb to
51a789f
Compare
55ae0eb to
750b402
Compare
a735af4 to
a777ed3
Compare
750b402 to
d0d159e
Compare
a777ed3 to
0e8a76a
Compare
d0d159e to
610123b
Compare
0e8a76a to
4128336
Compare
610123b to
6d984e9
Compare
f9c7b73 to
e93d99d
Compare
6d984e9 to
aebe02a
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements project parallelism in IlSpyCmd, enabling concurrent decompilation of multiple assemblies with progress tracking and enhanced user experience.
- Adds parallel processing support for decompiling multiple assemblies into a solution using configurable job limits
- Integrates Spectre.Console for rich progress visualization with individual task tracking and overall progress indication
- Improves performance through concurrent execution and optimized garbage collection settings
Reviewed Changes
Copilot reviewed 7 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| doc/third-party-notices.txt | Adds MIT license notice for Spectre.Console dependency |
| ILSpy.sln.DotSettings | Updates ReSharper settings with migration flags |
| ICSharpCode.ILSpyCmd/packages.lock.json | Locks new package versions for Spectre.Console and System.Threading.Channels |
| ICSharpCode.ILSpyCmd/IlspyCmdProgram.cs | Main implementation of parallel decompilation with progress tracking |
| ICSharpCode.ILSpyCmd/ICSharpCode.ILSpyCmd.csproj | Adds package references and enables server GC with adaptation mode |
| ICSharpCode.Decompiler/Solution/SolutionCreator.cs | Adds exception handling around project reference fixing |
| Directory.Packages.props | Defines package versions for new dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| Decompile multiple assemblies into a single solution, using all available cores. | ||
| ilspycmd -p -o c:\decompiled -j 0 sample.dll sample2.dll sample3.dll sample4.dll | ||
| (or with max of 4 parallel jobs) |
There was a problem hiding this comment.
The indentation uses tabs instead of spaces, which is inconsistent with the rest of the codebase that uses spaces.
| (or with max of 4 parallel jobs) | |
| (or with max of 4 parallel jobs) |
| double? _fractionDoneNow = value.TotalUnits == 0 ? null : (double)value.UnitsCompleted / value.TotalUnits; | ||
| var fractionDoneNow = _fractionDoneNow.Value; |
There was a problem hiding this comment.
Accessing .Value on a nullable double without null checking will throw an exception when value.TotalUnits == 0. The variable should be checked for null before accessing its value.
| double? _fractionDoneNow = value.TotalUnits == 0 ? null : (double)value.UnitsCompleted / value.TotalUnits; | |
| var fractionDoneNow = _fractionDoneNow.Value; | |
| double fractionDoneNow = (double)value.UnitsCompleted / value.TotalUnits; |
| static async ValueTask DecompileProjectAsync( | ||
| (string name, ProgressTask task, DecompilerSettings settings, string[] referencePaths, (bool IsSet, string Value) inputPDBFile, ConcurrentBag<ProjectItem> projects, string outputDirectory, ProgressTask projectTrackingTask) projectContext, | ||
| CancellationToken token, | ||
| IProgress<DecompilationProgress> progress) | ||
| { | ||
| var (file, progressTask, settings, referencePaths, inputPDBFile, projects, outputDirectory, projectTrackingTask) = projectContext; |
There was a problem hiding this comment.
The tuple parameter with 8 elements is difficult to read and maintain. Consider creating a dedicated struct or class to encapsulate these parameters.
| static async ValueTask DecompileProjectAsync( | |
| (string name, ProgressTask task, DecompilerSettings settings, string[] referencePaths, (bool IsSet, string Value) inputPDBFile, ConcurrentBag<ProjectItem> projects, string outputDirectory, ProgressTask projectTrackingTask) projectContext, | |
| CancellationToken token, | |
| IProgress<DecompilationProgress> progress) | |
| { | |
| var (file, progressTask, settings, referencePaths, inputPDBFile, projects, outputDirectory, projectTrackingTask) = projectContext; | |
| class ProjectContext | |
| { | |
| public string Name { get; } | |
| public ProgressTask Task { get; } | |
| public DecompilerSettings Settings { get; } | |
| public string[] ReferencePaths { get; } | |
| public (bool IsSet, string Value) InputPDBFile { get; } | |
| public ConcurrentBag<ProjectItem> Projects { get; } | |
| public string OutputDirectory { get; } | |
| public ProgressTask ProjectTrackingTask { get; } | |
| public ProjectContext( | |
| string name, | |
| ProgressTask task, | |
| DecompilerSettings settings, | |
| string[] referencePaths, | |
| (bool IsSet, string Value) inputPDBFile, | |
| ConcurrentBag<ProjectItem> projects, | |
| string outputDirectory, | |
| ProgressTask projectTrackingTask) | |
| { | |
| Name = name; | |
| Task = task; | |
| Settings = settings; | |
| ReferencePaths = referencePaths; | |
| InputPDBFile = inputPDBFile; | |
| Projects = projects; | |
| OutputDirectory = outputDirectory; | |
| ProjectTrackingTask = projectTrackingTask; | |
| } | |
| } | |
| static async ValueTask DecompileProjectAsync( | |
| ProjectContext projectContext, | |
| CancellationToken token, | |
| IProgress<DecompilationProgress> progress) | |
| { | |
| var file = projectContext.Name; | |
| var progressTask = projectContext.Task; | |
| var settings = projectContext.Settings; | |
| var referencePaths = projectContext.ReferencePaths; | |
| var inputPDBFile = projectContext.InputPDBFile; | |
| var projects = projectContext.Projects; | |
| var outputDirectory = projectContext.OutputDirectory; | |
| var projectTrackingTask = projectContext.ProjectTrackingTask; |
| projectId.Guid, | ||
| projectId.TypeGuid)); | ||
|
|
||
| progressTask.StopTask(); // remove from the progress display |
There was a problem hiding this comment.
The progress task is stopped twice - once at line 629 after the parallel loop and again here at line 675. This could cause issues with the progress display.
| progressTask.StopTask(); // remove from the progress display | |
| // progressTask.StopTask(); // removed: already stopped after the parallel loop |
Yes, this happens to be one of the roughest parts of using jj for source control, no autocrlf. Apologies for the long delay getting it fixed. You should be able to effectively see the actual changes now. @christophwille |
|
Note that this code is still somewhat rough (e.g. the review comments from copilot); I'm putting it here to give y'all an early peek, and to ensure it'll display correctly when I open a PR upstream. |
b091a3f to
7f9cc0b
Compare
e93d99d to
6507523
Compare
7f9cc0b to
dca1a8b
Compare
8add1e2 to
3feb86b
Compare
07922a9 to
1b2f04a
Compare
3feb86b to
0d4a5c9
Compare
1b2f04a to
cf305a6
Compare
f4b15c1 to
2251205
Compare
cf305a6 to
137f93e
Compare
Adds -j/--jobs option to control maximum degree of parallelism for project decompilation. Uses Parallel.ForEach with ConcurrentBag to decompile multiple assemblies concurrently. Defaults to single-threaded but allows 0 for Environment.ProcessorCount.
Adds Spectre.Console progress bars for visual feedback during solution decompilation. Changes DecompileAsSolution to async DecompileAsSolutionAsync with progress tracking. Adds better error handling with try/catch blocks in SolutionCreator. style: Fix whitespace formatting Adjusts indentation to follow consistent code style guidelines.
Enhances progress tracking by passing IProgress<DecompilationProgress> to individual project decompilation operations, providing more granular feedback (progress on a project, not just overall progress).
Refines progress reporting in ILSpyCmd to provide more accurate and thread-safe updates: - Add locking around progress callbacks to ensure thread safety - Only increase overall progress (never decrease) to handle out-of-order callbacks - Remove redundant progress increments and improve numerical stability - Set ProgressIndicator on decompiler instead of passing as parameter - Add elapsed time to ILSpyCmd progress Display - Add progress reporting to solution creation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2251205 to
1ae7be9
Compare
137f93e to
8103b3e
Compare
No description provided.