Skip to content
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

add support for comparing various .NET Runtimes performance #15387

Merged
merged 6 commits into from
May 14, 2021

Conversation

adamsitnik
Copy link
Contributor

PR Summary

This PR adds support for comparing the performance of various .NET Runtimes.

The users can specify a single TargetFramework using dotnet run or Start-Benchmarking:

Start-Benchmarking -Filter *using* -TargetFramework net5.0
dotnet run -c Release -f net5.0 --filter *using*

image

Or multiple target framework monikers using dotnet run:

dotnet run -c Release --filter Engine.Parsing.UsingStatement -f netcoreapp2.1 --runtimes netcoreapp2.1 net5.0 net6.0

image

For more information please refer to BenchmarkDotNet docs

PR Context

dotnet/sdk#17013

To get it working I had to:

  • clear TargetFramework as Test.Common.props sets TargetFramework to net6.0. This was mandatory to be able to target multiple TFMs.
  • specify all package references in explicit way. It seems to be some kind of SDK/MSBuild limitation. If the project references project A which references project B and we want to reference their packages for different TFMs, we have to explicitly reference package A and B. Referencing just A gives errors about missing B.

PR Checklist

@daxian-dbw @adityapatwardhan PTAL

@daxian-dbw
Copy link
Member

daxian-dbw commented May 12, 2021

@adamsitnik Thanks for submitting a PR!

I played with your changes, and it seems to break the ability to target a specific version of Microsoft.PowerShell.SDK. For example:

PS: benchmarks> $env:PERF_TARGET_VERSION = '7.2.0-preview.5'
PS: benchmarks> dotnet run -c release -f net6.0 --filter *using* --keepFiles

C:\arena\source\PowerShell\test\perf\benchmarks\powershell-perf.csproj : error NU1202: Package Microsoft.PowerShell.Commands.Diagnostics 7.2.0-preview.5 is not compatible with netcoreapp3.1 (.NETCoreApp,Version=v3.1). Package Microsoft.PowerShell.Commands.Diagnostics 7.2.0-preview.5 supports: net6.0 (.NETCoreApp,Version=v6.0)
C:\arena\source\PowerShell\test\perf\benchmarks\powershell-perf.csproj : error NU1202: Package Microsoft.PowerShell.Commands.Management 7.2.0-preview.5 is not compatible with netcoreapp3.1 (.NETCoreApp,Version=v3.1). Package Microsoft.PowerShell.Commands.Management 7.2.0-preview.5 supports: net6.0 (.NETCoreApp,Version=v6.0)
C:\arena\source\PowerShell\test\perf\benchmarks\powershell-perf.csproj : error NU1202: Package Microsoft.PowerShell.Commands.Utility 7.2.0-preview.5 is not compatible with netcoreapp3.1 (.NETCoreApp,Version=v3.1). Package Microsoft.PowerShell.Commands.Utility 7.2.0-preview.5 supports: net6.0 (.NETCoreApp,Version=v6.0)

...

In this case, 7.2.0-preview.5 is a preview version built against net6.0.
I tried to make it work but my efforts didn't go anywhere. Tried --envVars PERF_TARGET_VERSION:7.2.0-preview.5 but it turned out the environment variable specified this way doesn't affect the build phase of the auto-generated project.

To us, the ability to compare against different PowerShell SDK is more important than running benchmarks against different .NET runtimes. To some extent, running benchmarks with different PowerShell SDKs targeting the same net6.0 runtime help us to eliminate the runtime difference, and only comparing with the changes in PowerShell.

BTW, the building of the auto-generated project is still biting us because we cannot specify --no-dependencies for the first try

// start dotnet build -c Release /p:DebugType=portable --no-restore /p:UseSharedCompilation=false /p:BuildInParallel=false /m:1 /p:Deterministic=true /p:Optimize=true in C:\arena\source\PowerShell\test\perf\benchmarks\bin\release\net6.0\Job-DYFHWE
// command took 58.65s and exited with 1

I think it makes sense to try --no-dependencies first, and fall back to a full build if that failed. I can submit a PR to BenchmarkDotNet to make that change if you agree that's the way to go.

@adamsitnik
Copy link
Contributor Author

I played with your changes, and it seems to break the ability to target a specific version

Great catch, I've fixed that.

In order to verify which version is being benchmarked I've added a setup method that just prints assembly version:

[GlobalSetup]
public void PrintAssemblyVersion() => Console.WriteLine(typeof(Parser).Assembly.FullName);

And run it without env var set:

dotnet run -c release -f net6.0 --filter *using*
System.Management.Automation, Version=7.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

With env var set:

$env:PERF_TARGET_VERSION = '7.2.0-preview.5'
dotnet run -c release -f net6.0 --filter *using*
System.Management.Automation, Version=7.2.0.5, Culture=neutral, PublicKeyToken=31bf3856ad364e35

And with env var not set, but against multiple TFMs:

$env:PERF_TARGET_VERSION = ''
dotnet run -c release -f net6.0 --filter *using* --runtimes netcoreapp2.1 net5.0 net6.0
// Runtime=.NET 5.0.6 (5.0.621.22011), X64 RyuJIT
System.Management.Automation, Version=7.1.3.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

// Runtime=.NET 6.0.0 (6.0.21.21801), X64 RyuJIT
System.Management.Automation, Version=7.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

// Runtime=.NET Core 2.1.28 (CoreCLR 4.6.30015.01, CoreFX 4.6.30015.01), X64 RyuJIT
System.Management.Automation, Version=6.2.7.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35

…d it triggers full build of all dependencies in the benchmarks project
@adamsitnik
Copy link
Contributor Author

the building of the auto-generated project is still biting us

I've noticed that dotnet run was taking a LOT of time to start as well. It turned out that if we build PowerShell.sln in Release and then try to build any project in release (change of the capitalization of the first letter) MSBuild recognizes this as two different configurations and rebuilds all dependencies. The fix is to always use the same config name. I've updated the script and README.md for that.

cd C:\Projects\PowerShell\
Start-PSBuild -Clean -Configuration Release
cd test\perf\benchmark
dotnet build -c release -f net6.0
(omitted for brevity)
Time Elapsed 00:00:21.15
cd C:\Projects\PowerShell\
Start-PSBuild -Clean -Configuration Release
cd test\perf\benchmark
dotnet build -c Release -f net6.0
(omitted for brevity)
Time Elapsed 00:00:04.49

@daxian-dbw
Copy link
Member

Thanks for the quick turnaround. I wasn't aware that the MSBuild is case sensitive :) I will review again today.

@daxian-dbw
Copy link
Member

daxian-dbw commented May 13, 2021

@adamsitnik Your changes look good! <TargetFramework Condition="'$(PerfTargetVersion)' == ''"></TargetFramework> is a smart way to solve the "using specific SDK version" scenario, and I definitely learnt something new!

I made some updates to your changes mainly in 3 aspects:

  1. use netcoreapp3.1 and the corresponding 7.0.6 version PS sdk, because the 6.x versions were out of life cycle.
  2. add more comments.
  3. update the Start-Benchmarking function to support -Runtime and add validations.

Please take a look at my changes and let me know if I did anything wrong. Thanks!

@adamsitnik
Copy link
Contributor Author

Please take a look at my changes and let me know if I did anything wrong

@daxian-dbw your changes look good to me, I've also tested it locally and everything works as expected :shipit:

@daxian-dbw daxian-dbw merged commit 5e91723 into PowerShell:master May 14, 2021
@daxian-dbw
Copy link
Member

@adamsitnik Thank you again for the help!

@adamsitnik adamsitnik deleted the benchmarkDifferentRuntimes branch May 14, 2021 18:49
@daxian-dbw daxian-dbw added the CL-Test Indicates that a PR should be marked as a test change in the Change Log label May 26, 2021
@ghost
Copy link

ghost commented May 27, 2021

🎉v7.2.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Test Indicates that a PR should be marked as a test change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants