C#: Avoid explicitly restoring projects in solution files. #14111

michaelnebel · 2023-08-31T11:30:56Z

In this PR we do the following performance optimisations to the dependency manager.

If solution file(s) are restored then we don't explictly restore projects that are up to date with respect to dotnet restore (after restoring the solution).
dotnet restore on projects are executed in parallel.

A couple of implementation details

We need to change the dotnet restore verbosity level to normal when restoring solutions to get all the relevant information related to restored projects.
The DotNet.cs file is starting to contain business logic. In C#: Re-factor Dotnet.cs to enable unit testing. #14142 we re-factor DotNet.cs to enable unit testing.

The changes have been tested on OrchardCMS/OrchardCore as it contains a solution file.
The numbers below show that we get the same accuracy for the comparison queries, but the wall clock time drops from approximately 16m to 9m (on my machine).

On main we have:

Results for OrchardCMS/OrchardCore
Query Files.ql:
Result count - both/traced/standalone: 4151/3692/20
Query Declarations.ql:
Result count - both/traced/standalone: 69128/84740/113
Result count in the matching 4151 files - both/traced/standalone: 69128/0/0
Matching 100.0%
Query Dependencies.ql:
Result count - both/traced/standalone: 491/2/229
Query CallTargets.ql:
Result count - both/traced/standalone: 87430/264261/213
Result count in the matching 4151 files - both/traced/standalone: 87430/1523/154
Matching 98.12%
Query TypeHierarchy.ql:
Result count - both/traced/standalone: 6351/4027/23
Result count in the matching 4151 files - both/traced/standalone: 6351/7/5
Matching 99.81%
Query Expressions.ql:
Result count - both/traced/standalone: 311660/1070097/1049
Result count in the matching 4151 files - both/traced/standalone: 311660/6370/870
Matching 97.73%
Query VariableAccesses.ql:
Result count - both/traced/standalone: 86270/193815/39
Result count in the matching 4151 files - both/traced/standalone: 86270/363/0
Matching 99.58%
Query TypeMentions.ql:
Result count - both/traced/standalone: 86709/131893/396
Result count in the matching 4151 files - both/traced/standalone: 86709/1439/319
Matching 98.01%

python3 diff_standalone.py 1510.23s user 299.76s system 188% cpu 15:58.18 total

On the feature branch including the unit test re-factor we have

Results for OrchardCMS/OrchardCore
Query Files.ql:
Result count - both/traced/standalone: 4151/3692/20
Query Declarations.ql:
Result count - both/traced/standalone: 69128/84740/113
Result count in the matching 4151 files - both/traced/standalone: 69128/0/0
Matching 100.0%
Query Dependencies.ql:
Result count - both/traced/standalone: 491/2/229
Query CallTargets.ql:
Result count - both/traced/standalone: 87439/264252/206
Result count in the matching 4151 files - both/traced/standalone: 87439/1514/147
Matching 98.14%
Query TypeHierarchy.ql:
Result count - both/traced/standalone: 6351/4027/23
Result count in the matching 4151 files - both/traced/standalone: 6351/7/5
Matching 99.81%
Query Expressions.ql:
Result count - both/traced/standalone: 311657/1070100/1056
Result count in the matching 4151 files - both/traced/standalone: 311657/6373/877
Matching 97.73%
Query VariableAccesses.ql:
Result count - both/traced/standalone: 86271/193814/39
Result count in the matching 4151 files - both/traced/standalone: 86271/362/0
Matching 99.58%
Query TypeMentions.ql:
Result count - both/traced/standalone: 86709/131893/396
Result count in the matching 4151 files - both/traced/standalone: 86709/1439/319
Matching 98.01%

python3 diff_standalone.py 1107.11s user 143.95s system 234% cpu 8:53.80 total

csharp/extractor/Semmle.Extraction.Tests/DotNet.cs

…files.

tamasvajk

Added some questions and suggestions. Have you checked how the performance changes simply with using multiple threads for the restores?

tamasvajk · 2023-09-05T13:42:49Z

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs

        }

        public bool Exec(string execArgs)
        {
            var args = $"exec {execArgs}";
            return RunCommand(args);
        }
+
+        [GeneratedRegex("Restored\\s+(.+\\.csproj)", RegexOptions.Compiled)]


Is the output of dotnet restore always English?

Uh, that is an excellent question!
I have tried to set the CLI UI language to german and verified that it prints help in german (so it does respect setting the CLI language). The output of dotnet restore is still in english.
Do you have any other suggestions?

It seems to be translated to German: https://github.com/NuGet/NuGet.Build.Localization/blob/c65073ce9be5c699d7b1cbee913223b77988f6ad/localize/DEU/15/NuGet.Commands.dll.lcl#L1459

DOTNET_CLI_UI_LANGUAGE=de dotnet restore prints the message in German. So maybe we need to set DOTNET_CLI_UI_LANGUAGE=en before running the command. Also, nuget.exe seems to have an option named -ForceEnglishOutput. I don't know if this could be passed to the dotnet CLI to forward it to nuget.

tamasvajk · 2023-09-05T13:47:11Z

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DotNet.cs

+                return true;
+            }
+
+            projects = new List<string>();


It's probably enough to declare projects with type IEnumerable<string> , and then this new List<string>() could be changed to Array.Empty<string>(), and the above .ToList() could also be removed.

tamasvajk · 2023-09-05T13:51:07Z

csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs

+                });
+
+        private void RestoreProjects(IEnumerable<string> projects) =>
+            Parallel.ForEach(projects, new ParallelOptions { MaxDegreeOfParallelism = options.Threads }, project => RestoreProject(project));


If we restore in parallel, I think the restore logs will overlap each other. Would it make sense to store the stdout of individual processes in a string and only printing it when the process is done?

Ah, yes we can risk that the output is interleaved. I will try to look into this.

michaelnebel · 2023-09-05T14:27:22Z

Added some questions and suggestions. Have you checked how the performance changes simply with using multiple threads for the restores?

No, I have not - I will try that 👍

michaelnebel · 2023-09-06T14:13:42Z

Added some questions and suggestions. Have you checked how the performance changes simply with using multiple threads for the restores?

No, I have not - I will try that 👍

Result for only executing project restore in parallel can be seen below.
It is a bit better than the original solution, but it would ideal, if we can also rely projects being restored by dotnet restore on the solution.

Execution time when restoring projects in parallel (without using info from restore of solution).

Results for OrchardCMS/OrchardCore
Query Files.ql:
Result count - both/traced/standalone: 4151/3692/20
Query Declarations.ql:
Result count - both/traced/standalone: 69128/84740/113
Result count in the matching 4151 files - both/traced/standalone: 69128/0/0
Matching 100.0%
Query Dependencies.ql:
Result count - both/traced/standalone: 491/2/234
Query CallTargets.ql:
Result count - both/traced/standalone: 87451/264240/195
Result count in the matching 4151 files - both/traced/standalone: 87451/1502/136
Matching 98.16%
Query TypeHierarchy.ql:
Result count - both/traced/standalone: 6353/4025/22
Result count in the matching 4151 files - both/traced/standalone: 6353/5/4
Matching 99.86%
Query Expressions.ql:
Result count - both/traced/standalone: 311668/1070089/1045
Result count in the matching 4151 files - both/traced/standalone: 311668/6362/866
Matching 97.73%
Query VariableAccesses.ql:
Result count - both/traced/standalone: 86271/193814/39
Result count in the matching 4151 files - both/traced/standalone: 86271/362/0
Matching 99.58%
Query TypeMentions.ql:
Result count - both/traced/standalone: 86710/131892/395
Result count in the matching 4151 files - both/traced/standalone: 86710/1438/318
Matching 98.02%

python3 diff_standalone.py 2123.84s user 421.09s system 289% cpu 14:38.71 total

github-actions bot added the C# label Aug 31, 2023

michaelnebel force-pushed the csharp/reduceprojectrestore branch 3 times, most recently from 1d86206 to f41830f Compare September 5, 2023 08:48

github-code-scanning bot found potential problems Sep 5, 2023

View reviewed changes

csharp/extractor/Semmle.Extraction.Tests/DotNet.cs Fixed Show fixed Hide fixed

csharp/extractor/Semmle.Extraction.Tests/DotNet.cs Fixed Show fixed Hide fixed

csharp/extractor/Semmle.Extraction.Tests/DotNet.cs Fixed Show fixed Hide fixed

C#: Avoid explicitly restoring the projects in the restored solution …

e256144

…files.

michaelnebel force-pushed the csharp/reduceprojectrestore branch from f41830f to e256144 Compare September 5, 2023 11:31

michaelnebel added the no-change-note-required This PR does not need a change note label Sep 5, 2023

michaelnebel mentioned this pull request Sep 5, 2023

C#: Re-factor Dotnet.cs to enable unit testing. #14142

Draft

michaelnebel marked this pull request as ready for review September 5, 2023 12:15

michaelnebel requested a review from a team as a code owner September 5, 2023 12:15

tamasvajk reviewed Sep 5, 2023

View reviewed changes

C#: Avoid explicitly restoring projects in solution files. #14111

C#: Avoid explicitly restoring projects in solution files. #14111

michaelnebel commented Aug 31, 2023 •

edited

tamasvajk left a comment

tamasvajk Sep 5, 2023

michaelnebel Sep 5, 2023

tamasvajk Sep 6, 2023

tamasvajk Sep 5, 2023

tamasvajk Sep 5, 2023

michaelnebel Sep 5, 2023

michaelnebel commented Sep 5, 2023

michaelnebel commented Sep 6, 2023 •

edited

C#: Avoid explicitly restoring projects in solution files. #14111

Are you sure you want to change the base?

C#: Avoid explicitly restoring projects in solution files. #14111

Conversation

michaelnebel commented Aug 31, 2023 • edited

On main we have:

On the feature branch including the unit test re-factor we have

tamasvajk left a comment

Choose a reason for hiding this comment

tamasvajk Sep 5, 2023

Choose a reason for hiding this comment

michaelnebel Sep 5, 2023

Choose a reason for hiding this comment

tamasvajk Sep 6, 2023

Choose a reason for hiding this comment

tamasvajk Sep 5, 2023

Choose a reason for hiding this comment

tamasvajk Sep 5, 2023

Choose a reason for hiding this comment

michaelnebel Sep 5, 2023

Choose a reason for hiding this comment

michaelnebel commented Sep 5, 2023

michaelnebel commented Sep 6, 2023 • edited

Execution time when restoring projects in parallel (without using info from restore of solution).

michaelnebel commented Aug 31, 2023 •

edited

michaelnebel commented Sep 6, 2023 •

edited