Skip to content

Fix Start-Benchmarking to put TargetPSVersion and TargetFramework in separate parameter sets #15508

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.

Sign up for GitHub

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

Merged
anmenaga merged 1 commit into from
Jun 11, 2021
Merged

Fix Start-Benchmarking to put TargetPSVersion and TargetFramework in separate parameter sets #15508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions test/perf/benchmarks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ This folder contains micro benchmarks that test the performance of PowerShell En
You can run the benchmarks directly using `dotnet run` in this directory:
1. To run the benchmarks in Interactive Mode, where you will be asked which benchmark(s) to run:
```
dotnet run -c Release
dotnet run -c Release -f net6.0
```

2. To list all available benchmarks ([read more](https://github.com/dotnet/performance/blob/main/docs/benchmarkdotnet.md#Listing-the-Benchmarks)):
```
dotnet run -c Release --list [flat/tree]
dotnet run -c Release -f net6.0 --list [flat/tree]
```

3. To filter the benchmarks using a glob pattern applied to `namespace.typeName.methodName` ([read more](https://github.com/dotnet/performance/blob/main/docs/benchmarkdotnet.md#Filtering-the-Benchmarks)]):
Expand All @@ -44,7 +44,11 @@ You can run the benchmarks directly using `dotnet run` in this directory:

You can also use the function `Start-Benchmarking` from the module [`perf.psm1`](../perf.psm1) to run the benchmarks:
```powershell
Start-Benchmarking [[-TargetPSVersion] <string>] [[-List] <string>] [[-Filter] <string[]>] [[-Artifacts] <string>] [-KeepFiles] [<CommonParameters>]
Start-Benchmarking [-TargetFramework <string>] [-List <string>] [-Filter <string[]>] [-Artifacts <string>] [-KeepFiles] [<CommonParameters>]

Start-Benchmarking [-TargetPSVersion <string>] [-Filter <string[]>] [-Artifacts <string>] [-KeepFiles] [<CommonParameters>]

Start-Benchmarking -Runtime <string[]> [-Filter <string[]>] [-Artifacts <string>] [-KeepFiles] [<CommonParameters>]
```
Run `Get-Help Start-Benchmarking -Full` to see the description of each parameter.

Expand Down
66 changes: 30 additions & 36 deletions test/perf/perf.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,19 @@ function Start-Benchmarking
.PARAMETER KeepFiles
Indicates to keep all temporary files produced for running benchmarks.
#>
[CmdletBinding(DefaultParameterSetName = 'Default')]
[CmdletBinding(DefaultParameterSetName = 'TargetFramework')]
param(
[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'TargetPSVersion')]
[ValidatePattern(
'^7\.(0|1|2)\.\d+(-preview\.\d{1,2})?$',
Copy link
Collaborator

@vexx32 vexx32 Jun 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining the pattern in this way requires that it be updated for every new minor (and/or major) PSVersion. This seems like there should be a better way; is there not a way the function can check the supplied version against an automatically-determined list of possible versions?

Copy link
Member Author

@daxian-dbw daxian-dbw Jun 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the current validation is not optimal. In fact, not just here, the powershell-perf.csproj also requires manual updates when we move to a new PS version or new .NET runtime, because it hardcodes the supported .NET frameworks, as well as the latest PS version corresponding to a specific .NET framework. It would be nice if we can avoid the need of manual updates to both perf.psm1 and powershell-perf.csproj. Anyways, that should be a separate PR 😄

Copy link
Collaborator

@iSazonov iSazonov Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could move this in a json config file.

ErrorMessage = 'The package version is invalid or not supported')]
[string] $TargetPSVersion,

[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'TargetFramework')]
[ValidateSet('netcoreapp3.1', 'net5.0', 'net6.0')]
[string] $TargetFramework = 'net6.0',

[Parameter(ParameterSetName = 'Default')]
[Parameter(ParameterSetName = 'TargetFramework')]
[ValidateSet('flat', 'tree')]
[string] $List,

Expand All @@ -70,23 +70,6 @@ function Start-Benchmarking
Remove-Item -Path $Artifacts -Recurse -Force -ErrorAction Stop
}

if ($TargetPSVersion) {
## Validate the specified 'TargetPSVersion' and 'TargetFramework' are compatible.
$TargetFramework -match '\d.\d' > $null
$targetDotNetVersion = $Matches[0]

$minimalDotNetVersion = switch -Wildcard ($TargetPSVersion) {
'7.0.*' { '3.1' }
'7.1.*' { '5.0' }
'7.2.*' { '6.0' }
}

if ($targetDotNetVersion -lt $minimalDotNetVersion) {
$dotnetVer = $minimalDotNetVersion -eq '3.1' ? 'netcoreapp3.1' : "net$minimalDotNetVersion"
throw "The '$TargetPSVersion' version of 'Microsoft.PowerShell.SDK' requires '$dotnetVer' as the minimal target framework."
}
}

if ($Runtime) {
## Remove duplicate values.
$hash = [ordered]@{}
Expand All @@ -113,26 +96,35 @@ function Start-Benchmarking
if ($List) { $runArgs += '--list', $List }
if ($KeepFiles) { $runArgs += "--keepFiles" }

if ($PSCmdlet.ParameterSetName -eq 'Default') {
if ($TargetPSVersion) {
switch ($PSCmdlet.ParameterSetName) {
'TargetPSVersion' {
Write-Log -message "Run benchmarks targeting '$TargetFramework' and the 'Microsoft.PowerShell.SDK' version '$TargetPSVersion' ..."
$env:PERF_TARGET_VERSION = $TargetPSVersion

## Use 'Release' instead of 'release' (note the capital case) because BDN uses 'Release' when building the auto-generated
## project, and MSBuild somehow recognizes 'release' and 'Release' as two different configurations and thus will rebuild
## all dependencies unnecessarily.
dotnet run -c Release -f $TargetFramework $runArgs
}
elseif ($TargetFramework -eq 'net6.0') {
Write-Log -message "Run benchmarks targeting the current PowerShell code base ..."
}
else {
Write-Log -message "Run benchmarks targeting '$TargetFramework' and the corresponding latest version of 'Microsoft.PowerShell.SDK' ..."

'TargetFramework' {
$message = if ($TargetFramework -eq 'net6.0') { 'the current PowerShell code base ...' } else { "the corresponding latest version of 'Microsoft.PowerShell.SDK' ..." }
Write-Log -message "Run benchmarks targeting '$TargetFramework' and $message"

## Use 'Release' instead of 'release' (note the capital case) because BDN uses 'Release' when building the auto-generated
## project, and MSBuild somehow recognizes 'release' and 'Release' as two different configurations and thus will rebuild
## all dependencies unnecessarily.
dotnet run -c Release -f $TargetFramework $runArgs
}

## Use 'Release' instead of 'release' (note the capital case) because BDN uses 'Release' when building the auto-generated
## project, and MSBuild somehow recognizes 'release' and 'Release' as two different configurations and thus will rebuild
## all dependencies unnecessarily.
dotnet run -c Release -f $TargetFramework $runArgs
}
else {
Write-Log -message "Run benchmarks targeting multiple .NET runtimes: $Runtime ..."
dotnet run -c Release -f net6.0 --runtimes $Runtime $runArgs
'Runtimes' {
Write-Log -message "Run benchmarks targeting multiple .NET runtimes: $Runtime ..."

## Use 'Release' instead of 'release' (note the capital case) because BDN uses 'Release' when building the auto-generated
## project, and MSBuild somehow recognizes 'release' and 'Release' as two different configurations and thus will rebuild
## all dependencies unnecessarily.
dotnet run -c Release -f net6.0 --runtimes $Runtime $runArgs
}
}

if (Test-Path $Artifacts) {
Expand Down Expand Up @@ -211,3 +203,5 @@ function Compare-BenchmarkResult
Pop-Location
}
}

Export-ModuleMember -Function 'Start-Benchmarking', 'Compare-BenchmarkResult'